Mercurial > hg
changeset 33443:e48082e0a8d5
phabricator: verify local tags before trusting them
Previously we trust local tags blindly and that could cause wrong
Differential Revision to be updated, when people switch between Phabricator
instances.
This patch adds verification logic to detect such issue and remove
problematic tags. For example, a tag "D19" was on node "X", the code will
fetch all diffs attached to D19, and check if nodes server-side overlaps
with nodes in precursors. If they do not overlap, create a new Differential
Revision.
Test Plan:
Use a test Phabricator instance, send patches using `hg phabsend`, then
change the local tag manually to a wrong Differential Revision number.
Amend the patch and send again. Make sure the tag gets ignored and deleted.
Differential Revision: https://phab.mercurial-scm.org/D36
author | Jun Wu <quark@fb.com> |
---|---|
date | Tue, 11 Jul 2017 08:17:29 -0700 |
parents | 3ab0d5767b54 |
children | c4e39512a661 |
files | contrib/phabricator.py |
diffstat | 1 files changed, 28 insertions(+), 3 deletions(-) [+] |
line wrap: on
line diff
--- a/contrib/phabricator.py Mon Jul 10 13:50:50 2017 -0700 +++ b/contrib/phabricator.py Tue Jul 11 08:17:29 2017 -0700 @@ -35,6 +35,7 @@ import json import re +from mercurial.node import bin, nullid from mercurial.i18n import _ from mercurial import ( encoding, @@ -158,15 +159,17 @@ nodemap = unfi.changelog.nodemap result = {} # {node: (oldnode or None, drev)} + toconfirm = {} # {node: (oldnode, {precnode}, drev)} for node in nodelist: ctx = unfi[node] - # Check tags like "D123" - for n in obsolete.allprecursors(unfi.obsstore, [node]): + # For tags like "D123", put them into "toconfirm" to verify later + precnodes = list(obsolete.allprecursors(unfi.obsstore, [node])) + for n in precnodes: if n in nodemap: for tag in unfi.nodetags(n): m = _differentialrevisiontagre.match(tag) if m: - result[node] = (n, int(m.group(1))) + toconfirm[node] = (n, set(precnodes), int(m.group(1))) continue # Check commit message @@ -174,6 +177,28 @@ if m: result[node] = (None, int(m.group(1))) + # Double check if tags are genuine by collecting all old nodes from + # Phabricator, and expect precursors overlap with it. + if toconfirm: + confirmed = {} # {drev: {oldnode}} + drevs = [drev for n, precs, drev in toconfirm.values()] + diffs = callconduit(unfi, 'differential.querydiffs', + {'revisionIDs': drevs}) + for diff in diffs.values(): + drev = int(diff[r'revisionID']) + oldnode = bin(encoding.unitolocal(getdiffmeta(diff).get(r'node'))) + if node: + confirmed.setdefault(drev, set()).add(oldnode) + for newnode, (oldnode, precset, drev) in toconfirm.items(): + if bool(precset & confirmed.get(drev, set())): + result[newnode] = (oldnode, drev) + else: + tagname = 'D%d' % drev + tags.tag(repo, tagname, nullid, message=None, user=None, + date=None, local=True) + unfi.ui.warn(_('D%s: local tag removed - does not match ' + 'Differential history\n') % drev) + return result def getdiff(ctx, diffopts):