| 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
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 >
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
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
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.
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", ...}
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.
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