[PATCH 05 of 10 V10] bookmarks: make bookmarks.compare accept binary nodes
Gregory Szorc
gregory.szorc at gmail.com
Mon Nov 21 22:15:38 EST 2016
On Sun, Nov 20, 2016 at 4:13 AM, Stanislau Hlebik <stash at fb.com> wrote:
> # HG changeset patch
> # User Stanislau Hlebik <stash at fb.com>
> # Date 1479373181 28800
> # Thu Nov 17 00:59:41 2016 -0800
> # Node ID 57d7f92db34461da87850e26d831d2d235282356
> # Parent bd590f83eb640f4464ba5465f4e10677e348e83c
> bookmarks: make bookmarks.compare accept binary nodes
>
> New `bookmarks` bundle2 part will contain byte nodes, and since bookmarks
> are
> stored in binary format, it doesn't make sense to convert them from binary
> to
> hex and then back to binary again
>
I /think/ this is OK. But I'd like another set of eyes to be sure because
it's a large patch.
The commit message needs to be flagged "(API)" because of the change to
bookmarks.compare(). Also, since you are removing optional arguments, it
feels safer to rename the function to avoid any ambiguity from old callers
[in extensions].
>
> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> --- a/mercurial/bookmarks.py
> +++ b/mercurial/bookmarks.py
> @@ -390,8 +390,7 @@
> finally:
> lockmod.release(tr, l, w)
>
> -def compare(repo, srcmarks, dstmarks,
> - srchex=None, dsthex=None, targets=None):
> +def compare(repo, srcmarks, dstmarks, targets=None):
> '''Compare bookmarks between srcmarks and dstmarks
>
> This returns tuple "(addsrc, adddst, advsrc, advdst, diverge,
> @@ -414,19 +413,9 @@
> 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)
> @@ -448,14 +437,14 @@
> 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:
> @@ -506,11 +495,17 @@
>
> return None
>
> +def hexifybookmarks(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
> - ) = compare(repo, remotemarks, localmarks, dsthex=hex)
> + ) = compare(repo, remotemarks, localmarks)
>
> status = ui.status
> warn = ui.warn
> @@ -521,15 +516,15 @@
> 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)
> @@ -537,13 +532,12 @@
> 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:
> @@ -552,13 +546,13 @@
> 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()
> @@ -572,8 +566,8 @@
> '''
> ui.status(_("searching for changed bookmarks\n"))
>
> - r = compare(repo, other.listkeys('bookmarks'), repo._bookmarks,
> - dsthex=hex)
> + remotemarks = hexifybookmarks(other.listkeys('bookmarks'))
> + r = compare(repo, remotemarks, repo._bookmarks)
> addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same = r
>
> incomings = []
> @@ -589,16 +583,16 @@
> 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"))
> @@ -614,8 +608,8 @@
> '''
> ui.status(_("searching for changed bookmarks\n"))
>
> - r = compare(repo, repo._bookmarks, other.listkeys('bookmarks'),
> - srchex=hex)
> + remotemarks = hexifybookmarks(other.listkeys('bookmarks'))
> + r = compare(repo, repo._bookmarks, remotemarks)
> addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same = r
>
> outgoings = []
> @@ -631,19 +625,19 @@
> 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"))
> @@ -659,8 +653,8 @@
>
> This returns "(# of incoming, # of outgoing)" tuple.
> '''
> - r = compare(repo, other.listkeys('bookmarks'), repo._bookmarks,
> - dsthex=hex)
> + remotemarks = hexifybookmarks(other.listkeys('bookmarks'))
> + r = compare(repo, remotemarks, repo._bookmarks)
> addsrc, adddst, advsrc, advdst, diverge, differ, invalid, same = r
> return (len(addsrc), len(adddst))
>
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -608,8 +608,21 @@
> explicit = set([repo._bookmarks.expandname(bookmark)
> for bookmark in pushop.bookmarks])
>
> - comp = bookmod.compare(repo, repo._bookmarks, remotebookmark,
> srchex=hex)
> + remotebookmark = bookmod.hexifybookmarks(remotebookmark)
> + comp = bookmod.compare(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)
> @@ -621,7 +634,7 @@
> 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))
> @@ -1460,6 +1473,7 @@
> pullop.stepsdone.add('bookmarks')
> repo = pullop.repo
> remotebookmarks = pullop.remotebookmarks
> + remotebookmarks = bookmod.hexifybookmarks(remotebookmarks)
> bookmod.updatefromremote(repo.ui, repo, remotebookmarks,
> pullop.remote.url(),
> pullop.gettransaction,
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20161121/635796c9/attachment.html>
More information about the Mercurial-devel
mailing list