All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Altmanninger <aclopte@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Johannes Altmanninger" <aclopte@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"René Scharfe" <l.s.r@web.de>, "Jeff King" <peff@peff.net>
Subject: [PATCH v2] t/perf: do not run tests in user's $SHELL
Date: Sat, 25 Dec 2021 09:16:58 +0100	[thread overview]
Message-ID: <20211225081656.1311583-1-aclopte@gmail.com> (raw)
In-Reply-To: <xmqqilvjugu0.fsf@gitster.g>

The environment variable $SHELL is usually set to the user's
interactive shell. Our build and test scripts never use $SHELL because
there are no guarantees about its input language.  Instead, we use
/bin/sh which should be a POSIX shell.

For systems with a broken /bin/sh, we allow to override that path via
SHELL_PATH.  To run tests in yet another shell we allow to override
SHELL_PATH with TEST_SHELL_PATH.

Perf tests run in $SHELL via a wrapper defined in t/perf/perf-lib.sh,
so they break with e.g. SHELL=python.  Use TEST_SHELL_PATH like
in other tests.  TEST_SHELL_PATH is always defined because
t/perf/perf-lib.sh includes t/test-lib.sh, which includes
GIT-BUILD-OPTIONS.

Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Johannes Altmanninger <aclopte@gmail.com>
---
 t/perf/perf-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

I extended the commit message because in hindsight it was overly terse
(judging from both re-reading it and from review comments).

We could add more Acked-bys but one seems enough here.

range-diff to the first version:

    @@ Commit message
         t/perf: do not run tests in user's $SHELL
     
         The environment variable $SHELL is usually set to the user's
    -    interactive shell. We never use that shell for build and test scripts
    -    because it might not be a POSIX shell.
    +    interactive shell. Our build and test scripts never use $SHELL because
    +    there are no guarantees about its input language.  Instead, we use
    +    /bin/sh which should be a POSIX shell.
     
    -    Perf tests are run inside $SHELL via a wrapper defined in
    -    t/perf/perf-lib.sh. Use $TEST_SHELL_PATH like elsewhere.
    +    For systems with a broken /bin/sh, we allow to override that path via
    +    SHELL_PATH.  To run tests in yet another shell we allow to override
    +    SHELL_PATH with TEST_SHELL_PATH.
    +
    +    Perf tests run in $SHELL via a wrapper defined in t/perf/perf-lib.sh,
    +    so they break with e.g. SHELL=python.  Use TEST_SHELL_PATH like
    +    in other tests.  TEST_SHELL_PATH is always defined because
    +    t/perf/perf-lib.sh includes t/test-lib.sh, which includes
    +    GIT-BUILD-OPTIONS.
    +
    +    Acked-by: Jeff King <peff@peff.net>

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 780a7402d5..407252bac7 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -161,7 +161,7 @@ test_run_perf_ () {
 	test_cleanup=:
 	test_export_="test_cleanup"
 	export test_cleanup test_export_
-	"$GTIME" -f "%E %U %S" -o test_time.$i "$SHELL" -c '
+	"$GTIME" -f "%E %U %S" -o test_time.$i "$TEST_SHELL_PATH" -c '
 . '"$TEST_DIRECTORY"/test-lib-functions.sh'
 test_export () {
 	test_export_="$test_export_ $*"
-- 
2.34.1


      parent reply	other threads:[~2021-12-25  8:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-20 11:05 [PATCH RESEND] t/perf: do not run tests in user's $SHELL René Scharfe
2021-12-20 11:56 ` Ævar Arnfjörð Bjarmason
2021-12-20 13:11   ` Johannes Altmanninger
2021-12-20 21:06     ` Junio C Hamano
2021-12-25  7:47       ` Johannes Altmanninger
2021-12-25  8:16       ` Johannes Altmanninger [this message]

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=20211225081656.1311583-1-aclopte@gmail.com \
    --to=aclopte@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    /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.