From: Jeff King Date: Wed, 17 Jul 2024 07:00:50 GMT Subject: [PATCH] t4153: stop redirecting input from /dev/zero Message-ID: <20240717070050.GG547635@coredump.intra.peff.net> In-Reply-To: <20240714070558.GA575927@coredump.intra.peff.net> On Sun, Jul 14, 2024 at 03:05:58AM -0400, Jeff King wrote: > With --retry I think we would not actually read stdin at all, so we > could just remove the mention of /dev/zero entirely. But if we wanted to > be sure it did not read and choke on any input provided, I think just: > > diff --git a/t/t4153-am-resume-override-opts.sh b/t/t4153-am-resume-override-opts.sh > index a4d0c03ca6..76783bdd67 100755 > --- a/t/t4153-am-resume-override-opts.sh > +++ b/t/t4153-am-resume-override-opts.sh > @@ -45,6 +45,7 @@ test_expect_success '--3way overrides --no-3way' ' > > # Applying side1 with am --3way will succeed due to the threeway-merge. > # Applying side2 will fail as --3way does not apply to it. > + echo garbage | > test_must_fail git am --retry --3way && > test_path_is_dir .git/rebase-apply && > test side1 = "$(cat file2)" > @@ -99,7 +100,8 @@ test_expect_success '--reject overrides --no-reject' ' > test_path_is_dir .git/rebase-apply && > test_path_is_missing file.rej && > > - test_must_fail git am --retry --reject + echo garbage | > + test_must_fail git am --retry --reject && > test_path_is_dir .git/rebase-apply && > test_path_is_file file.rej > ' Looking at the history, the use of /dev/zero was not here to simulate garbage on stdin. It really was just meant to be a descriptor which never ran out of input, as a workaround for test_terminal. This is described in the commit mentioned below. So I think we should just do this, which can go on top of jk/am-retry. I still do not think it is super-urgent, but given the simplicity of the fix is probably worth doing for the upcoming v2.46, which exposes these tests to more platforms. -- >8 -- Subject: [PATCH] t4153: stop redirecting input from /dev/zero Commit 852a171018 (am: let command-line options override saved options, 2015-08-04) redirected a few "git am" invocations from /dev/zero, even though it did not expect "am" to read the input. This was necessary at the time because those tests used test_terminal, and as described in 18d8c26930 (test_terminal: redirect child process' stdin to a pty, 2015-08-04): 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: Jeff King --- t/t4153-am-resume-override-opts.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t4153-am-resume-override-opts.sh b/t/t4153-am-resume-override-opts.sh index a4d0c03ca6..dd6ad8f7a8 100755 --- a/t/t4153-am-resume-override-opts.sh +++ b/t/t4153-am-resume-override-opts.sh @@ -99,7 +99,7 @@ test_expect_success '--reject overrides --no-reject' ' test_path_is_dir .git/rebase-apply && test_path_is_missing file.rej && - test_must_fail git am --retry --reject