All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Lars Schneider <larsxschneider@gmail.com>
Subject: [PATCH 2/3] t5615: avoid re-using descriptor 4
Date: Thu, 19 Oct 2017 17:07:31 -0400	[thread overview]
Message-ID: <20171019210730.sgm4g4tmor2dgjv7@sigill.intra.peff.net> (raw)
In-Reply-To: <20171019210140.64lb52cqtgdh22ew@sigill.intra.peff.net>

File descriptors 3 and 4 are special in our test suite, as
they link back to the test script's original stdout and
stderr. Normally this isn't something tests need to worry
about: they are free to clobber these descriptors for
sub-commands without affecting the overall script.

But there's one very special thing about descriptor 4: since
d88785e424 (test-lib: set BASH_XTRACEFD automatically,
2016-05-11), we ask bash to output "set -x" output to it by
number. This goes to _any_ descriptor 4, even if it no
longer points to the place it did when we set BASH_XTRACEFD.

But in t5615, we run a shell loop with descriptor 4
redirected.  As a result, t5615 works with non-bash shells
even with "-x". And it works with bash without "-x". But the
combination of "bash t5615-alternate-env.sh -x" gets a test
failure (because our "set -x" output pollutes one of the
files).

We can fix this by using any descriptor _except_ the magical
4. So let's switch arbitrarily to using 5/6 in this loop,
not 3/4.

Signed-off-by: Jeff King <peff@peff.net>
---
I also considered trying to bump the "set -x" output descriptor to "9".
That just moves the problem around, but presumably scripts are less
likely to go that high. :)

It would also be possible to pick something insanely high, like "999".
Many shells choke on descriptors higher than 9, but since the issue is
related to BASH_XTRACEFD, we could make it a bash-only thing. I don't
know if it's worth the trouble. I hate leaving a subtle "don't use
descriptor 4 in a subshell or your script will break" hand-grenade like
this lying around, but we do seem to have only one instance of it over
the whole test suite.

 t/t5615-alternate-env.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t5615-alternate-env.sh b/t/t5615-alternate-env.sh
index d2d883f3a1..b4905b822c 100755
--- a/t/t5615-alternate-env.sh
+++ b/t/t5615-alternate-env.sh
@@ -7,9 +7,9 @@ check_obj () {
 	alt=$1; shift
 	while read obj expect
 	do
-		echo "$obj" >&3 &&
-		echo "$obj $expect" >&4
-	done 3>input 4>expect &&
+		echo "$obj" >&5 &&
+		echo "$obj $expect" >&6
+	done 5>input 6>expect &&
 	GIT_ALTERNATE_OBJECT_DIRECTORIES=$alt \
 		git "$@" cat-file --batch-check='%(objectname) %(objecttype)' \
 		<input >actual &&
-- 
2.15.0.rc1.560.g5f0609e481


  parent reply	other threads:[~2017-10-19 21:07 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-19 21:01 [PATCH 0/3] making test-suite tracing more useful Jeff King
2017-10-19 21:03 ` [PATCH 1/3] test-lib: silence "-x" cleanup under bash Jeff King
2017-10-19 21:07 ` Jeff King [this message]
2017-10-19 21:46   ` [PATCH 2/3] t5615: avoid re-using descriptor 4 Stefan Beller
2017-10-19 23:23     ` Jeff King
2017-10-20  5:50       ` Jeff King
2017-10-20 21:27         ` Stefan Beller
2017-10-20 22:46           ` Jeff King
2017-10-21  0:19             ` Simon Ruderich
2017-10-21  2:02               ` Junio C Hamano
2017-10-21  3:23               ` Jeff King
2017-10-19 21:08 ` [PATCH 3/3] test-lib: make "-x" work with "--verbose-log" Jeff King
2017-10-23 10:56   ` Johannes Schindelin
2017-10-20 22:53 ` [PATCH 4/3] t/Makefile: introduce TEST_SHELL_PATH Jeff King
2017-10-20 23:50   ` SZEDER Gábor
2017-10-21  3:12     ` Jeff King
2017-10-23 11:01   ` Johannes Schindelin
2017-10-24  1:31     ` Jeff King
2017-10-25 21:35       ` Johannes Schindelin
2017-10-25 21:50         ` Jeff King
2017-10-27 14:26           ` Johannes Schindelin
2017-10-23 11:04 ` [PATCH 0/3] making test-suite tracing more useful Johannes Schindelin
2017-10-24  1:32   ` Jeff King
2017-12-08 10:46 ` [PATCH v2 0/4] " Jeff King
2017-12-08 10:47   ` [PATCH v2 1/4] test-lib: silence "-x" cleanup under bash Jeff King
2017-12-08 10:47   ` [PATCH v2 2/4] t5615: avoid re-using descriptor 4 Jeff King
2017-12-08 10:47   ` [PATCH v2 3/4] test-lib: make "-x" work with "--verbose-log" Jeff King
2017-12-08 10:47   ` [PATCH v2 4/4] t/Makefile: introduce TEST_SHELL_PATH Jeff King
2017-12-08 15:08     ` Johannes Schindelin
2017-12-08 22:00       ` Jeff King
2017-12-09 13:44         ` Johannes Schindelin
2017-12-10 14:23           ` Jeff King
2017-12-11 20:37             ` Junio C Hamano
2017-12-15 10:41               ` Jeff King
2017-12-15 16:58                 ` Junio C Hamano
2017-12-21  9:47                   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171019210730.sgm4g4tmor2dgjv7@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=larsxschneider@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.