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

[Frank Lichtenheld <frank@lichtenheld.de>]

On Mon, May 25, 2009 at 09:33:20AM +0200, Johannes Sixt wrote:
> Frank Lichtenheld schrieb:
> > From: Frank Lichtenheld <flichtenheld@astaro.com>
> > 
> > So far we only set it to absolute paths in some cases which lead
> > to problems like wc_chdir not working.
> > 
> > Signed-off-by: Frank Lichtenheld <flichtenheld@astaro.com>
> > ---
> >  perl/Git.pm     |    2 +-
> >  t/t9700/test.pl |   10 ++--------
> >  2 files changed, 3 insertions(+), 9 deletions(-)
> > 
> > Resent unchanged. There was one comment which I've reponded too and
> > argued that it didn't apply and there was no further objections.
> > 
> > diff --git a/perl/Git.pm b/perl/Git.pm
> > index 4313db7..e8df55d 100644
> > --- 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.

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.

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/