cleanupnodes: trust caller when "moves" is not None
authorMartin von Zweigbergk <martinvonz@google.com>
Wed, 21 Mar 2018 16:46:28 -0700
changeset 53620 b7823bd59b078bfdf7ce9c208a4f9e3d470fa5c0
parent 53619 236af7cfa4c3dfe358ec4f0f4f1c9590298a200c
child 53621 c8eb8d1fc6cfc8ca60d9b48904e5d26bc39ba00d
push id1079
push usergszorc@mozilla.com
push dateMon, 10 Dec 2018 19:44:59 +0000
cleanupnodes: trust caller when "moves" is not None If "moves" (indicating how to move bookmarks) is None, we fill it out based on "replacements" (indicating which obsmarkers to add). If "moves" is not None, we would still add items based on "replacements". This makes it impossible to pass "moves={}" and not move bookmarks, which surprised me. The only caller that currently passes a value for "moves" was the rebase extension and there we were already adding bookmark moves corresponding to obsmarker additions, so it should not be impacted. Differential Revision: https://phab.mercurial-scm.org/D5391
mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -896,42 +896,43 @@ def cleanupnodes(repo, replacements, ope
         # upgrading non tuple "source" to tuple ones for BC
         repls = {}
         for key, value in replacements.items():
             if not isinstance(key, tuple):
                 key = (key,)
             repls[key] = value
         replacements = repls
 
+    # Unfiltered repo is needed since nodes in replacements might be hidden.
+    unfi = repo.unfiltered()
+
     # Calculate bookmark movements
     if moves is None:
         moves = {}
-    # Unfiltered repo is needed since nodes in replacements might be hidden.
-    unfi = repo.unfiltered()
-    for oldnodes, newnodes in replacements.items():
-        for oldnode in oldnodes:
-            if oldnode in moves:
-                continue
-            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
-                allreplaced = []
-                for rep in replacements:
-                    allreplaced.extend(rep)
-                roots = list(unfi.set('max((::%n) - %ln)', oldnode,
-                                      allreplaced))
-                if roots:
-                    newnode = roots[0].node()
+        for oldnodes, newnodes in replacements.items():
+            for oldnode in oldnodes:
+                if oldnode in moves:
+                    continue
+                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
+                    allreplaced = []
+                    for rep in replacements:
+                        allreplaced.extend(rep)
+                    roots = list(unfi.set('max((::%n) - %ln)', oldnode,
+                                          allreplaced))
+                    if roots:
+                        newnode = roots[0].node()
+                    else:
+                        newnode = nullid
                 else:
-                    newnode = nullid
-            else:
-                newnode = newnodes[0]
-            moves[oldnode] = newnode
+                    newnode = newnodes[0]
+                moves[oldnode] = newnode
 
     allnewnodes = [n for ns in replacements.values() for n in ns]
     toretract = {}
     toadvance = {}
     if fixphase:
         precursors = {}
         for oldnodes, newnodes in replacements.items():
             for oldnode in oldnodes: