Thread (29 messages) 29 messages, 9 authors, 2017-05-16

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

flat view

From: Junio C Hamano <hidden>
Date: 2017-05-16 03:40:58

Jeff King [off-list ref] 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.
The optimization in run-command is very old, but the switch to
rebase--helper presumably moved us from doing that exec in the actual
shell script to doing it via the C code.

Which means your exported-function technique has been broken for _most_
of Git all along, but it just now affected this particular spot.

I'm not sure how to feel about it. In the face of exported functions, we
can never do the shell-skipping optimization, because we don't know how
the shell is going to interpret even a simple-looking command. And it is
kind of a neat trick. But I also hate to add extra useless shell
invocations for the predominantly common case that people aren't using
this trick (or aren't even using a shell that supports function
exports).
I was about to write this off as "an unfortunate regression, a
fallout that will likely left unfixed, due to lack of a good
practical workaround."

The point of rewriting things in C and using run_command() interface
was to avoid shell overhead.  We are doing an exec already, but
adding a shell invocation in the middle will double the number of
exec (and probably add an extra fork as well), which probably is
measurable on systems with slow fork/exec.

The "semicolon" trick is way too obscure, but perhaps can be
documented as an escape hatch?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help