D748: cleanupnodes: separate out bookmark destination calculation from actual update

[martinvonz (Martin von Zweigbergk)]

martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We will soon want to pass in overrides for bookmark movements and this
  will make that patch simpler. I also think this makes the code easier
  to follow regardless of the later patch.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/scmutil.py

CHANGE DETAILS

diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -591,29 +591,35 @@
     if not util.safehasattr(mapping, 'items'):
         mapping = {n: () for n in mapping}
 
+    # Calculate bookmark movements
+    moves = {}
+    unfi = repo.unfiltered()
+    for oldnode, newnodes in mapping.items():
+        if len(newnodes) > 1:
+            # usually a split, take the one with biggest rev number
+            newnode = next(unfi.set('max(%ln)', newnodes)).node()
+        elif len(newnodes) == 0:
+            # move bookmark backwards
+            roots = list(unfi.set('max((::%n) - %ln)', oldnode,
+                                  list(mapping)))
+            if roots:
+                newnode = roots[0].node()
+            else:
+                newnode = nullid
+        else:
+            newnode = newnodes[0]
+        moves[oldnode] = newnode
+
     with repo.transaction('cleanup') as tr:
         # Move bookmarks
         bmarks = repo._bookmarks
         bmarkchanges = []
         allnewnodes = [n for ns in mapping.values() for n in ns]
-        for oldnode, newnodes in mapping.items():
+        for oldnode, newnode in moves.items():
             oldbmarks = repo.nodebookmarks(oldnode)
             if not oldbmarks:
                 continue
             from . import bookmarks # avoid import cycle
-            if len(newnodes) > 1:
-                # usually a split, take the one with biggest rev number
-                newnode = next(repo.set('max(%ln)', newnodes)).node()
-            elif len(newnodes) == 0:
-                # move bookmark backwards
-                roots = list(repo.set('max((::%n) - %ln)', oldnode,
-                                      list(mapping)))
-                if roots:
-                    newnode = roots[0].node()
-                else:
-                    newnode = nullid
-            else:
-                newnode = newnodes[0]
             repo.ui.debug('moving bookmarks %r from %s to %s\n' %
                           (oldbmarks, hex(oldnode), hex(newnode)))
             # Delete divergent bookmarks being parents of related newnodes



To: martinvonz, #hg-reviewers
Cc: mercurial-devel