Re: git rebase regression: cannot pass a shell expression directly to --exec
- From
Jeff King <peff@peff.net>
- Date
- May 16, 2017, 17:23 UTC
- Message-ID
- <20170516172307.36hyshwypomlsubx@sigill.intra.peff.net>
- In-Reply-To
- <20170516171540.GG79147@google.com>
On Tue, May 16, 2017 at 10:15:40AM -0700, Brandon Williams wrote:
Show 9 quoted lines
> > > > 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.
Most other shells don't do function-exporting at all. Certainly dash and most traditional bourne shells don't. I wouldn't be surprised if zsh does. But yeah, we'd have to support them one by one (and possibly variants across different versions of each shell). Workable, but gross.
Show 14 quoted lines
> > 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
I think the logic here would be more like:
1. During prepare_shell_cmd(), even if we optimize out the shell call,
still prepare a fallback argv (since we can't allocate memory
post-fork). 2. In the forked child, if we get ENOENT from exec and cmd->use_shell
is set, then exec the fallback shell argv instead. Propagate its
results, even if it's 127.That still means we'd prefer a $PATH copy of a command to its shell builtin variant, but that can't be helped (and I kind of doubt anybody would care too much).
-Peff