Patchwork D2807: remotenames: add functionality to hoist remotebookmarks

login
register
mail settings
Submitter phabricator
Date March 11, 2018, 1:16 p.m.
Message ID <differential-rev-PHID-DREV-ycnz53fwauthr46hbxzn-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/29305/
State Superseded
Headers show

Comments

phabricator - March 11, 2018, 1:16 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This patch adds the functionality to hoist remotebookmarks to the top level
  namespace. The peer of which bookmarks should be hoisted can be set using
  `remotenames.hoist` config option. Only bookmarks can be hoisted. If a hoisted
  name and local bookmark exists of the same name, the local bookmark takes
  precedence.
  
  While I was here, I documented the default values of two other remotenames
  config options.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2807

AFFECTED FILES
  hgext/remotenames.py
  tests/test-logexchange.t

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - March 12, 2018, 9:39 p.m.
durin42 accepted this revision as: durin42.
durin42 added a comment.


  Seems fine, I'd like to hear what others think?
  
  I'm not sure I'm crazy about "hoistedname" I guess.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2807

To: pulkit, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
phabricator - March 20, 2018, 1:20 p.m.
pulkit added a comment.


  A gentle reminder to get this series reviewed.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2807

To: pulkit, #hg-reviewers, durin42
Cc: durin42, mercurial-devel
phabricator - March 22, 2018, 12:10 a.m.
indygreg added a comment.


  I like the functionality. I'm not crazy about "hoist." I think if the config option were named `hoistedpeer`, that would be a bit better. If it is just `hoist`, it sounds like it should be a boolean.
  
  `hoistedname` appearing in `hg log` output doesn't feel right. It seems to want a multiple word label. If we kept "hoist," I suppose that would be `hoisted name`. But I'm not crazy about the naming. But I can't think of better terminology right now. I want to incorporate "primary" or "main" somehow. But that terminology doesn't seem to work in the context of an `hg log` label.
  
  With those minor fixups, I'll queue this. I think we can bikeshed the naming later. We can always alias the old config option if we rename things.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2807

To: pulkit, #hg-reviewers, durin42
Cc: indygreg, durin42, mercurial-devel
phabricator - March 23, 2018, 6:41 p.m.
indygreg accepted this revision.
indygreg added a comment.
This revision is now accepted and ready to land.


  I still don't have a good suggestion for a better name. We can always change terminology later.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2807

To: pulkit, #hg-reviewers, durin42, indygreg
Cc: indygreg, durin42, mercurial-devel

Patch

diff --git a/tests/test-logexchange.t b/tests/test-logexchange.t
--- a/tests/test-logexchange.t
+++ b/tests/test-logexchange.t
@@ -145,6 +145,7 @@ 
   | |  bookmark:    bar
   | |  remote bookmark:  $TESTTMP/server2/bar
   | |  remote bookmark:  default/bar
+  | |  hoistedname:  bar
   | |  user:        test
   | |  date:        Thu Jan 01 00:00:00 1970 +0000
   | |  summary:     Added g
@@ -163,6 +164,7 @@ 
   |  bookmark:    foo
   |  remote bookmark:  $TESTTMP/server2/foo
   |  remote bookmark:  default/foo
+  |  hoistedname:  foo
   |  user:        test
   |  date:        Thu Jan 01 00:00:00 1970 +0000
   |  summary:     Added d
@@ -226,6 +228,28 @@ 
   |
   o  0:18d04c59bb5d [] ()
   
+The `hoistednames` template keyword
+
+  $ hg log -GT "{rev}:{node|short} ({hoistednames})"
+  @  8:3e1487808078 ()
+  |
+  | o  7:ec2426147f0e ()
+  | |
+  | o  6:87d6d6676308 (bar)
+  | |
+  | o  5:825660c69f0c ()
+  |/
+  o  4:aa98ab95a928 ()
+  |
+  o  3:62615734edd5 (foo)
+  |
+  o  2:28ad74487de9 ()
+  |
+  o  1:29becc82797a ()
+  |
+  o  0:18d04c59bb5d ()
+  
+
 Testing the revsets provided by remotenames extension
 
 `remotenames` revset
@@ -259,3 +283,47 @@ 
   o  3:62615734edd5 $TESTTMP/server2/foo default/foo
   |
   ~
+
+Updating to revision using hoisted name
+
+Deleting local bookmark to make sure we update to hoisted name only
+
+  $ hg bookmark -d bar
+
+  $ hg up bar
+  2 files updated, 0 files merged, 1 files removed, 0 files unresolved
+
+  $ hg log -r .
+  changeset:   6:87d6d6676308
+  remote bookmark:  $TESTTMP/server2/bar
+  remote bookmark:  default/bar
+  hoistedname:  bar
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     Added g
+  
+When both local bookmark and hoisted name exists but on different revs
+
+  $ hg up 8
+  1 files updated, 0 files merged, 2 files removed, 0 files unresolved
+
+  $ hg bookmark foo
+  moving bookmark 'foo' forward from 62615734edd5
+
+Local bookmark should take precedence over hoisted name
+
+  $ hg up foo
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+
+  $ hg log -r .
+  changeset:   8:3e1487808078
+  branch:      wat
+  bookmark:    foo
+  tag:         tip
+  remote branch:  $TESTTMP/server2/wat
+  remote branch:  default/wat
+  parent:      4:aa98ab95a928
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     added bar
+  
diff --git a/hgext/remotenames.py b/hgext/remotenames.py
--- a/hgext/remotenames.py
+++ b/hgext/remotenames.py
@@ -14,10 +14,14 @@ 
 Config options to tweak the default behaviour:
 
 remotenames.bookmarks
-  Boolean value to enable or disable showing of remotebookmarks
+  Boolean value to enable or disable showing of remotebookmarks (default: True)
 
 remotenames.branches
-  Boolean value to enable or disable showing of remotebranches
+  Boolean value to enable or disable showing of remotebranches (default: True)
+
+remotenames.hoist
+  Name of the peer whose remotebookmarks should be hoisted into the top-level
+  namespace (default: 'default')
 """
 
 from __future__ import absolute_import
@@ -55,6 +59,9 @@ 
 configitem('remotenames', 'branches',
     default=True,
 )
+configitem('remotenames', 'hoist',
+    default='default',
+)
 
 class lazyremotenamedict(collections.MutableMapping):
     """
@@ -167,6 +174,8 @@ 
     def _invalidatecache(self):
         self._nodetobmarks = None
         self._nodetobranch = None
+        self._hoisttonodes = None
+        self._nodetohoists = None
 
     def bmarktonodes(self):
         return self.bookmarks
@@ -191,6 +200,28 @@ 
                     self._nodetobranch.setdefault(node, []).append(name)
         return self._nodetobranch
 
+    def hoisttonodes(self, hoist):
+        if not self._hoisttonodes:
+            marktonodes = self.bmarktonodes()
+            self._hoisttonodes = {}
+            hoist += '/'
+            for name, node in marktonodes.iteritems():
+                if name.startswith(hoist):
+                    name = name[len(hoist):]
+                    self._hoisttonodes[name] = node
+        return self._hoisttonodes
+
+    def nodetohoists(self, hoist):
+        if not self._nodetohoists:
+            marktonodes = self.bmarktonodes()
+            self._nodetohoists = {}
+            hoist += '/'
+            for name, node in marktonodes.iteritems():
+                if name.startswith(hoist):
+                    name = name[len(hoist):]
+                    self._nodetohoists.setdefault(node[0], []).append(name)
+        return self._nodetohoists
+
 def reposetup(ui, repo):
     if not repo.local():
         return
@@ -211,6 +242,22 @@ 
                 repo._remotenames.nodetobmarks().get(node, []))
         repo.names.addnamespace(remotebookmarkns)
 
+        # hoisting only works if there are remote bookmarks
+        hoist = ui.config('remotenames', 'hoist')
+        if hoist:
+            hoistednamens = ns(
+                'hoistednames',
+                templatename='hoistednames',
+                colorname='hoistedname',
+                logfmt='hoistedname:  %s\n',
+                listnames = lambda repo:
+                    repo._remotenames.hoisttonodes(hoist).keys(),
+                namemap = lambda repo, name:
+                    repo._remotenames.hoisttonodes(hoist).get(name, []),
+                nodemap = lambda repo, node:
+                    repo._remotenames.nodetohoists(hoist).get(node, []))
+            repo.names.addnamespace(hoistednamens)
+
     if ui.configbool('remotenames', 'branches'):
         remotebranchns = ns(
             'remotebranches',