git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steve Haslam <shaslam@lastminute.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, Johannes Sixt <johannes.sixt@telecom.at>
Subject: Re: [PATCH] Glean libexec path from argv[0] for git-upload-pack and git-receive-pack.
Date: Wed, 30 Jul 2008 17:15:18 +0100	[thread overview]
Message-ID: <48909396.3080500@lastminute.com> (raw)
In-Reply-To: <alpine.LSU.1.00.0807301650060.3486@wbgn129.biozentrum.uni-wuerzburg.de>

[sorry, was intended to cc Johannes Sixt when I mailed the patch to the 
list... not doing well with the recipient etiquette]

[and then Icedove decided it was going to send mail as HTML. This is so embarassing.]

Johannes Schindelin wrote:
>> diff --git a/upload-pack.c b/upload-pack.c
>> index c911e70..086eff6 100644
>> --- a/upload-pack.c
>> +++ b/upload-pack.c
>> @@ -616,6 +616,9 @@ int main(int argc, char **argv)
>>  	int i;
>>  	int strict = 0;
>>  
>> +	if (argv[0] && *argv[0])
>> +		git_extract_argv0_path(argv[0]);
>> +
>>     
>
> This is ugly.  The called function should already do it itself.
>   

Fair enough.

> Further, why not go the full nine yards and avoid the calculation 
> altogether, until it is necessary?  Then the change to add 
> lookup_program_in_path() would be nice and non-intrusive.

git.c will always need to do the calculation, to determine which command 
it is being invoked as, so is there much value in delaying until necessary?

If the code in git.c is left alone, then it needs to be eventually 
duplicated in upload-pack.c and receive-pack.c, or in exec_cmd.c. [I 
botched when sending out the patch originally and only sent it to 
Johannes and not the list, so it's my fault that the history of how this 
has evolved is unclear I'm afraid]

> IOW why not leave the function name as-is, and just enhance system_path() 
> to have a static variable "initialized", which does the whole calculation? 
> I.e. move the calculation from git.c to exec_cmd.c, but at the same time 
> do it only when needed.

Hmm, system_path and setup_path both use argv0_path; git.c would need to 
call an additional function in exec_cmd.c to get the "leafname" result 
of the calculation, though.

> And your change to set argv0_path from receive-pack and upload-pack would 
> be a second patch.

OK.

> And then the patch to add support to "glean" (did not know that word) the 
> path from the PATH (lookup_program_in_path()) could come as a third patch.

I think that once git-upload-pack.c et al get the argv[0] path over to 
setup_path() then there's nothing more to do; setup_path() already uses 
argv0_path in its list of paths to try. I'm confused to the reference to 
PATH, though: we're avoiding the PATH environment variable completely.

SRH

  reply	other threads:[~2008-07-30 16:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-30 11:27 [PATCH] Glean libexec path from argv[0] for git-upload-pack and git-receive-pack Steve Haslam
2008-07-30 14:56 ` Johannes Schindelin
2008-07-30 16:15   ` Steve Haslam [this message]
2008-07-30 18:21     ` Johannes Schindelin
2008-07-30 18:53       ` Steve Haslam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48909396.3080500@lastminute.com \
    --to=shaslam@lastminute.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=johannes.sixt@telecom.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).