git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Frank Lichtenheld <frank@lichtenheld.de>
Date
May 27, 2009, 10:54 UTC
Message-ID
<20090527105454.GW17706@mail-vs.djpig.de>
In-Reply-To
<4A1A49C0.7040102@viscovery.net>
On Mon, May 25, 2009 at 09:33:20AM +0200, Johannes Sixt wrote:
Show 34 quoted lines
> 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/
Previous: Johannes SixtNext: Johannes Sixt
Message 4 of 7 in “Git.pm: Set GIT_WORK_TREE if we set GIT_DIR”
  1. Git.pm: Set GIT_WORK_TREE if we set GIT_DIRFrank Lichtenheld, May 7, 2009
  2. Git.pm: Always set Repository to absolute path if autodetectingFrank Lichtenheld, May 7, 2009
  3. Johannes SixtMay 25, 2009
  4. Frank LichtenheldMay 27, 2009
  5. Johannes SixtMay 27, 2009
  6. Frank LichtenheldMay 27, 2009
  7. Petr BaudisMay 7, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.