Re: [PATCH v3] To make GIT-VERSION-FILE, search for git more widely

From: Matthias Andree <matthias.andree@gmx.de>
Date: May 8, 2009, 08:27 UTC
Message-ID: <op.utlq3vgx1e62zd@merlin.emma.line.org>
In-Reply-To: <7v7i0scvcf.fsf@alter.siamese.dyndns.org>

Am 08.05.2009, 02:05 Uhr, schrieb Junio C Hamano <gitster@pobox.com>:

Show 20 quoted lines

> Matthias Andree <matthias.andree@gmx.de> writes:
>
>> The underlying problem flow is:
>>
>> 1 - Makefile has "include GIT-VERSION-FILE", thus gmake builds
>>     GIT-VERSION-FILE early.
>>
>> 2 - GIT-VERSION-FILE depends on a .PHONY target  
>> (.FORCE-GIT-VERSION-FILE)
>> 3 - Thus, GNU make *always* executes GIT-VERSION-GEN
>> 4 - GIT-VERSION-GEN now, under the stripped $PATH, cannot find "git" and
>>     sees a different version number.
>> 5 - GIT-VERSION-GEN notes the difference in versions and regenerates
>>     GIT-VERSION-FILE, with up-to-date timestamp.
>
> Interesting.  I wonder if you need the change to the Makefile.
>
> As long as GIT-VERSION-GEN notices that you have a freshly built git
> available (test -x) and uses it, falling back to whatever on the PATH, it
> would not have to touch GIT-VERSION-FILE, no?

Hi Junio,

Makefile (+ GNU make) is the actual culprit and introduces this cycle, why do we want to leave Makefile - of all things - alone?

Makefile also has all the info: (1) locations, for VPATH builds, (2) $(prefix), (3) $X (extension), so let's have it communicate that (through the $GIT variable). Let's not introduce second-guessing into the script. It would be error prone and manually duplicates efforts that either are already there or are automatic in Makefile. G-V-GEN is run under Makefile's control, so let's steer it into the right direction.

The G-V-GEN test -x is only there to fall back to a path search if the variable is unset when running things directly, rather than through make.

HTH Matthias

Show 26 quoted lines

> IOW, instead of this:
>
>> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
>> index 39cde78..d0dfef3 100755
>> --- a/GIT-VERSION-GEN
>> +++ b/GIT-VERSION-GEN
>> @@ -2,6 +2,7 @@
>>
>>  GVF=GIT-VERSION-FILE
>>  DEF_VER=v1.6.3.GIT
>> +test -x "$GIT" || GIT=git
>
> wouldn't it make more sense to do
>
> 	if test -x "git"
>         then
>         	GIT=./git
> 	elif test -x "git.exe"
>         then
>         	GIT=./git.exe
> 	else
>         	GIT=git
> 	fi
>
> and use the rest of the patch to GIT-VERSION-GEN, without touching
> Makefile at all?

-- 
Matthias Andree