From: Johannes Sixt <j.sixt@viscovery.net>
To: Frank Lichtenheld <frank@lichtenheld.de>
Cc: gitster@pobox.com, Petr Baudis <pasky@suse.cz>, git@vger.kernel.org
Subject: Re: [PATCH RESEND] Git.pm: Always set Repository to absolute path if autodetecting
Date: Wed, 27 May 2009 13:16:16 +0200 [thread overview]
Message-ID: <4A1D2100.5040903@viscovery.net> (raw)
In-Reply-To: <20090527105454.GW17706@mail-vs.djpig.de>
Frank Lichtenheld schrieb:
> 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
next prev parent reply other threads:[~2009-05-27 11:20 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-07 13:41 [PATCH RESEND] Git.pm: Set GIT_WORK_TREE if we set GIT_DIR Frank Lichtenheld
2009-05-07 13:41 ` [PATCH RESEND] Git.pm: Always set Repository to absolute path if autodetecting Frank Lichtenheld
2009-05-25 7:33 ` Johannes Sixt
2009-05-27 10:54 ` Frank Lichtenheld
2009-05-27 11:16 ` Johannes Sixt [this message]
2009-05-27 13:46 ` Frank Lichtenheld
2009-05-07 19:40 ` [PATCH RESEND] Git.pm: Set GIT_WORK_TREE if we set GIT_DIR Petr Baudis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A1D2100.5040903@viscovery.net \
--to=j.sixt@viscovery.net \
--cc=frank@lichtenheld.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=pasky@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.