git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
EREric Rannaud <eric.rannaud@gmail.com>
Date
May 16, 2017, 17:21 UTC
Message-ID
<CA+zRj8XLQiTmiVbsJZ7XjdW4bQvMSJpCtKW8xx5JvMKOhBDmLQ@mail.gmail.com>
In-Reply-To
<CA+zRj8Uxh1SGdVW=zz29Q4m4OnDcs665T0XtV6FiZ_qZSiG-8g@mail.gmail.com>
On Tue, May 16, 2017 at 9:59 AM, Eric Rannaud <eric.rannaud@gmail.com> wrote:
Show 5 quoted lines
> 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

In this case it doesn't really matter. If the optimization is only applied to "simple commands" (i.e. no arguments), then it's probably OK. I can't think of problematic cases. Except weird things like:

  $ git rebase --exec time master^
  Executing: time
  Usage: time [-apvV] [-f format] [-o file] [--append] [--verbose]
         [--portability] [--format=format] [--output=file] [--version]
         [--help] command [arg...]
  warning: execution failed: time

/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

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.

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>".

Previous: Jeff KingNext: Jeff King
Message 28 of 29 in “git rebase regression: cannot pass a shell expression directly to --exec”
  1. Eric RannaudMay 15, 2017
  2. Jeff KingMay 16, 2017
  3. Jeff KingMay 16, 2017
  4. Jonathan NiederMay 16, 2017
  5. Jeff KingMay 16, 2017
  6. Eric RannaudMay 16, 2017
  7. Brandon WilliamsMay 16, 2017
  8. Jeff KingMay 16, 2017
  9. Brandon WilliamsMay 16, 2017
  10. Linus TorvaldsMay 16, 2017
  11. [TANGENT] run-command: use vfork instead of forkEric Wong, May 16, 2017
  12. Linus TorvaldsMay 16, 2017
  13. Brandon WilliamsMay 16, 2017
  14. Johannes SchindelinMay 16, 2017
  15. Linus TorvaldsMay 16, 2017
  16. Jonathan NiederMay 16, 2017
  17. Junio C HamanoMay 16, 2017
  18. Jeff KingMay 16, 2017
  19. Jeff KingMay 16, 2017
  20. Eric RannaudMay 16, 2017
  21. Johannes SchindelinMay 16, 2017
  22. Johannes SchindelinMay 16, 2017
  23. Jeff KingMay 16, 2017
  24. Eric RannaudMay 16, 2017
  25. Kevin DaudtMay 16, 2017
  26. Eric RannaudMay 16, 2017
  27. Jeff KingMay 16, 2017
  28. Eric RannaudMay 16, 2017
  29. Jeff KingMay 16, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.