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...
