[PATCH 1 of 2 V3] serve: add support for Mercurial subrepositories

[Matt Harbison]

On Thu, 30 Mar 2017 21:19:01 -0400, Matt Harbison <mharbison72 at gmail.com>  
wrote:

> On Tue, 28 Mar 2017 10:11:15 -0400, Yuya Nishihara <yuya at tcha.org> wrote:
>
>> On Sun, 26 Mar 2017 23:04:30 -0400, Matt Harbison wrote:
>>> # HG changeset patch
>>> # User Matt Harbison <matt_harbison at yahoo.com>
>>> # Date 1488146743 18000
>>> #      Sun Feb 26 17:05:43 2017 -0500
>>> # Node ID 0ff9bef3e0f67422cf29c200fa4a671d861d060b
>>> # Parent  c60091fa1426892552dd6c0dd4b9c49e3c3da045
>>> serve: add support for Mercurial subrepositories
>>
>>> +def addwebdirpath(repo, serverpath, webconf):
>>> +    webconf[serverpath] = repo.root
>>> +    repo.ui.debug('adding %s = %s\n' % (serverpath, repo.root))
>>> +
>>> +    for r in repo.revs('filelog("path:.hgsub")'):
>>> +        ctx = repo[r]
>>> +        for subpath in ctx.substate:
>>> +            ctx.sub(subpath).addwebdirpath(serverpath, webconf)
>>
>> [...]
>>
>>> --- a/mercurial/server.py
>>> +++ b/mercurial/server.py
>>> @@ -15,6 +15,7 @@
>>>
>>>  from . import (
>>>      chgserver,
>>> +    cmdutil,
>>>      commandserver,
>>>      error,
>>>      hgweb,
>>> @@ -130,11 +131,24 @@
>>>          baseui = ui
>>>      webconf = opts.get('web_conf') or opts.get('webdir_conf')
>>>      if webconf:
>>> +        if opts.get('subrepos'):
>>> +            raise error.Abort(_('--web-conf cannot be used with  
>>> --subrepos'))
>>> +
>>>          # load server settings (e.g. web.port) to "copied" ui, which  
>>> allows
>>>          # hgwebdir to reload webconf cleanly
>>>          servui = ui.copy()
>>>          servui.readconfig(webconf, sections=['web'])
>>>          alluis.add(servui)
>>> +    elif opts.get('subrepos'):
>>> +        servui = ui.copy()
>>> +        alluis.add(servui)
>>
>> No need to make a copy of ui since nothing loaded into servui.
>>
>>> +    @annotatesubrepoerror
>>> +    def addwebdirpath(self, serverpath, webconf):
>>> +        # The URL request contains the subrepo source path, not the  
>>> local
>>> +        # subrepo path.  The distinction matters for 'foo = ../foo'  
>>> type
>>> +        # entries.  It isn't possible to serve up 'foo = http://..'  
>>> type
>>> +        # entries, because the server path is relative to this local  
>>> server.
>>> +        src = self._state[0]
>>> +        if util.url(src).islocal():
>>> +            path = util.normpath(serverpath + src)
>>> +            cmdutil.addwebdirpath(self._repo, path + '/', webconf)
>>
>> What happens if src is '../../escape_from_web_root' ?
>>
>> I don't think it's correct to lay out subrepositories by peer URL since  
>> we're
>> exporting a _local_ clone. If you do "hg clone sub1 sub2", you'll see  
>> sub1
>> in local-path layout. "hg serve -S" just allows us to see sub1 over  
>> http.
>
> That was actually how I first coded it, but switched it because it  
> wasn't handling the test in test-subrepo-deep-nested-change.t properly.   
> I forget what exactly the problem was.  I'll take another look this  
> weekend.

I finally got back to this, now that I've got the serve tests working on  
Windows.  The problem I had run into is that in  
test-subrepo-deep-nested-change.t, the repos exist in the filesystem as:

   main
   sub1
   sub2

If you clone `hg clone main` right after the "main import" commit around  
line 74, it builds a tree like:

   cloning subrepo sub1 from $TESTTMP/sub1
   cloning subrepo sub1\sub2 from $TESTTMP/sub2

The fact that you can locally clone the original 'main' strongly suggests  
that we should be able to serve the original main, and be able to clone  
 from it too.  The subsequent test I added there does that.  It took using  
the peer layout to do this.  (Though it really isn't a peer- it's what is  
in the local filesystem where the serve is happening.)

It's easy enough to setup the dictionary such that the repos would be  
served up as:

   /
   /sub1
   /sub1/sub2

But clone seems to want to follow the peer layout specified in .hgsub of  
the parent.  (I can see  "GET /../sub1?cmd=capabilities ..." in  
access.log, which is where clone is dying with a 404.)

I know that escaping '/' is generally a security concern.  But I'm not  
sure that it is here, since the only thing served above '/' is something  
explicitly in the repository (via .hgsub).  So it can't be anything else  
that is potentially sensitive.

I don't care too much about this case.  I'd rather have the functionality  
for the recommended nested subrepo case, than not at all.  But I don't  
want to sneak something in that is broken, with no obvious way to fix if  
somebody complains.

Thoughts?

>> Maybe it's okay to add all repositories found under repo.root to  
>> webconf.
>> For strictness, we could check if a subrepo path exists in .hgsub of any
>> revision.
>
> I'd like to avoid searching through the directory tree, if possible.