[PATCH STABLE] subrepo: handle 'C:' style paths on the command line (issue5770)

[Matt Harbison]

On Mon, 22 Jan 2018 07:38:39 -0500, Yuya Nishihara <yuya at tcha.org> wrote:

> On Sun, 21 Jan 2018 15:50:33 -0500, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison at yahoo.com>
>> # Date 1516560845 18000
>> #      Sun Jan 21 13:54:05 2018 -0500
>> # Node ID a79c2c7aeffeee2f4681f5410864ef688e688b36
>> # Parent  1dd5724664ab53e0cf2a69809bb3f4158d4e30d7
>> subrepo: handle 'C:' style paths on the command line (issue5770)
>
> Queued, many thanks.
>
>> Rather than fixing every command call site, just convert it in the  
>> common
>> subrepo location.  I don't see any sanitizing on the path config  
>> options, so I
>> fixed those too.  Note that while the behavior is fixed here, there are  
>> still
>> places where 'comparing with C:' gets printed out, and that's not great  
>> for
>> debugging purposes.  (Specifically I saw it in `hg incoming -B C:`,  
>> without
>> subrepos.)  While clone will write out an absolute default path, I  
>> wonder what
>> would happen if a user edited that path to be 'C:'.  (I don't think  
>> supporting
>> relative paths in .hgrc is a sane thing to do, but while we're poking  
>> holes in
>> things...)
>>
>> Since this is such an oddball case, it still leaks through in places,  
>> and there
>> seems to be a lot of duplicate url parsing, maybe the url parsing  
>> should be
>> moved to dispatch, and provide the command with a url object?  Then we  
>> could
>> convert this to an absolute path once, and not have to worry about it  
>> in the
>> rest of the code.
>
> It wouldn't be easy to determine which argument is a URL at dispatcher  
> level. Perhaps we can instead add a utility function.

I was afraid of that.  But each of the related commands.py functions have  
an explicit source/dest parameter, so I figured there had to be some magic  
in dispatch to line this up.

The good news is explicit file paths work without help:

   D:\test>hg cat -R C: C:.hgeol
   [...]
   D:\test>hg cat --cwd C: C:.hgeol
   [...]

> FYI, there's another weird bug
> you might be interested. :)
>
>   $ mkdir '$foo'
>   $ cd '$foo'
>   $ hg init
>   $ foo=bar hg root
>   abort: repository $TESTTMP/$foo not found!

Yikes.  I'm assuming the expandpath() is in there to handle `hg init  
~\myrepo` on Windows.  Moving it out would avoid breaking existing repos,  
but how do we avoid the BC when given on the command line?

> https://bz.mercurial-scm.org/show_bug.cgi?id=5739
>
>> +        path = None
>>          if util.safehasattr(repo, '_subtoppath'):
>> -            return repo._subtoppath
>> -        if push and repo.ui.config('paths', 'default-push'):
>> -            return repo.ui.config('paths', 'default-push')
>> -        if repo.ui.config('paths', 'default'):
>> -            return repo.ui.config('paths', 'default')
>
> I don't fully understand the _subtopath magic, but maybe we could resolve
> the peer subrepo URLs recursively from the peerrepo.url(), not from the
> localrepo attribute.

I don't think there's much magic here other than to recursively climb to  
the top level repo to get the base path, and then as the stack unwinds,  
tack on the current repo's relative path.  These path checks were probably  
added in the order somebody thought of them.  Certainly the shared path  
was a late add.

It's an interesting idea, I'll keep it in mind.