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

From: Brandon Williams <bmwill@google.com>
Date: May 16, 2017, 17:15 UTC
Message-ID: <20170516171540.GG79147@google.com>
In-Reply-To: <20170516164750.3tw6xlbcbyuu5t72@sigill.intra.peff.net>

On 05/16, Jeff King wrote:

Show 21 quoted lines

> On Tue, May 16, 2017 at 09:41:24AM -0700, Jonathan Nieder wrote:
> 
> > Jeff King wrote:
> > > On Mon, May 15, 2017 at 11:25:03PM -0400, Jeff King wrote:
> > 
> > >> One hack would be to look for BASH_FUNC_* in the environment and disable
> > >> the optimization in that case. I think that would make your case Just
> > >> Work. It doesn't help other oddball cases, like:
> > >>
> > >>   - you're trying to run a shell builtin that behaves differently than
> > >>     its exec-able counterpart
> > >>
> > >>   - your shell has some other mechanism for defining commands that we
> > >>     would not find via exec. I don't know of one offhand. Obviously $ENV
> > >>     could point to a file which defines some, but for most shells would
> > >>     not read any startup files for a non-interactive "sh -c" invocation.
> > >
> > > So I was thinking something like the patch below, though I guess
> > > technically you could look for BASH_FUNC_$argv[0]%%, which seems to be
> > > bash's magic variable name. I hate to get too intimate with those
> > > details, though.

One concern with that is what about all other shells that are not BASH? I'm sure they use a different env var for storing functions so we may need to handle other shell's too? That is assuming we want to keep the old behavior.

Show 18 quoted lines

> > >
> > > Another option is to speculatively run "foo" without the shell, and if
> > > execve fails to find it, then fall back to running the shell. That would
> > > catch any number of cases where the shell "somehow" finds a command that
> > > we can't.
> > 
> > Hm.  execvp explicitly does this when it hits ENOEXEC, but not for
> > ENOENT.  Do you know why that is?
> 
> When execvp(foo) falls back on ENOEXEC, it is not running "sh -c foo".
> It is actually running "sh foo" to run the script contents. So it's
> about letting you do:
> 
>   echo "no shebang line" >script
>   chmod +x script
>   ./script
> 
> And nothing to do with shell builtins.

That's correct, and is the exact behavior I was trying to mimic with the
changes to run_command.
  1. try executing the command.
  2. if it fails with ENOEXEC then attempt to execute it with a shell

Show 11 quoted lines

> 
> > I think we want to behave consistently for shell builtins and for
> > exported functions --- they are different sides of the same problem,
> > and behaving differently between the two feels confusing.
> 
> Yes, I think ideally we'd handle it all transparently. Although I'd also
> point out that Git the behavior under discussion dates back to 2009 and
> this is (as far as I can recall) the first report. So documenting the
> current behavior might not be that bad an option.
> 
> -Peff

-- 
Brandon Williams