Re: git rebase regression: cannot pass a shell expression directly to --exec

From: Jeff King <peff@peff.net>
Date: May 16, 2017, 03:53 UTC
Message-ID: <20170516035357.2wutcd2tyax4tjyn@sigill.intra.peff.net>
In-Reply-To: <xmqqr2zp31vg.fsf@gitster.mtv.corp.google.com>

On Tue, May 16, 2017 at 12:40:51PM +0900, Junio C Hamano wrote:

Show 11 quoted lines

> Jeff King <peff@peff.net> 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