| 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
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