From: Johannes Altmanninger <aclopte@gmail.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Git List <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] perf: fix test_export with SHELL=zsh
Date: Sat, 2 Oct 2021 13:26:40 +0200 [thread overview]
Message-ID: <20211002112640.hrn2ojndhoa2dd4c@gmail.com> (raw)
In-Reply-To: <8b70d04f-0ad1-6e68-f5a2-2d8ec3bb98ea@web.de>
On Sat, Oct 02, 2021 at 11:40:02AM +0200, René Scharfe wrote:
> Pass the value through an unquoted command substitution to force
> word-splitting even in zsh.
This seems like the wrong direction. There are probably other places where
we rely on such POSIX features. Also it doesn't fix the case where users
use other non-POSIX shells, say
SHELL=/bin/fish t/perf/run p0000-perf-lib-sanity.sh
We already have solutions in place - our top-level Makefile has
# Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
...
ifndef SHELL_PATH
SHELL_PATH = /bin/sh
endif
...
SHELL = $(SHELL_PATH)
Meanwhile t/Makefile uses
SHELL_PATH ?= $(SHELL)
TEST_SHELL_PATH ?= $(SHELL_PATH)
A related fix is to make those two consistent. I would prefer the version
in the top-level Makefile, because t/Makefile gives the false illusion that
$SHELL is honored:
SHELL=garbage make -C t t4211-line-log.sh # succeeds because make sets SHELL=/bin/sh
make SHELL=garbage -C t t4211-line-log.sh # fails
---
Anyway, for this issue we should just have t/perf/perf-lib.sh follow our
convention of using ${SHELL_PATH:-/bin/sh} instead of $SHELL.
Looks like t/interop/Makefile has the same problem.
I'll prepare the patches later unless someone beats me to it :)
next prev parent reply other threads:[~2021-10-02 11:26 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-02 9:40 [PATCH] perf: fix test_export with SHELL=zsh René Scharfe
2021-10-02 11:26 ` Johannes Altmanninger [this message]
2021-10-07 18:47 ` [PATCH] t/perf: do not run tests in user's $SHELL Johannes Altmanninger
2021-10-08 3:07 ` Jeff King
2021-10-08 5:34 ` Johannes Altmanninger
2021-10-08 5:41 ` Jeff King
2021-10-02 21:02 ` [PATCH] perf: fix test_export with SHELL=zsh brian m. carlson
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=20211002112640.hrn2ojndhoa2dd4c@gmail.com \
--to=aclopte@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=l.s.r@web.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).