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

On 05/16, Jeff King wrote:

On Tue, May 16, 2017 at 09:41:24AM -0700, Jonathan Nieder wrote:

quoted

Jeff King wrote:

quoted

On Mon, May 15, 2017 at 11:25:03PM -0400, Jeff King wrote:

quoted

quoted

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.

quoted

quoted

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

quoted

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