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

On Tue, May 16, 2017 at 10:21:51AM -0700, Eric Rannaud wrote:

On Tue, May 16, 2017 at 9:59 AM, Eric Rannaud [off-list ref] wrote:

quoted

When I use "git rebase --exec <cmd>" I'm basically writing a "foreach
commit in range { <cmd> }" in my shell. Same idea with git bisect run.

A transparent optimization that tries execve() then falls back to the
user's shell sounds like a good idea.

One issue with the execve-else-shell optimization is that sometimes a
binary exists that will shadow an exported function or a shell
builtin:

  git rebase --exec true master^^  # OK but in fact this runs /usr/bin/true

Yeah, this is the builtin thing I mentioned elsewhere. I think it's
pretty rare to run a builtin with no arguments and care about the
behavior differences.

/usr/bin/time requires an argument. Even though the bash builtin time
runs fine without argument.

  $ time

  real    0m0.000s
  user    0m0.000s
  sys     0m0.000s

I've run into the "time" distinction even when running things from the
shell, because the time builtin is special. E.g.:

  $ time true
  real	0m0.000s
  user	0m0.000s
  sys	0m0.000s

  $ (echo foo | time true) 2>&1
  0.00user 0.00system 0:00.00elapsed 0%CPU (0avgtext+0avgdata 1136maxresident)k
  0inputs+0outputs (0major+61minor)pagefaults 0swaps

So to some degree, depending on builtins versus external commands
(especially when you're round-tripping through another program running a
second shell) is going to have some surprises.

But if the optimization is applied to more complex commands, then we
will have problems. For instance, the builtin echo supports \E, but
/usr/bin/echo doesn't support it.

No, it shouldn't. If any of

     |&;<>()$`\\\"' \t\n*?[#~=%

appear in the string, we always run the shell.

In any case, the manpage says --exec <cmd> and "<cmd> will be
interpreted as one or more shell commands.", it doesn't say "--exec
<executable>".

Right, it's clearly supposed to use the shell, or behave as if the shell
were invoked (within reason).

-Peff