Re: [PATCH RESEND] Git.pm: Always set Repository to absolute path if autodetecting

From: Johannes Sixt <j.sixt@viscovery.net>
Date: May 27, 2009, 11:16 UTC
Message-ID: <4A1D2100.5040903@viscovery.net>
In-Reply-To: <20090527105454.GW17706@mail-vs.djpig.de>

Frank Lichtenheld schrieb:

Show 23 quoted lines

> On Mon, May 25, 2009 at 09:33:20AM +0200, Johannes Sixt wrote:
>> Frank Lichtenheld schrieb:
>>> --- a/perl/Git.pm
>>> +++ b/perl/Git.pm
>>> @@ -185,7 +185,7 @@ sub repository {
>>>  
>>>  		if ($dir) {
>>>  			$dir =~ m#^/# or $dir = $opts{Directory} . '/' . $dir;
>>> -			$opts{Repository} = $dir;
>>> +			$opts{Repository} = abs_path($dir);
>> Unfortunately, this change breaks MinGW git because the absolute path that
>> this produces is MSYS-style /c/path/to/repo, but git does not understand
>> this; it should be c:/path/to/repo. This value is ultimately assigned to
>> GIT_DIR, but the path name mangling that usually happens when an MSYS
>> program (like perl) spawns a non-MSYS program (like git) does not happen.
>>
>> Your commit message is quite vague about the problems that you have seen.
>> I vote to revert this change.
> 
> Note that abs_path is already used twice in the same function. Why are those
> usages not problematic? I would be happy to work with you on finding a patch
> that doesn't break, but I have to admit that I have no idea of the
> Windows<->Perl<->git interactions.

The result of abs_path() three lines below the cited context is never passed to git; only its trailing part is ever used. This does not seem to be problematic on Windows, according to the test suite.

The other use if abs_path() is about bare repositories and that is certainly problematic, but nobody uses the tools written in perl in a bare repository on Windows, obviously, otherwise we would have heard complaints. ;)

> As for the problems, a part of the public API of the module simply doesn't work
> (i.e. wc_chdir) which I fixed. If we can't fix it we should at least not pretend
> that it works.

Since you keep repeating "does not work", without any specifics, I can't help (and I'm not going to find out myself what "does not work").

-- Hannes