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
BWBrandon Williams <bmwill@google.com>
Date
May 16, 2017, 17:30 UTC
Message-ID
<20170516173041.GH79147@google.com>
In-Reply-To
<20170516172307.36hyshwypomlsubx@sigill.intra.peff.net>
On 05/16, Jeff King wrote:
Show 33 quoted lines
> On Tue, May 16, 2017 at 10:15:40AM -0700, Brandon Williams wrote:
> 
> > > > > 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.
> 
> > > 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:

Oh yes, I was just saying what I did not taking into account how we would solve this issue.

Show 14 quoted lines
> 
>   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
-- 
Brandon Williams
Previous: Jeff KingNext: Linus Torvalds
Message 9 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.