Mercurial Mercurial > mercurial / changeset
summary | shortlog | changelog | graph | tags | bookmarks | branches | files | changeset | raw | help
Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
phabricator: verify local tags before trusting them
authorJun Wu <quark@fb.com>
Tue, 11 Jul 2017 08:17:29 -0700
changeset 33443 e48082e0a8d5
parent 33442 3ab0d5767b54
child 33444 c4e39512a661
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
contrib/phabricator.py file | annotate | diff | comparison | revisions 
--- 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):
mercurial