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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 10, 2013, 05:18 UTC
Message-ID: <7vioze2kev.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAMP44s1Q2x9uz5Ajr=BgVjSjO88XD5UYzVSEqgMeK5_YAYSa5A@mail.gmail.com>

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

Show 23 quoted lines

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