Re: [PATCH v2 1/3] test_terminal: redirect child process' stdin to a pty
On Fri, Aug 7, 2015 at 6:15 AM, Eric Sunshine wrote: > An alternative would be to have git-am detect that it is being tested > and pretend that isatty() returns true. I would vastly prefer a solution that would work for everything, for all the C code and scripts, instead of implementing a workaround in git-am :( In this case, I implemented a generic solution in test-terminal.perl that works for POSIX systems, so if there are no problems with its implementation, I do think it's better. Other than the fact that it does not work on non-Unix platforms, of course. The other approach I would consider is to implement a xisatty() function that returns true for xisatty(0) if TEST_TTY=0 or something. However, I do wonder if this would lead us to have to hack around other functions of terminals as well (e.g. if xisatty(0), tcgetattr()), which would be a big can of worms I think... > There is some precedent for > having core functionality recognize that it is being tested. See, for > instance, environment variable TEST_DATE_NOW, (Hmm, I took a look, and it seems that TEST_DATE_NOW is only checked in test-date.c...) > and rev-list --test-bitmap. > Doing so would allow the tests work on non-Unix > platforms, as well. Ehh, if the non-Unix platforms do not implement terminals, it means that the git-am logic to detect if we are attempting to feed it a patch by checking if stdin is a TTY is invalid anyway, so implementing a "yeah-it-is-a-tty" workaround for the sake of tests would be hiding the problem, I think. Thanks, Paul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] test_terminal: redirect child process' stdin to a pty
On Tue, Aug 4, 2015 at 10:08 AM, Paul Tan wrote: > When resuming, git-am detects if we are trying to feed it patches or not > by checking if stdin is a TTY. > > However, the test library redirects stdin to /dev/null. This makes it > difficult, for instance, to test the behavior of "git am -3" when > resuming, as git-am will think we are trying to feed it patches and > error out. > > Support this use case by extending test-terminal.perl to create a > pseudo-tty for the child process' standard input as well. An alternative would be to have git-am detect that it is being tested and pretend that isatty() returns true. There is some precedent for having core functionality recognize that it is being tested. See, for instance, environment variable TEST_DATE_NOW, and rev-list --test-bitmap. Doing so would allow the tests work on non-Unix platforms, as well. > Note that due to the way the code is structured, the child's stdin > pseudo-tty will be closed when we finish reading from our stdin. This > means that in the common case, where our stdin is attached to /dev/null, > the child's stdin pseudo-tty will be closed immediately. Some operations > like isatty(), which git-am uses, require the file descriptor to be > open, and hence if the success of the command depends on such functions, > test_terminal's stdin should be redirected to a source with large amount > of data to ensure that the child's stdin is not closed, e.g. > > test_terminal git am --3way > Signed-off-by: Paul Tan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] test_terminal: redirect child process' stdin to a pty
When resuming, git-am detects if we are trying to feed it patches or not
by checking if stdin is a TTY.
However, the test library redirects stdin to /dev/null. This makes it
difficult, for instance, to test the behavior of "git am -3" when
resuming, as git-am will think we are trying to feed it patches and
error out.
Support this use case by extending test-terminal.perl to create a
pseudo-tty for the child process' standard input as well.
Note that due to the way the code is structured, the child's stdin
pseudo-tty will be closed when we finish reading from our stdin. This
means that in the common case, where our stdin is attached to /dev/null,
the child's stdin pseudo-tty will be closed immediately. Some operations
like isatty(), which git-am uses, require the file descriptor to be
open, and hence if the success of the command depends on such functions,
test_terminal's stdin should be redirected to a source with large amount
of data to ensure that the child's stdin is not closed, e.g.
test_terminal git am --3way
Cc: Jeff King
Signed-off-by: Paul Tan
---
t/test-terminal.perl | 25 -
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/t/test-terminal.perl b/t/test-terminal.perl
index 1fb373f..f6fc9ae 100755
--- a/t/test-terminal.perl
+++ b/t/test-terminal.perl
@@ -5,15 +5,17 @@ use warnings;
use IO::Pty;
use File::Copy;
-# Run @$argv in the background with stdio redirected to $out and $err.
+# Run @$argv in the background with stdio redirected to $in, $out and $err.
sub start_child {
- my ($argv, $out, $err) = @_;
+ my ($argv, $in, $out, $err) = @_;
my $pid = fork;
if (not defined $pid) {
die "fork failed: $!"
} elsif ($pid == 0) {
+ open STDIN, "<&", $in;
open STDOUT, ">&", $out;
open STDERR, ">&", $err;
+ close $in;
close $out;
exec(@$argv) or die "cannot exec '$argv->[0]': $!"
}
@@ -50,14 +52,23 @@ sub xsendfile {
}
sub copy_stdio {
- my ($out, $err) = @_;
+ my ($in, $out, $err) = @_;
my $pid = fork;
+ if (!$pid) {
+ close($out);
+ close($err);
+ xsendfile($in, \*STDIN);
+ exit 0;
+ }
+ $pid = fork;
defined $pid or die "fork failed: $!";
if (!$pid) {
+ close($in);
close($out);
xsendfile(\*STDERR, $err);
exit 0;
}
+ close($in);
close($err);
xsendfile(\*STDOUT, $out);
finish_child($pid) == 0
@@ -67,14 +78,18 @@ sub copy_stdio {
if ($#ARGV < 1) {
die "usage: test-terminal program args";
}
+my $master_in = new IO::Pty;
my $master_out = new IO::Pty;
my $master_err = new IO::Pty;
+$master_in->set_raw();
$master_out->set_raw();
$master_err->set_raw();
+$master_in->slave->set_raw();
$master_out->slave->set_raw();
$master_err->slave->set_raw();
-my $pid = start_child(\@ARGV, $master_out->slave, $master_err->slave);
+my $pid = start_child(\@ARGV, $master_in->slave, $master_out->slave,
$master_err->slave);
+close $master_in->slave;
close $master_out->slave;
close $master_err->slave;
-copy_stdio($master_out, $master_err);
+copy_stdio($master_in, $master_out, $master_err);
exit(finish_child($pid));
--
2.5.0.280.gd88bd6e
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
