Re: [PATCH 4/4] drop git_exec_path() from non-Git commands' PATH

[Patrick Steinhardt <ps@pks.im>]

On Wed, May 21, 2025 at 10:50:03AM -0500, Justin Tobler wrote:
> On 25/05/21 03:17PM, Patrick Steinhardt wrote:
> > On Wed, May 21, 2025 at 02:07:25PM +0100, Phillip Wood wrote:
> > > On 21/05/2025 09:27, Patrick Steinhardt wrote:
> > > > I agree with what Junio mentioned in a parallel thread, especially
> > > > around Git hooks. The expectation there is that those may execute other
> > > > Git commands, and that should typically be using the same execution
> > > > environment as the original Git command that has been invoking the hook.
> > > > So refining this patch so that the mechanism is opt-in probably makes
> > > > sense.
> > > > 
> > > >      A slight tangent: I wonder whether it is even required nowadays to
> > > >      adapt PATH at all anymore. As far as I understand this was a
> > > >      requirement back when people still executed dashed binaries
> > > >      directly. But nowadays scripts don't really do that anymore, but
> > > >      instead use the git binary. And that one doesn't need PATH to be
> > > >      adapted at all, as it knows to listen to GIT_EXEC_PATH and its
> > > >      built-in path anyway.
> > > 
> > > But don't we still need to change PATH so that hooks, shell aliases, git
> > > rebase --exec, git bisect run, etc. still run the same git executable that
> > > started them? For example "/usr/bin/git -c alias.g=!git g --version" should
> > > report the version of /usr/bin/git, not ~/.local/bin/git which comes first
> > > in my PATH if git doesn't change it.
> > 
> > There's two parts to this: PATH and GIT_EXEC_PATH. We do have to adjust
> > PATH indeed to contain the location of the 'git' executable. But we also
> > add GIT_EXEC_PATH to it, which I'm less sure whether it's actually
> > needed.
> 
> In instances where GIT_EXEC_PATH is set on the parent process to
> override the default, wouldn't we also want this configuration to
> propagate to child Git processes?

Yes, but what we should be propagating is GIT_EXEC_PATH, not
GIT_EXEC_PATH inserted into PATH, shouldn't we?

Patrick