[PATCH] shelve: support shared repositories

[Sean Farley]

Ryan McElroy <rm at fb.com> writes:

> On 3/14/2016 11:10 PM, Sean Farley wrote:
>> Oleg Afanasyev <olegaf at fb.com> writes:
>>
>>> # HG changeset patch
>>> # User Oleg Afanasyev <olegaf at fb.com>
>>> # Date 1457972723 25200
>>> #      Mon Mar 14 09:25:23 2016 -0700
>>> # Node ID 6ec353e7cc031f453d26925fcbf15a035aaf2c90
>>> # Parent  1c658391b22fb4d98ccfb60c0e57315b55634117
>>> shelve: support shared repositories
>>>
>>> Shelve will use shared data instead of its own shelves. That would
>>> allow moving changes from share to share using shelve.
>>>
>>> When unshare is performed shelve will not be cloned to new repo. That
>>> would change behaviour from what it is doing now e.g. shelves created
>>> in share will disappear when unshare is done vs now shares have their
>>> own shelves.
>>>
>>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>>> --- a/hgext/shelve.py
>>> +++ b/hgext/shelve.py
>>> @@ -61,6 +61,12 @@
>>>   
>>>   backupdir = 'shelve-backup'
>>>   
>>> +def shareawarevfs(repo):
>>> +    if repo.shared():
>>> +        return scmutil.vfs(repo.sharedpath)
>>> +    else:
>>> +        return repo.vfs
>>> +
>> This is beginning to become a pattern in a few extensions now. I think
>> that this is a good example of something that should go into core so
>> that all extensions can benefit.
>>
>> Is there a reason 'repo.vfs' can't easily return the shared path? When
>> does it make sense for it to return the 'parent' repo path?
>>
>> Off the top of my head, I would expect 'repo.vfs' to return the repo I'm
>> currently in (i.e. the repo.sharedpath).
>
> Ideally this functionality would go into the share machinery in core, 
> but I started some work down that direction a while back, and largefiles 
> breaks. I didn't deep dive at the time.

Yes, we should try to fix that.

> The issue with making everything use the parent repo is 
> dirstate-specific stuff -- think of the sparse extension, for example. 
> So we can't just forward everything.

Agreed, but this something I think is deserving of the work.

> In fact, I wrote up a wiki page a while back outlining some of the 
> bigger possible solutions to this problem: 
> https://www.mercurial-scm.org/wiki/SharedRepository

Ah, cool, thanks for the link.

> That is well outside the scope of this patch, which is much more 
> tactical and aimed at fixing a specific pain point. It does so 
> effectively with minimal outside impact, so I think we should be okay 
> with this approach.

I think we should split the patch into the first common function and
discuss adding that to scmutil (or vfs or something similar) so that we
don't have to copy code. Other than that, yes, it seems fine.