Re: [PATCH] Use is_absolute_path() in diff-lib.c, lockfile.c, setup.c, trace.c

From: Steffen Prohaska <prohaska@zib.de>
Date: Nov 26, 2007, 06:45 UTC
Message-ID: <F8F93DBB-F0F5-440A-BA0C-434422C8122A@zib.de>
In-Reply-To: <7vy7clhd9z.fsf@gitster.siamese.dyndns.org>

On Nov 26, 2007, at 4:54 AM, Junio C Hamano wrote:

Show 16 quoted lines

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> Using the helper function to test for absolute paths makes porting  
>> easier.
>
> These probably make sense.  I obviously do not see any downside  
> from the
> POSIX side, and can imagine that treating "C:\" prefix as "absolute
> paths" at these four places will not have any ill effect on the  
> Windows
> side (IOW, the codepaths that follow these four places seem to do a
> sensible thing even if the "absolute path" prefix is not a single '/',
> but would work fine as-is).
>
> I am a bit surprised that there are only four places you needed to
> touch, though.

Yes, I was a bit surprised, too. I used grep to find these places. Maybe my regular expression was not good enough. On the other side, is_absolute_path() is already used at 11 places before this patch. I also cross checked with the msysgit code base. It does not use is_absolute_path() at more places.

Show 10 quoted lines

>> BTW, what happend to the msysgit related patches:
>>
>> [PATCH 1/3] sha1_file.c: Fix size_t related printf format warnings
>> [PATCH 2/3] builtin-init-db: use get_git_dir() instead of getenv()
>>
>> I never received comments about them, nor do I find them on pu.
>
> Lack of comments was probably due to mixture of bad timing and general
> lack of interests.  Many people are busy working on their turkeys than
> hacking this time of the year ;-)

Yeah, list traffic was quiet low.

Show 7 quoted lines

> I am reluctant to queue msysgit/gitwin related patches without seeing
> positive comments from other people involved on the Windows side,  
> unless
> they are trivial and obvious improvements.
>
>  * [1/3] seems without harm but on the other hand does not seem so
>    urgent either.

I did not find a simpler way to achieve a compile free warning on mingw, without introducing more complex ifdefs. I'm currently trying to reduce the differences between git.git, mingw, and msysgit.

>  * [2/3] may introduce chicken-and-egg problem (use of get_git_dir()
>    inside git-init feels quite iffy, as it calls setup_git_env(),  
> which
>    does repository discovery), without an obvious and clear advantage.

I see. I'll rethink 2/3 and 3/3. Either I come up with more convincing arguments or I'll try if the changes can be reverted in the msysgit code base.

> For these reasons, both of them disqualify from being trivial and
> obvious improvements, so I did not pick them up unilaterally before
> seeing positive comments from other people.

	Steffen