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

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

[Antoine Pelisse]

The current code fixes the path to make it absolute when cloning, but
doesn't consider tilde expansion, so that scenario fails throwing an
exception because /home/myuser/~/my/repository doesn't exists:

    $ git clone hg::~/my/repository && cd repository && git fetch

Fix that by using python os.path.expanduser method.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 contrib/remote-helpers/git-remote-hg |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 02404dc..4bbd296 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -1137,7 +1137,7 @@ def fix_path(alias, repo, orig_url):
     url = urlparse.urlparse(orig_url, 'file')
     if url.scheme != 'file' or os.path.isabs(url.path):
         return
-    abs_url = urlparse.urljoin("%s/" % os.getcwd(), orig_url)
+    abs_url = os.path.abspath(os.path.expanduser(orig_url))
     cmd = ['git', 'config', 'remote.%s.url' % alias, "hg::%s" % abs_url]
     subprocess.call(cmd)
 
-- 
1.7.9.5

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

[Felipe Contreras]

On Mon, Aug 5, 2013 at 3:12 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
> The current code fixes the path to make it absolute when cloning, but
> doesn't consider tilde expansion, so that scenario fails throwing an
> exception because /home/myuser/~/my/repository doesn't exists:
>
>     $ git clone hg::~/my/repository && cd repository && git fetch
>
> Fix that by using python os.path.expanduser method.

Shouldn't that be the job of the shell? (s/~/$HOME/)

-- 
Felipe Contreras

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

[Antoine Pelisse]

On Mon, Aug 5, 2013 at 10:30 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Mon, Aug 5, 2013 at 3:12 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
>>     $ git clone hg::~/my/repository && cd repository && git fetch
>>
>> Fix that by using python os.path.expanduser method.
>
> Shouldn't that be the job of the shell? (s/~/$HOME/)

I guess it is, as long as it looks like a path:

    $ echo ~
    /home/myuser
    $ echo hg::~
    hg::~

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

[Antoine Pelisse]

The current code fixes the path to make it absolute when cloning, but
doesn't consider tilde expansion, so that scenario fails throwing an
exception because /home/myuser/~/my/repository doesn't exists:

    $ git clone hg::~/my/repository && cd repository && git fetch

Expand the tilde when checking if the path is absolute, so that we don't
fix a path that doesn't need to be.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
On Mon, Aug 5, 2013 at 10:30 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:
> Shouldn't that be the job of the shell? (s/~/$HOME/)

I'm not sure what you mean here. Does it mean that I should stop cloning using "~" ?
I also send this patch as I think it makes more sense to keep the ~ in the path, but just make
sure we don't build invalid absolute path.

By the way, I don't exactly understand why:

    abs_url = urlparse.urljoin("%s/" % os.getcwd(), orig_url)

is done right after instead of:

    abs_url = os.path.abspath(orig_url)

Cheers,
Antoine

 contrib/remote-helpers/git-remote-hg |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 1897327..861c498 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -1135,7 +1135,7 @@ def do_option(parser):

 def fix_path(alias, repo, orig_url):
     url = urlparse.urlparse(orig_url, 'file')
-    if url.scheme != 'file' or os.path.isabs(url.path):
+    if url.scheme != 'file' or os.path.isabs(os.path.expanduser(url.path)):
         return
     abs_url = urlparse.urljoin("%s/" % os.getcwd(), orig_url)
     cmd = ['git', 'config', 'remote.%s.url' % alias, "hg::%s" % abs_url]
--
1.7.9.5

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

[Junio C Hamano]

Antoine Pelisse <apelisse@gmail.com> writes:

> The current code fixes the path to make it absolute when cloning, but
> doesn't consider tilde expansion, so that scenario fails throwing an
> exception because /home/myuser/~/my/repository doesn't exists:
>
>     $ git clone hg::~/my/repository && cd repository && git fetch
>
> Expand the tilde when checking if the path is absolute, so that we don't
> fix a path that doesn't need to be.
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---
> On Mon, Aug 5, 2013 at 10:30 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> Shouldn't that be the job of the shell? (s/~/$HOME/)
>
> I'm not sure what you mean here. Does it mean that I should stop cloning using "~" ?

I think shells do not expand ~ when it appears in a string (e.g. hg::~/there);
you could work it around with

	git clone hg::$(echo ~/there)

and I suspect that is what Felipe is alluding to.  A tool (like
remote-hg bridge with this patch) that expands ~ in the middle of a
string also may be surprising to some people, especially to those
who know the shell does not.

> I also send this patch as I think it makes more sense to keep the
> ~ in the path, but just make sure we don't build invalid absolute
> path.
>
> By the way, I don't exactly understand why:
>
>     abs_url = urlparse.urljoin("%s/" % os.getcwd(), orig_url)
>
> is done right after instead of:
>
>     abs_url = os.path.abspath(orig_url)

That looks like a good cleanup to me, too, but I may be missing some
subtle points...

By the way, you earlier sent an updated 1/2; is this supposed to be
2/2 to conclude the two-patch series?

> Cheers,
> Antoine
>
>  contrib/remote-helpers/git-remote-hg |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
> index 1897327..861c498 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -1135,7 +1135,7 @@ def do_option(parser):
>
>  def fix_path(alias, repo, orig_url):
>      url = urlparse.urlparse(orig_url, 'file')
> -    if url.scheme != 'file' or os.path.isabs(url.path):
> +    if url.scheme != 'file' or os.path.isabs(os.path.expanduser(url.path)):
>          return
>      abs_url = urlparse.urljoin("%s/" % os.getcwd(), orig_url)
>      cmd = ['git', 'config', 'remote.%s.url' % alias, "hg::%s" % abs_url]
> --
> 1.7.9.5

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

[Antoine Pelisse]

On Fri, Aug 9, 2013 at 8:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Antoine Pelisse <apelisse@gmail.com> writes:
>> On Mon, Aug 5, 2013 at 10:30 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>> Shouldn't that be the job of the shell? (s/~/$HOME/)
>>
>> I'm not sure what you mean here. Does it mean that I should stop cloning using "~" ?
>
> I think shells do not expand ~ when it appears in a string (e.g. hg::~/there);
> you could work it around with
>
>         git clone hg::$(echo ~/there)
>
> and I suspect that is what Felipe is alluding to.  A tool (like
> remote-hg bridge with this patch) that expands ~ in the middle of a
> string also may be surprising to some people, especially to those
> who know the shell does not.

It looks like mercurial will expand the tilde (it it starts with it):

   hg init \~

will create a $HOME/.hg. (while git init \~ will create ./~).

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.

So what should we do ? I think we should stick as close as possible to
Hg behavior:
That is consider that a path starting with tilde is absolute, and not
try to fix it by building /home/user/~/repo/path.
Of course if we could not depend on "I think Hg works like that", it
would be better if we could resolve that by asking Mercurial.
I will dig into it.

> 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).

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

[Junio C Hamano]

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...

>> 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)?

Thanks.

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

[Antoine Pelisse]

Confusion everywhere :-)

On Fri, Aug 9, 2013 at 10:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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).

>>> 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,

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

[Junio C Hamano]

Antoine Pelisse <apelisse@gmail.com> writes:

> OK, I think I see why you are puzzled.
> ...
> 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.

OK, so clone works, but subsequent fetch from the cloned resoitory
does not?  "git fetch hg::~/my_repo" will still work but the call to
"git config" done near the place your patch touches does not store
"hg::~/my_repo" because it thinks "~/my_repo" refers to
"./~/my_repo" and tries to come up with an absolute path.  The patch
tries to notice this case and return without rewriting, so that
remote.*.url is kept as "hg::~/my_repo".

Assuming that I am following your reasoning so far, I think I can
agree with the patch (not that my agreement matters that much, as
you seem to be a lot more familiar with this codepath).

Thanks for explaining.

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

[Antoine Pelisse]

On Fri, Aug 9, 2013 at 11:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> OK, so clone works, but subsequent fetch from the cloned resoitory
> does not?  "git fetch hg::~/my_repo" will still work but the call to
> "git config" done near the place your patch touches does not store
> "hg::~/my_repo" because it thinks "~/my_repo" refers to
> "./~/my_repo" and tries to come up with an absolute path.  The patch
> tries to notice this case and return without rewriting, so that
> remote.*.url is kept as "hg::~/my_repo".
>
> Assuming that I am following your reasoning so far, I think I can
> agree with the patch (not that my agreement matters that much, as
> you seem to be a lot more familiar with this codepath).
>
> Thanks for explaining.

Thanks

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

[Felipe Contreras]

On Fri, Aug 9, 2013 at 4:19 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
> Confusion everywhere :-)
>
> On Fri, Aug 9, 2013 at 10:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> 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:

So if we didn't store a different path, it would work. So instead of
expanding '~' ourselves, it would be better to don't expand anything,
and leave it as it is, but how to detect that in fix_path()?

-- 
Felipe Contreras

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

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> 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:
>
> So if we didn't store a different path, it would work. So instead of
> expanding '~' ourselves, it would be better to don't expand anything,
> and leave it as it is, but how to detect that in fix_path()?

I think that the patch relies on that os.path.expanduser(), if
url.path is such a path that begins with "~" (or "~whom"), returns
an absolute path.  When given an absolute path, or "~whom/path",
fix_path returns without running 'git config' on remote.<alias>.url
configuration.

Presumably this "git config" is to "fix" what is already there, and
in the case where the path is already absolute
(e.g. "/home/ap/hgrepo" as opposed to "~ap/hgrepo") the resulting
repository has a correct value for the variable set already without
the need to fix it (that is why the original code just returns from
the function), so doing the same for "~whom" case with this patch
should leave the setting, which presumably is "hg::~ap/hgrepo"?

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

[Felipe Contreras]

On Fri, Aug 9, 2013 at 5:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> 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:
>>
>> So if we didn't store a different path, it would work. So instead of
>> expanding '~' ourselves, it would be better to don't expand anything,
>> and leave it as it is, but how to detect that in fix_path()?
>
> I think that the patch relies on that os.path.expanduser(), if
> url.path is such a path that begins with "~" (or "~whom"), returns
> an absolute path.  When given an absolute path, or "~whom/path",
> fix_path returns without running 'git config' on remote.<alias>.url
> configuration.

I think ~whom/path would run 'git config'.

-- 
Felipe Contreras

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

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Fri, Aug 9, 2013 at 5:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>>> 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:
>>>
>>> So if we didn't store a different path, it would work. So instead of
>>> expanding '~' ourselves, it would be better to don't expand anything,
>>> and leave it as it is, but how to detect that in fix_path()?
>>
>> I think that the patch relies on that os.path.expanduser(), if
>> url.path is such a path that begins with "~" (or "~whom"), returns
>> an absolute path.  When given an absolute path, or "~whom/path",
>> fix_path returns without running 'git config' on remote.<alias>.url
>> configuration.
>
> I think ~whom/path would run 'git config'.

Hmph, do you mean the third example of this?

        $ python
        >>> import os
        >>> os.path.expanduser("~/repo")
        '/home/junio/repo'
        >>> os.path.expanduser("~junio/repo")
        '/home/junio/repo'
        >>> os.path.expanduser("~felipe/repo")
        '~felipe/repo'

which will give "~felipe/repo" that is _not_ an absolute repository
because no such user exists on this box?

It is true that in that case fix_path() will not return early and
will throw a bogus path at "git config", but if the "~whom" does not
resolve to an existing home directory of a user, I am not sure what
we can do better than what Antoine's patch does.

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

[Felipe Contreras]

On Fri, Aug 9, 2013 at 6:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Fri, Aug 9, 2013 at 5:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>>> 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:
>>>>
>>>> So if we didn't store a different path, it would work. So instead of
>>>> expanding '~' ourselves, it would be better to don't expand anything,
>>>> and leave it as it is, but how to detect that in fix_path()?
>>>
>>> I think that the patch relies on that os.path.expanduser(), if
>>> url.path is such a path that begins with "~" (or "~whom"), returns
>>> an absolute path.  When given an absolute path, or "~whom/path",
>>> fix_path returns without running 'git config' on remote.<alias>.url
>>> configuration.
>>
>> I think ~whom/path would run 'git config'.
>
> Hmph, do you mean the third example of this?
>
>         $ python
>         >>> import os
>         >>> os.path.expanduser("~/repo")
>         '/home/junio/repo'
>         >>> os.path.expanduser("~junio/repo")
>         '/home/junio/repo'
>         >>> os.path.expanduser("~felipe/repo")
>         '~felipe/repo'
>
> which will give "~felipe/repo" that is _not_ an absolute repository
> because no such user exists on this box?
>
> It is true that in that case fix_path() will not return early and
> will throw a bogus path at "git config", but if the "~whom" does not
> resolve to an existing home directory of a user, I am not sure what
> we can do better than what Antoine's patch does.

I was thinking something like this:

if url.scheme != 'file' or os.path.isabs(url.path) or url.path[0] == '~':
  return

-- 
Felipe Contreras

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

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> Hmph, do you mean the third example of this?
>>
>>         $ python
>>         >>> import os
>>         >>> os.path.expanduser("~/repo")
>>         '/home/junio/repo'
>>         >>> os.path.expanduser("~junio/repo")
>>         '/home/junio/repo'
>>         >>> os.path.expanduser("~felipe/repo")
>>         '~felipe/repo'
>>
>> which will give "~felipe/repo" that is _not_ an absolute repository
>> because no such user exists on this box?
>>
>> It is true that in that case fix_path() will not return early and
>> will throw a bogus path at "git config", but if the "~whom" does not
>> resolve to an existing home directory of a user, I am not sure what
>> we can do better than what Antoine's patch does.
>
> I was thinking something like this:
>
> if url.scheme != 'file' or os.path.isabs(url.path) or url.path[0] == '~':
>   return

That did cross my mind.

I know ~/ and ~who/ are expanded on UNIXy systems, and I read in
Python documentation that Python on Windows treats ~/ and ~who/ the
same way as on UNIXy systems, so the "begins with ~" test would work
on both systems.  But it is probably a better design to outsource
that knowledge to os.path.expanduser(), with the emphasis on "os."
part of that function.  That way, we do not even have to care about
such potential platform specifics, which is a big plus.  The only
possible difference that approach makes is the above example of
naming a non-existent ~user, but that will not work anyway, so...

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

[Felipe Contreras]

On Sat, Aug 10, 2013 at 12:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> Hmph, do you mean the third example of this?
>>>
>>>         $ python
>>>         >>> import os
>>>         >>> os.path.expanduser("~/repo")
>>>         '/home/junio/repo'
>>>         >>> os.path.expanduser("~junio/repo")
>>>         '/home/junio/repo'
>>>         >>> os.path.expanduser("~felipe/repo")
>>>         '~felipe/repo'
>>>
>>> which will give "~felipe/repo" that is _not_ an absolute repository
>>> because no such user exists on this box?
>>>
>>> It is true that in that case fix_path() will not return early and
>>> will throw a bogus path at "git config", but if the "~whom" does not
>>> resolve to an existing home directory of a user, I am not sure what
>>> we can do better than what Antoine's patch does.
>>
>> I was thinking something like this:
>>
>> if url.scheme != 'file' or os.path.isabs(url.path) or url.path[0] == '~':
>>   return
>
> That did cross my mind.
>
> I know ~/ and ~who/ are expanded on UNIXy systems, and I read in
> Python documentation that Python on Windows treats ~/ and ~who/ the
> same way as on UNIXy systems, so the "begins with ~" test would work
> on both systems.  But it is probably a better design to outsource
> that knowledge to os.path.expanduser(), with the emphasis on "os."
> part of that function.  That way, we do not even have to care about
> such potential platform specifics, which is a big plus.  The only
> possible difference that approach makes is the above example of
> naming a non-existent ~user, but that will not work anyway, so...

We would be doing better than os.path.expanduser(), because if
Mercurial somehow decided to treat ~ differently, our code would still
work just fine. If we do os.path.expanduser(), then we are not
outsourcing anything, we are taking ownership and fixing the path by
ourselves and dealing with all the consequences.

If I clone ~/git, and then change my username, and move my home
directory, doing a 'git fetch' in ~/git wouldn't work anymore, because
we have expanded the path and fixed it to my old home, if instead we
simply return without fixing, it would still work just fine.

-- 
Felipe Contreras

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

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> If I clone ~/git, and then change my username, and move my home
> directory, doing a 'git fetch' in ~/git wouldn't work anymore, because
> we have expanded the path and fixed it to my old home, if instead we
> simply return without fixing, it would still work just fine.

Antoine's patch runs expanduser() only to see if the given one gets
modified to absolute path, and makes fix_path() return without
calling the extra 'git config', so it is my understanding that the
above describes exactly what the patch does.  Am I reading the patch
incorrectly?

It outsources the determination of "is this a special notation to
name a home directory?" logic to .isabs(.expanduser()) instead of
doing a .beginswith('~') ourselves.

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

[Felipe Contreras]

On Sat, Aug 10, 2013 at 1:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> If I clone ~/git, and then change my username, and move my home
>> directory, doing a 'git fetch' in ~/git wouldn't work anymore, because
>> we have expanded the path and fixed it to my old home, if instead we
>> simply return without fixing, it would still work just fine.
>
> Antoine's patch runs expanduser() only to see if the given one gets
> modified to absolute path, and makes fix_path() return without
> calling the extra 'git config', so it is my understanding that the
> above describes exactly what the patch does.  Am I reading the patch
> incorrectly?

Antoine's *second* patch, which I missed, does that, yeah. That should
work fine.

-- 
Felipe Contreras

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

[Antoine Pelisse]

On Sat, Aug 10, 2013 at 9:07 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sat, Aug 10, 2013 at 1:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> If I clone ~/git, and then change my username, and move my home
>>> directory, doing a 'git fetch' in ~/git wouldn't work anymore, because
>>> we have expanded the path and fixed it to my old home, if instead we
>>> simply return without fixing, it would still work just fine.
>>
>> Antoine's patch runs expanduser() only to see if the given one gets
>> modified to absolute path, and makes fix_path() return without
>> calling the extra 'git config', so it is my understanding that the
>> above describes exactly what the patch does.  Am I reading the patch
>> incorrectly?
>
> Antoine's *second* patch, which I missed, does that, yeah. That should
> work fine.

OK Cool,
Thank you both,