git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] t/perf: do not run tests in user's $SHELL

From
Johannes Altmanninger <aclopte@gmail.com>
Date
Dec 25, 2021, 08:16 UTC
Message-ID
<20211225081656.1311583-1-aclopte@gmail.com>
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
Previous: Johannes Altmanninger
Message 6 of 6 in “t/perf: do not run tests in user's $SHELL”
  1. t/perf: do not run tests in user's $SHELLRené Scharfe, Dec 20, 2021
  2. Ævar Arnfjörð BjarmasonDec 20, 2021
  3. Johannes AltmanningerDec 20, 2021
  4. Junio C HamanoDec 20, 2021
  5. Johannes AltmanningerDec 25, 2021
  6. t/perf: do not run tests in user's $SHELLJohannes Altmanninger, Dec 25, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.