Patchwork [01,of,12] localrepo: add an unused dictionary for arbitrary labels

login
register
mail settings
Submitter Sean Farley
Date Aug. 18, 2014, 9:17 p.m.
Message ID <ed72c4bcbbbcab7f75f6.1408396677@1.0.0.127.in-addr.arpa>
Download mbox | patch
Permalink /patch/5459/
State Changes Requested
Headers show

Comments

Sean Farley - Aug. 18, 2014, 9:17 p.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@gmail.com>
# Date 1396216512 18000
#      Sun Mar 30 16:55:12 2014 -0500
# Node ID ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
# Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
localrepo: add an unused dictionary for arbitrary labels
Pierre-Yves David - Aug. 21, 2014, 7:42 a.m.
On 08/18/2014 02:17 PM, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1396216512 18000
> #      Sun Mar 30 16:55:12 2014 -0500
> # Node ID ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
> # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
> localrepo: add an unused dictionary for arbitrary labels
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -301,10 +301,13 @@ class localrepository(object):
>           # - new obsolescence marker,
>           # - working directory parent change,
>           # - bookmark changes
>           self.filteredrevcache = {}
>
> +        # dictionary of arbitrary labels, e.g. _labels["foo"]["bar] = node
> +        self._labels = {}
> +

You docstring is confusing. do you mean ?

eg. _labels["labeltype"]["labelname"] = node

>       def close(self):
>           pass
>
>       def _restrictcapabilities(self, caps):
>           # bundle2 is not ready for prime time, drop it unless explicitly
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Sean Farley - Aug. 21, 2014, 6:02 p.m.
Pierre-Yves David writes:

> On 08/18/2014 02:17 PM, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@gmail.com>
>> # Date 1396216512 18000
>> #      Sun Mar 30 16:55:12 2014 -0500
>> # Node ID ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
>> # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
>> localrepo: add an unused dictionary for arbitrary labels
>>
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -301,10 +301,13 @@ class localrepository(object):
>>           # - new obsolescence marker,
>>           # - working directory parent change,
>>           # - bookmark changes
>>           self.filteredrevcache = {}
>>
>> +        # dictionary of arbitrary labels, e.g. _labels["foo"]["bar] = node
>> +        self._labels = {}
>> +
>
> You docstring is confusing. do you mean ?
>
> eg. _labels["labeltype"]["labelname"] = node

Man, I really thought I fixed this already. Yes, I mean:

_labels["namespace"]["name"] = node

A concrete example:

_labels["tags"]["v1.2.3"] = node
Gregory Szorc - Aug. 21, 2014, 6:29 p.m.
On 8/18/14 2:17 PM, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1396216512 18000
> #      Sun Mar 30 16:55:12 2014 -0500
> # Node ID ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
> # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
> localrepo: add an unused dictionary for arbitrary labels

I wanted to say how much I enjoy what you are trying to do here. This 
approach should drastically lower the barrier to robust implementations 
of things like remote refs.

I recently implemented an extension for Mozilla (firefoxtree) that 
implements a light version of remote refs. Essentially, when you pull 
from a known Firefox repository (identified by URL), it creates a local 
tag with the name of that repo.

I had to jump through hoops [1] to implement this properly. The tags 
APIs are far from robust and writing local tags would result in multiple 
lines in the localtags file for a given tag. I had to hack around this. 
It is very ugly.

I like the approach in this patch series of using a two-level dict to 
identify "labels". My extension could easily invent a "firefox" 
namespace and put its node mappings there. The separation into multiple 
namespaces makes the filtering problem go away and makes extension code 
easier to write.

This patch series seems to start solving some problems I have with the 
existing tags/bookmarks implementations.

[1] 
https://hg.mozilla.org/hgcustom/version-control-tools/file/6bc1ab4e0cff/hgext/firefoxtree/__init__.py#l225
Sean Farley - Aug. 21, 2014, 6:50 p.m.
Gregory Szorc writes:

> On 8/18/14 2:17 PM, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@gmail.com>
>> # Date 1396216512 18000
>> #      Sun Mar 30 16:55:12 2014 -0500
>> # Node ID ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
>> # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
>> localrepo: add an unused dictionary for arbitrary labels
>
> I wanted to say how much I enjoy what you are trying to do here. This 
> approach should drastically lower the barrier to robust implementations 
> of things like remote refs.
>
> I recently implemented an extension for Mozilla (firefoxtree) that 
> implements a light version of remote refs. Essentially, when you pull 
> from a known Firefox repository (identified by URL), it creates a local 
> tag with the name of that repo.
>
> I had to jump through hoops [1] to implement this properly. The tags 
> APIs are far from robust and writing local tags would result in multiple 
> lines in the localtags file for a given tag. I had to hack around this. 
> It is very ugly.
>
> I like the approach in this patch series of using a two-level dict to 
> identify "labels". My extension could easily invent a "firefox" 
> namespace and put its node mappings there. The separation into multiple 
> namespaces makes the filtering problem go away and makes extension code 
> easier to write.
>
> This patch series seems to start solving some problems I have with the 
> existing tags/bookmarks implementations.

Thanks! I also want (obviously) this kind of functionality in core as a
first-class citizen. Ideally, before / during the sprint so that I can
push my changes that implement remote bookmarks.
Matt Mackall - Aug. 27, 2014, 11:17 a.m.
On Mon, 2014-08-18 at 16:17 -0500, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1396216512 18000
> #      Sun Mar 30 16:55:12 2014 -0500
> # Node ID ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
> # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
> localrepo: add an unused dictionary for arbitrary labels

I'm not terribly excited about this series as it's a bit too much
restructuring for a minor feature. I think we should just add a little
helper function that tells you all the labels on a ctx and their type.

repo['.'].labels() -> { "foo": "bookmark", "bar": "tag", ...}
Gregory Szorc - Aug. 27, 2014, 2:43 p.m.
On 8/27/2014 4:17 AM, Matt Mackall wrote:
> On Mon, 2014-08-18 at 16:17 -0500, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean.michael.farley@gmail.com>
>> # Date 1396216512 18000
>> #      Sun Mar 30 16:55:12 2014 -0500
>> # Node ID ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
>> # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
>> localrepo: add an unused dictionary for arbitrary labels
> 
> I'm not terribly excited about this series as it's a bit too much
> restructuring for a minor feature. I think we should just add a little
> helper function that tells you all the labels on a ctx and their type.
> 
> repo['.'].labels() -> { "foo": "bookmark", "bar": "tag", ...}

This is somewhat restricting for more advanced use cases, where multiple
namespaces may define the same label. For example, you may wish to
implement remote refs as a namespace per remote.

I've implemented an extension for Mozilla where its labels may conflict
with e.g. local bookmarks.

I'd really like to see the namespaces approach so backends can keep
labels separate. I think this will breed more interesting extensions
that provide alternative development workflows.
Pierre-Yves David - Aug. 27, 2014, 8:58 p.m.
On 08/27/2014 04:43 PM, Gregory Szorc wrote:
> On 8/27/2014 4:17 AM, Matt Mackall wrote:
>> On Mon, 2014-08-18 at 16:17 -0500, Sean Farley wrote:
>>> # HG changeset patch
>>> # User Sean Farley <sean.michael.farley@gmail.com>
>>> # Date 1396216512 18000
>>> #      Sun Mar 30 16:55:12 2014 -0500
>>> # Node ID ed72c4bcbbbcab7f75f6b86fee6c67aa45590e7c
>>> # Parent  8dda6f6ff564d8fe6ac7b8ce4c74eb9bfb5de14a
>>> localrepo: add an unused dictionary for arbitrary labels
>>
>> I'm not terribly excited about this series as it's a bit too much
>> restructuring for a minor feature. I think we should just add a little
>> helper function that tells you all the labels on a ctx and their type.
>>
>> repo['.'].labels() -> { "foo": "bookmark", "bar": "tag", ...}
>
> This is somewhat restricting for more advanced use cases, where multiple
> namespaces may define the same label. For example, you may wish to
> implement remote refs as a namespace per remote.
>
> I've implemented an extension for Mozilla where its labels may conflict
> with e.g. local bookmarks.
>
> I'd really like to see the namespaces approach so backends can keep
> labels separate. I think this will breed more interesting extensions
> that provide alternative development workflows.

I agree with Greg and Sean that is an area where it worth having and 
explicit, clean and extensible solution. The proposed implementation may 
not be the final says but we should improve that aspect of Mercurial.

This sounds like a good sprint topic.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -301,10 +301,13 @@  class localrepository(object):
         # - new obsolescence marker,
         # - working directory parent change,
         # - bookmark changes
         self.filteredrevcache = {}
 
+        # dictionary of arbitrary labels, e.g. _labels["foo"]["bar] = node
+        self._labels = {}
+
     def close(self):
         pass
 
     def _restrictcapabilities(self, caps):
         # bundle2 is not ready for prime time, drop it unless explicitly