Patchwork [01,of,12,V2] bookmarks: add function to centralize the logic to compare bookmarks

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Sept. 22, 2013, 2:05 p.m.
Message ID <2bb09ac15facc79ae3e9.1379858727@feefifofum>
Download mbox | patch
Permalink /patch/2599/
State Superseded
Headers show

Comments

Katsunori FUJIWARA - Sept. 22, 2013, 2:05 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1379858556 -32400
#      Sun Sep 22 23:02:36 2013 +0900
# Node ID 2bb09ac15facc79ae3e926317d250d4f2bf50bfc
# Parent  ea35caf324bb04cbc9ab5e2328367bc50f558cfb
bookmarks: add function to centralize the logic to compare bookmarks

This patch adds "compare()" function to centralize the logic to
compare bookmarks between two repositories.

This patch also adds utility function "_execactions()" to execute
action corresponded to comparison result for each bookmarks.
Katsunori FUJIWARA - Sept. 25, 2013, 3:56 p.m.
At Tue, 24 Sep 2013 22:46:41 -0500,
Kevin Bullock wrote:
> 
> On 22 Sep 2013, at 9:05 AM, FUJIWARA Katsunori wrote:
> 
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1379858556 -32400
> > #      Sun Sep 22 23:02:36 2013 +0900
> > # Node ID 2bb09ac15facc79ae3e926317d250d4f2bf50bfc
> > # Parent  ea35caf324bb04cbc9ab5e2328367bc50f558cfb
> > bookmarks: add function to centralize the logic to compare bookmarks
> > 
> > This patch adds "compare()" function to centralize the logic to
> > compare bookmarks between two repositories.
> > 
> > This patch also adds utility function "_execactions()" to execute
> > action corresponded to comparison result for each bookmarks.
> > 
> > diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> > --- a/mercurial/bookmarks.py
> > +++ b/mercurial/bookmarks.py
> > @@ -239,6 +239,84 @@
> >     finally:
> >         w.release()
> > 
> > +def compare(localrepo, srcmarks, dstmarks,
> > +            srchex=None, dsthex=None, targets=None):
> > +    '''Compare bookmarks between srcmarks and dstmarks, and create result list
> > +
> > +    Each elements in result list are tuple "(kind, bookmark name,
> > +    changeset ID on source side, changeset ID on destination
> > +    side)". "kind" is one of "addsrc", "adddst", "advsrc", "advdst",
> > +    "diverge", "differ" or "invalid". Each changeset IDs are 40
> > +    hexadecimal digit string or None.
> > +
> > +    Changeset IDs in result tuples may be unknown for localrepo, if
> > +    "kind" is one of "addsrc", "adddst", "differ" or "invalid".
> 
> Seems like it would be better to return a tuple of sets, like we do
> from localrepo.status(). I like the clarity of the dispatch-table
> ("actionmap") you have _execactions() take, but it would be just as
> clear (and _execactions would be unnecessary) if we had a call
> pattern like:
> 
>     addsrc, adddst, advsrc, advdst, diverge, differ, invalid =
>         bookmarks.compare(...)
> 
>     for mark, srcnode, dstnode in addsrc | advsrc | advdst | diverge | differ:
>         ...
>     for mark, srcnode, dstnode in adddst:
>         ...
>     etc.
> 
> We would also avoid making callers know the "magic strings" that are
> returned in these tuples, and callers could only look at the return
> sets they're interested in (cf. patch 4, where updateremote() is
> only interested in the advsrc tuples).


Thank you for your suggestion.

I just followed "merge.manifestmerge()" style, but
"localrepo.status()" style looks better for purpose of this series, as
you described.

I'll post revised ones.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -239,6 +239,84 @@ 
     finally:
         w.release()
 
+def compare(localrepo, srcmarks, dstmarks,
+            srchex=None, dsthex=None, targets=None):
+    '''Compare bookmarks between srcmarks and dstmarks, and create result list
+
+    Each elements in result list are tuple "(kind, bookmark name,
+    changeset ID on source side, changeset ID on destination
+    side)". "kind" is one of "addsrc", "adddst", "advsrc", "advdst",
+    "diverge", "differ" or "invalid". Each changeset IDs are 40
+    hexadecimal digit string or None.
+
+    Changeset IDs in result tuples may be unknown for localrepo, if
+    "kind" is one of "addsrc", "adddst", "differ" or "invalid".
+
+    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)
+    else:
+        srcmarkset = set(srcmarks)
+        dstmarkset = set(dstmarks)
+        bset = (srcmarkset - dstmarkset) | (dstmarkset - srcmarkset)
+        for b in srcmarkset & dstmarkset:
+            if srchex(srcmarks[b]) != dsthex(dstmarks[b]):
+                bset.add(b)
+
+    results = []
+    for b in sorted(bset):
+        if b not in srcmarks:
+            if b in dstmarks:
+                results.append(('adddst', b, None, dsthex(dstmarks[b])))
+            else:
+                results.append(('invalid', b, None, None))
+        elif b not in dstmarks:
+            results.append(('addsrc', b, srchex(srcmarks[b]), None))
+        else:
+            scid = srchex(srcmarks[b])
+            dcid = dsthex(dstmarks[b])
+            if scid in localrepo and dcid in localrepo:
+                sctx = localrepo[scid]
+                dctx = localrepo[dcid]
+                if sctx.rev() < dctx.rev():
+                    if validdest(localrepo, sctx, dctx):
+                        kind = 'advdst'
+                    else:
+                        kind = 'diverge'
+                else:
+                    if validdest(localrepo, dctx, sctx):
+                        kind = 'advsrc'
+                    else:
+                        kind = 'diverge'
+            else:
+                # it is too expensive to examine in detail, in this case
+                kind = 'differ'
+            results.append((kind, b, scid, dcid))
+    return results
+
+def _execactions(results, actionmap, ignore=None):
+    count = 0
+    for kind, b, scid, dcid in results:
+        if ignore and kind in ignore:
+            continue
+        assert kind in actionmap
+        if actionmap[kind](b, scid, dcid):
+            count += 1
+    return count
+
 def updatefromremote(ui, repo, remotemarks, path):
     ui.debug("checking for updated bookmarks\n")
     changed = False