Thread (16 messages) 16 messages, 4 authors, 2019-06-25

Re: [PATCH v2 3/4] rebase: fix garbled progress display with '-x'

From: SZEDER Gábor <hidden>
Date: 2019-06-17 19:13:35

Possibly related (same subject, not in this thread)

On Fri, Jun 14, 2019 at 08:42:29PM +0200, Johannes Schindelin wrote:
The `SHELL` thing here is important, as t/t3404-rebase-interactive.sh sets
this to the empty value explicitly:

https://github.com/git/git/blob/v2.22.0/t/t3404-rebase-interactive.sh#L63-L68
Hmm, hang on for a sec.  Why does it set SHELL to empty?

So t3404 sets SHELL to the empty value since cd035b1cef
(rebase -i: add exec command to launch a shell command, 2010-08-10),
and the in-test comment highlighted on the above link says:

  # "exec" commands are run with the user shell by default, but this may
  # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
  # to create a file. Unsetting SHELL avoids such non-portable behavior
  # in tests. It must be exported for it to take effect where needed.
  SHELL=

Furthermore, the corresponding documentation added in cd035b1cef
says the following:

  The "exec" command launches the command in a shell (the one specified
  in `$SHELL`, or the default shell if `$SHELL` is not set)

IOW if SHELL is not set/empty, then it will run the default '/bin/sh',
but who knows what it might be, there's no guarantee that it's a sane
POSIX shell.  That's why we have "Define SHELL_PATH to a POSIX shell
if your /bin/sh is broken." very near to the top of our Makefile.

So while setting SHELL to the empty value might indeed avoid
non-portable behavior when the user in general prefers a non-POSIX
shell, it wouldn't help if '/bin/sh' is broken.  For that it should be
set to $SHELL_PATH (or perhaps $TEST_SHELL_PATH nowadays...).

Though cd035b1cef is now close to 9 years old, plenty of time for
somebody to run into this issue in the wild and speak up...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help