Re: [PATCH] help git-upload-pack find git

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Sep 16, 2008, 13:15 UTC
Message-ID: <48CFB183.1090205@drmicha.warpmail.net>
In-Reply-To: <48CF4F6A.6080604@viscovery.net>

Johannes Sixt venit, vidit, dixit 16.09.2008 08:17:

Show 29 quoted lines

> Michael J Gruber schrieb:
>> I was actually surprised that setup_path() uses argv0_path without
>> setting it, same as with argv_exec_path. I assumed this is for a good
>> reason, I'm lacking the code base overview to judge this myself.
> 
> It argv_exec_path comes from git --exec-path=..., hence, git (via git.c)
> is the only caller of setup_path() that is able to set it. We can leave
> that one out of the equation.
> 
>> In any case, git.c sets argv0_path early, messes a bit with argv[0] and
>> calls setup_path afterwards anyways. So adding the path in setup_path()
>> should not hurt any "git foo" command.
>>
>> One could construe situations where even that wouldn't help, because
>> git-upload-pack can't pass --exec-dir to git and they can be in
>> different locations - but I think that's crazy.
>>
>> git.c, upload-pack.c, receive.pack.c and shell.c are the only callers.
>> setup_path() needs to get a parameter. If shell.c should profit from the
>> change then it needs to be taught how to pass an absolute path to
>> do_{generic,cvs}_cmd().
>>
>> So, I guess the general approach (change setup() path and have every
>> caller profit) is OK. OK with you?
> 
> Have you studied the commit message of e1464ca7bb0d (Record the command
> invocation path early) and the context in which this commit occurs? It's
> about relocatable git installations and how system_path() derives various
> other paths from argv[0].

That is what I meant by my lack of overview ;) I might have implemented what amounts to a revert of e1464ca7bb0d otherwise.

> Please show how you think you could change setup_path(), but keep in mind
> that in git.c you neither can do the equivalent of git_set_argv0_path()
> later nor setup_path() earlier.

Well, I'd be happy with the change in upload-pack as submitted. Junio suggested the more general approach. In fact I'd prefer everyone set their path, but not everyone seems to be allowed to in their restricted environments.

Why is it that setup_path() cannot be done before commit_pager_choice()? We need the pager in case list_common_cmds_help() is triggered, but why can't setup_path() be before that? Not because of "--", really.

Michael