Re: [PATCH] remote-hg: fix path when cloning with tilde expansion

From: Antoine Pelisse <apelisse@gmail.com>
Date: Aug 9, 2013, 21:19 UTC
Message-ID: <CALWbr2y5H_dfHAFW_qN+j8YtF4F9+VcG8G503hr4YN2Qv69CXA@mail.gmail.com>
In-Reply-To: <7vy58a4mcy.fsf@alter.siamese.dyndns.org>

Confusion everywhere :-)

On Fri, Aug 9, 2013 at 10:53 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

> Antoine Pelisse <apelisse@gmail.com> writes:
>
>> So when we run:
>>
>> git clone hg::~/my/repo
>>
>> Git will remove the "hg::" part, and Mercurial will expand tilde and
>> clone $HOME/my/repo.
>
> Now you confused me.  If the implementation were for us to remove
> the hg:: prefix and let Mercurial do whatever it wants to do with
> the rest, you are right that we will not have to do any expansion
> like your patch.  But you sent a patch to do so, so apparently it
> is not what happens.  So where does it go wrong?
>
> Puzzled...

OK, I think I see why you are puzzled.

Cloning works fine because we "fix the path" *after* the clone is done successfully, for the following reason: If you run:

   git clone hg::./my_repo my_new_repo

The remote path will be hg::./my_repo, so we have to fix this path (otherwise you won't be able to run git fetch from inside my_new_repo). It's currently done by checking if ./my_repo is an absolute path or not, and try to make it absolute if required.

But my issue is when I do that:

    git clone hg::~/my_repo my_new_repo

The clone works successfully by cloning $HOME/my_repo, but then, when we try to fix the repo path, we think that ~/my_repo is not an absolute path, so we make it absolute: /home/user/~/my_repo which is now off. So I'm not able to fetch that remote.

What the current patch does, is to expand the tilde before checking if the path is absolute. So that fixes the bug, but that indeed can be confusing to another user that would expect hg::~/my_repo/ to *not be* hg::$HOME/my_repo (because he knows the expansion should not happen in that case).

Show 9 quoted lines

>>> By the way, you earlier sent an updated 1/2; is this supposed to be
>>> 2/2 to conclude the two-patch series?
>>
>> Those two patches don't interact with each other, but you can of
>> course join them if it makes it easier for you (and I don't think one
>> is going to have to go "faster" than the other anyway).
>
> Hmph, so there is a different 2/2 that we haven't seen recently on
> the list (meaning you have three patches)?

I have 2 patch (1 from me, 1 from Felipe): One with the tilde expansion, the other one with shared_path initialization (which now conflicts with the resend from Felipe)

I will try to provide a better versioning of the patches next time.

Sorry for the confusion, Thanks,