bookmarks: make bookmarks.comparebookmarks accept binary nodes (API)

Binary bookmark format should be used internally. It doesn't make sense to have
optional parameters `srchex` and `dsthex`. This patch removes them. It will
also be useful for `bookmarks` bundle2 part because unnecessary conversions
between hex and bin nodes will be avoided.

mercurial/bookmarks.py

@@ -323,8 +323,7 b' def pushbookmark(repo, key, old, new):'
     finally:
         lockmod.release(tr, l, w)
 
-def comparebookmarks(repo, srcmarks, dstmarks,
-                     srchex=None, dsthex=None, targets=None):
+def comparebookmarks(repo, srcmarks, dstmarks, targets=None):
     '''Compare bookmarks between srcmarks and dstmarks
 
     This returns tuple "(addsrc, adddst, advsrc, advdst, diverge,
@@ -347,19 +346,9 b' def comparebookmarks(repo, srcmarks, dst'
     Changeset IDs of tuples in "addsrc", "adddst", "differ" or
      "invalid" list may be unknown for repo.
 
-    This function expects that "srcmarks" and "dstmarks" return
-    changeset ID in 40 hexadecimal digit string for specified
-    bookmark. If not so (e.g. bmstore "repo._bookmarks" returning
-    binary value), "srchex" or "dsthex" should be specified to convert
-    into such form.
-
     If "targets" is specified, only bookmarks listed in it are
     examined.
     '''
-    if not srchex:
-        srchex = lambda x: x
-    if not dsthex:
-        dsthex = lambda x: x
 
     if targets:
         bset = set(targets)
@@ -381,14 +370,14 b' def comparebookmarks(repo, srcmarks, dst'
     for b in sorted(bset):
         if b not in srcmarks:
             if b in dstmarks:
-                adddst((b, None, dsthex(dstmarks[b])))
+                adddst((b, None, dstmarks[b]))
             else:
                 invalid((b, None, None))
         elif b not in dstmarks:
-            addsrc((b, srchex(srcmarks[b]), None))
+            addsrc((b, srcmarks[b], None))
         else:
-            scid = srchex(srcmarks[b])
-            dcid = dsthex(dstmarks[b])
+            scid = srcmarks[b]
+            dcid = dstmarks[b]
             if scid == dcid:
                 same((b, scid, dcid))
             elif scid in repo and dcid in repo:
@@ -439,11 +428,17 b' def _diverge(ui, b, path, localmarks, re'
 
     return None
 
+def unhexlifybookmarks(marks):
+    binremotemarks = {}
+    for name, node in marks.items():
+        binremotemarks[name] = bin(node)
+    return binremotemarks
+
 def updatefromremote(ui, repo, remotemarks, path, trfunc, explicit=()):
     ui.debug("checking for updated bookmarks\n")
     localmarks = repo._bookmarks
     (addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same
-     ) = comparebookmarks(repo, remotemarks, localmarks, dsthex=hex)
+    ) = comparebookmarks(repo, remotemarks, localmarks)
 
     status = ui.status
     warn = ui.warn
@@ -454,15 +449,15 b' def updatefromremote(ui, repo, remotemar'
     changed = []
     for b, scid, dcid in addsrc:
         if scid in repo: # add remote bookmarks for changes we already have
-            changed.append((b, bin(scid), status,
+            changed.append((b, scid, status,
                             _("adding remote bookmark %s\n") % (b)))
         elif b in explicit:
             explicit.remove(b)
             ui.warn(_("remote bookmark %s points to locally missing %s\n")
-                    % (b, scid[:12]))
+                    % (b, hex(scid)[:12]))
 
     for b, scid, dcid in advsrc:
-        changed.append((b, bin(scid), status,
+        changed.append((b, scid, status,
                         _("updating bookmark %s\n") % (b)))
     # remove normal movement from explicit set
     explicit.difference_update(d[0] for d in changed)
@@ -470,13 +465,12 b' def updatefromremote(ui, repo, remotemar'
     for b, scid, dcid in diverge:
         if b in explicit:
             explicit.discard(b)
-            changed.append((b, bin(scid), status,
+            changed.append((b, scid, status,
                             _("importing bookmark %s\n") % (b)))
         else:
-            snode = bin(scid)
-            db = _diverge(ui, b, path, localmarks, snode)
+            db = _diverge(ui, b, path, localmarks, scid)
             if db:
-                changed.append((db, snode, warn,
+                changed.append((db, scid, warn,
                                 _("divergent bookmark %s stored as %s\n") %
                                 (b, db)))
             else:
@@ -485,13 +479,13 b' def updatefromremote(ui, repo, remotemar'
     for b, scid, dcid in adddst + advdst:
         if b in explicit:
             explicit.discard(b)
-            changed.append((b, bin(scid), status,
+            changed.append((b, scid, status,
                             _("importing bookmark %s\n") % (b)))
     for b, scid, dcid in differ:
         if b in explicit:
             explicit.remove(b)
             ui.warn(_("remote bookmark %s points to locally missing %s\n")
-                    % (b, scid[:12]))
+                    % (b, hex(scid)[:12]))
 
     if changed:
         tr = trfunc()
@@ -505,8 +499,8 b' def incoming(ui, repo, other):'
     '''
     ui.status(_("searching for changed bookmarks\n"))
 
-    r = comparebookmarks(repo, other.listkeys('bookmarks'), repo._bookmarks,
-                         dsthex=hex)
+    remotemarks = unhexlifybookmarks(other.listkeys('bookmarks'))
+    r = comparebookmarks(repo, remotemarks, repo._bookmarks)
     addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same = r
 
     incomings = []
@@ -522,16 +516,16 b' def incoming(ui, repo, other):'
             incomings.append("   %-25s %s\n" % (b, getid(id)))
     for b, scid, dcid in addsrc:
         # i18n: "added" refers to a bookmark
-        add(b, scid, _('added'))
+        add(b, hex(scid), _('added'))
     for b, scid, dcid in advsrc:
         # i18n: "advanced" refers to a bookmark
-        add(b, scid, _('advanced'))
+        add(b, hex(scid), _('advanced'))
     for b, scid, dcid in diverge:
         # i18n: "diverged" refers to a bookmark
-        add(b, scid, _('diverged'))
+        add(b, hex(scid), _('diverged'))
     for b, scid, dcid in differ:
         # i18n: "changed" refers to a bookmark
-        add(b, scid, _('changed'))
+        add(b, hex(scid), _('changed'))
 
     if not incomings:
         ui.status(_("no changed bookmarks found\n"))
@@ -547,8 +541,8 b' def outgoing(ui, repo, other):'
     '''
     ui.status(_("searching for changed bookmarks\n"))
 
-    r = comparebookmarks(repo, repo._bookmarks, other.listkeys('bookmarks'),
-                         srchex=hex)
+    remotemarks = unhexlifybookmarks(other.listkeys('bookmarks'))
+    r = comparebookmarks(repo, repo._bookmarks, remotemarks)
     addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same = r
 
     outgoings = []
@@ -564,19 +558,19 b' def outgoing(ui, repo, other):'
             outgoings.append("   %-25s %s\n" % (b, getid(id)))
     for b, scid, dcid in addsrc:
         # i18n: "added refers to a bookmark
-        add(b, scid, _('added'))
+        add(b, hex(scid), _('added'))
     for b, scid, dcid in adddst:
         # i18n: "deleted" refers to a bookmark
         add(b, ' ' * 40, _('deleted'))
     for b, scid, dcid in advsrc:
         # i18n: "advanced" refers to a bookmark
-        add(b, scid, _('advanced'))
+        add(b, hex(scid), _('advanced'))
     for b, scid, dcid in diverge:
         # i18n: "diverged" refers to a bookmark
-        add(b, scid, _('diverged'))
+        add(b, hex(scid), _('diverged'))
     for b, scid, dcid in differ:
         # i18n: "changed" refers to a bookmark
-        add(b, scid, _('changed'))
+        add(b, hex(scid), _('changed'))
 
     if not outgoings:
         ui.status(_("no changed bookmarks found\n"))
@@ -592,8 +586,8 b' def summary(repo, other):'
 
     This returns "(# of incoming, # of outgoing)" tuple.
     '''
-    r = comparebookmarks(repo, other.listkeys('bookmarks'), repo._bookmarks,
-                         dsthex=hex)
+    remotemarks = unhexlifybookmarks(other.listkeys('bookmarks'))
+    r = comparebookmarks(repo, remotemarks, repo._bookmarks)
     addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same = r
     return (len(addsrc), len(adddst))
 

mercurial/exchange.py

@@ -603,9 +603,21 b' def _pushdiscoverybookmarks(pushop):'
     explicit = set([repo._bookmarks.expandname(bookmark)
                     for bookmark in pushop.bookmarks])
 
-    comp = bookmod.comparebookmarks(repo, repo._bookmarks,
-                                    remotebookmark, srchex=hex)
+    remotebookmark = bookmod.unhexlifybookmarks(remotebookmark)
+    comp = bookmod.comparebookmarks(repo, repo._bookmarks, remotebookmark)
+
+    def safehex(x):
+        if x is None:
+            return x
+        return hex(x)
+
+    def hexifycompbookmarks(bookmarks):
+        for b, scid, dcid in bookmarks:
+            yield b, safehex(scid), safehex(dcid)
+
+    comp = [hexifycompbookmarks(marks) for marks in comp]
     addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same = comp
+
     for b, scid, dcid in advsrc:
         if b in explicit:
             explicit.remove(b)
@@ -617,7 +629,7 b' def _pushdiscoverybookmarks(pushop):'
             explicit.remove(b)
             pushop.outbookmarks.append((b, '', scid))
     # search for overwritten bookmark
-    for b, scid, dcid in advdst + diverge + differ:
+    for b, scid, dcid in list(advdst) + list(diverge) + list(differ):
         if b in explicit:
             explicit.remove(b)
             pushop.outbookmarks.append((b, dcid, scid))
@@ -1456,6 +1468,7 b' def _pullbookmarks(pullop):'
     pullop.stepsdone.add('bookmarks')
     repo = pullop.repo
     remotebookmarks = pullop.remotebookmarks
+    remotebookmarks = bookmod.unhexlifybookmarks(remotebookmarks)
     bookmod.updatefromremote(repo.ui, repo, remotebookmarks,
                              pullop.remote.url(),
                              pullop.gettransaction,