On Tue, May 16, 2017 at 12:40:51PM +0900, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > Interesting. The "exec" string is still run with a shell. E.g.:
> > ...
> > I wonder if this is falling afoul of the optimization in run-command's
> > prepare_shell_cmd() to skip shell invocations for "simple" commands.
> > ...
> > So I suspect if you added an extraneous semi-colon, your case would work
> > again (and that would confirm for us that this is indeed the problem).
>
> Wow, that's a brilliant analysis.
If it's right. :) It's all theory at this point.
My /bin/sh isn't bash, but I should be able to build with
SHELL_PATH=/bin/bash to reproduce. But I can't:
$ bash
$ foo() { echo >&2 "in foo"; }
$ export -f foo
$ bash -c foo
in foo
$ strace -f 2>&1 git.compile rebase -x 'foo;' HEAD^ | grep foo
Executing: foo;
[pid 1788] execve("/bin/bash", ["/bin/bash", "-c", "foo;", "foo;"], [/* 60 vars */] <unfinished ...>
foo;: foo: command not found
So I'm not sure why the direct "bash -c" can find it, but somehow the
variable doesn't make it through to the "bash -c" at the lower level.
Replacing "foo;" with "env" shows the environment, but BASH_FUNC_foo
isn't set in it. I'm not sure where it's getting eaten, though.
> The "semicolon" trick is way too obscure, but perhaps can be
> documented as an escape hatch?
Yeah, I agree this should be documented if it can't be fixed. I wasn't
sure if we were giving up just yet.
Either way, though, it wouldn't hurt to mention optimizing out "maybe
shell" optimization, because it can occasionally produce user-visible
effects. Where would be a good place? In git(1), I guess?
It's almost something that could go in gitcli(7), but it's not really
about the CLI in particular. In most cases the shell-exec'd commands are
from config, but not always (as this case shows). So git-config(1)
probably isn't the right place.
AFAIK, we don't talk about this behavior at all in the existing
documentation. And I really don't know where we'd put it that somebody
would find it without having to read the documentation exhaustively.
-Peff