[PATCH] path handling: don't treat paths with unknown scheme as file paths (issue3715)

[Till Schneidereit]

Sorry for the late reply.

The project for which this patch fixes a backwards-incompatible change
in Mercurial 2.4 is qimportbz
(http://hg.mozilla.org/users/robarnold_cmu.edu/qimportbz). I agree,
however, that the way I tried to fix it might not be the cleanest and
best one and that test-cases are very much required. Unfortunately, I
don't really have the time to get the patch to a better state and have
since changed qimportbz to work around the change instead.

Thanks for the feedback and sorry that I'm not able to work on this any further.


till

On Fri, Nov 30, 2012 at 8:07 PM, Kevin Bullock
<kbullock+mercurial at ringworld.org> wrote:
> On Nov 30, 2012, at 12:33 PM, Mads Kiilerich wrote:
>
>> On 11/29/2012 05:40 PM, Till Schneidereit wrote:
>>> # HG changeset patch
>>> # User Till Schneidereit <tschneidereit at gmail.com>
>>> # Date 1354207103 -3600
>>> # Node ID 3cb67ba27a0795c2fa2728dbcacccd63366723b6
>>> # Parent  83aa4359c49f67bcb98fb9c7d885ed4ac7443239
>>> path handling: don't treat paths with unknown scheme as file paths (issue3715)
>>>
>>> islocal in hg.py returns True for paths with unknown schemes, making it
>>> impossible for scheme handlers registered via the handlerfuncs list in url.py
>>> to handle the URIs.
>>
>> In which cases would it fail? Can you demonstrate the problem ... and perhaps add a test case?
>>
>>> diff --git a/mercurial/hg.py b/mercurial/hg.py
>>> --- a/mercurial/hg.py
>>> +++ b/mercurial/hg.py
>>> @@ -69,17 +69,17 @@ schemes = {
>>>      'https': httppeer,
>>>      'ssh': sshpeer,
>>>      'static-http': statichttprepo,
>>>  }
>>>    def _peerlookup(path):
>>>      u = util.url(path)
>>>      scheme = u.scheme or 'file'
>>> -    thing = schemes.get(scheme) or schemes['file']
>>> +    thing = schemes.get(scheme)
>>>      try:
>>>          return thing(path)
>>>      except TypeError:
>>>          return thing
>>
>> Before _peerlokup used the file scheme for unknown schemes. That probably failed, but with a meaningful error message.
>>
>> With this change _peerlookup will return None for unknown schemes. Can the rest of the code handle that? I guess it will crash in unfortunate ways.
>>
>> Why do you want it to return None?
>
> I agree, we need some solid test cases. I would guess that an abort would be better here, but a survey of the call sites is needed in any case.
>
> pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
> Kevin R. Bullock
>