Re: [PATCH] Use the best available exec path only

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Sun, 11 Nov 2007, Bj?rn Steinbrink wrote:

> On 2007.11.11 11:43:02 -0800, Junio C Hamano wrote:
> > Brian Gernhardt <benji@silverinsanity.com> writes:
> > 
> > > I'm sorry, I should have been more clear.  I was referring to the 
> > > GIT_EXEC_PATH build variable, not the environment variable.  The git 
> > > wrapper always adds the path determined during build to the front of 
> > > PATH.  When I was changing my build script, this got set to "/usr/ 
> > > local/bin" (I usually use /usr/local/stow/git, instead).  Since I 
> > > have a /usr/local/bin/vim, PATH for git-commit.sh during the test 
> > > was:
> > >
> > > - my git build directory
> > > - /usr/local/bin (containing a symlink vi -> vim)
> > > - the t/trash directory, added by the test via `PATH=".:$PATH"`
> > > (containing the test vi script)
> > > - my normal path
> > 
> > Maybe that is what is broken.  t/test-lib.sh makes the environment 
> > variable point at the build directory, and that should override the 
> > path that is compiled in, shouldn't it?
> 
> Maybe you prefer this patch then? "make test" survived up to 9101/25, 
> but that fails with the current master anyway and I didn't bother to run 
> the remaining tests manually, so it seems to be fine. Might break some 
> weird setups that rely on being able to set multiple additional paths 
> though (not that I think that that is a good idea to begin with).
> 
> Bj?rn
> ---
> Instead of adding all possible exec paths to PATH, only add the best
> one, following the same rules that --exec-path, without arguments, uses
> to figure out which path to display.
> 
> Signed-off-by: Bj?rn Steinbrink <B.Steinbrink@gmx.de>
> ---

For easy application by the maintainer, please make the commit message the 
first part, then have a single "---", and then the quoted mail.

> diff --git a/exec_cmd.c b/exec_cmd.c
> index 2d0a758..9c376ad 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -48,9 +48,7 @@ void setup_path(const char *cmd_path)
>  
>  	strbuf_init(&new_path, 0);
>  
> -	add_path(&new_path, argv_exec_path);
> -	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
> -	add_path(&new_path, builtin_exec_path);
> +	add_path(&new_path, git_exec_path());
>  	add_path(&new_path, cmd_path);

I wonder why cmd_path is still there, then.  (I'd have expected something 
like

	add_path(&new_path, cmd_path ? cmd_path : git_exec_path());

In related news, IMO cmd_path should be made absolute if it is not already 
the case.

Ciao,
Dscho