--- a/contrib/phabricator.py Thu Aug 03 03:09:33 2017 +0530
+++ b/contrib/phabricator.py Mon Jul 17 19:52:50 2017 -0700
@@ -138,28 +138,32 @@
_differentialrevisiontagre = re.compile('\AD([1-9][0-9]*)\Z')
_differentialrevisiondescre = re.compile(
- '^Differential Revision:\s*(.*)D([1-9][0-9]*)$', re.M)
+ '^Differential Revision:\s*(?:.*)D([1-9][0-9]*)$', re.M)
def getoldnodedrevmap(repo, nodelist):
"""find previous nodes that has been sent to Phabricator
- return {node: (oldnode or None, Differential Revision ID)}
+ return {node: (oldnode, Differential diff, Differential Revision ID)}
for node in nodelist with known previous sent versions, or associated
- Differential Revision IDs.
+ Differential Revision IDs. ``oldnode`` and ``Differential diff`` could
+ be ``None``.
- Examines all precursors and their tags. Tags with format like "D1234" are
- considered a match and the node with that tag, and the number after "D"
- (ex. 1234) will be returned.
+ Examines commit messages like "Differential Revision:" to get the
+ association information.
- If tags are not found, examine commit message. The "Differential Revision:"
- line could associate this changeset to a Differential Revision.
+ If such commit message line is not found, examines all precursors and their
+ tags. Tags with format like "D1234" are considered a match and the node
+ with that tag, and the number after "D" (ex. 1234) will be returned.
+
+ The ``old node``, if not None, is guaranteed to be the last diff of
+ corresponding Differential Revision, and exist in the repo.
"""
url, token = readurltoken(repo)
unfi = repo.unfiltered()
nodemap = unfi.changelog.nodemap
- result = {} # {node: (oldnode or None, drev)}
- toconfirm = {} # {node: (oldnode, {precnode}, drev)}
+ result = {} # {node: (oldnode?, lastdiff?, drev)}
+ toconfirm = {} # {node: (force, {precnode}, drev)}
for node in nodelist:
ctx = unfi[node]
# For tags like "D123", put them into "toconfirm" to verify later
@@ -169,39 +173,49 @@
for tag in unfi.nodetags(n):
m = _differentialrevisiontagre.match(tag)
if m:
- toconfirm[node] = (n, set(precnodes), int(m.group(1)))
+ toconfirm[node] = (0, set(precnodes), int(m.group(1)))
continue
- # Check commit message (make sure URL matches)
+ # Check commit message
m = _differentialrevisiondescre.search(ctx.description())
if m:
- if m.group(1).rstrip('/') == url.rstrip('/'):
- result[node] = (None, int(m.group(2)))
- else:
- unfi.ui.warn(_('%s: Differential Revision URL ignored - host '
- 'does not match config\n') % ctx)
+ toconfirm[node] = (1, set(precnodes), 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:
+ drevs = [drev for force, precs, drev in toconfirm.values()]
+ alldiffs = callconduit(unfi, 'differential.querydiffs',
+ {'revisionIDs': drevs})
+ getnode = lambda d: bin(encoding.unitolocal(
+ getdiffmeta(d).get(r'node', ''))) or None
+ for newnode, (force, precset, drev) in toconfirm.items():
+ diffs = [d for d in alldiffs.values()
+ if int(d[r'revisionID']) == drev]
+
+ # "precursors" as known by Phabricator
+ phprecset = set(getnode(d) for d in diffs)
+
+ # Ignore if precursors (Phabricator and local repo) do not overlap,
+ # and force is not set (when commit message says nothing)
+ if not force and not bool(phprecset & precset):
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)
+ continue
+
+ # Find the last node using Phabricator metadata, and make sure it
+ # exists in the repo
+ oldnode = lastdiff = None
+ if diffs:
+ lastdiff = max(diffs, key=lambda d: int(d[r'id']))
+ oldnode = getnode(lastdiff)
+ if oldnode and oldnode not in nodemap:
+ oldnode = None
+
+ result[newnode] = (oldnode, lastdiff, drev)
return result
@@ -354,7 +368,8 @@
phids = userphids(repo, reviewers)
actions.append({'type': 'reviewers.add', 'value': phids})
- oldnodedrev = getoldnodedrevmap(repo, [repo[r].node() for r in revs])
+ # {newnode: (oldnode, olddiff, olddrev}
+ oldmap = getoldnodedrevmap(repo, [repo[r].node() for r in revs])
# Send patches one by one so we know their Differential Revision IDs and
# can provide dependency relationship
@@ -364,7 +379,7 @@
ctx = repo[rev]
# Get Differential Revision ID
- oldnode, revid = oldnodedrev.get(ctx.node(), (None, None))
+ oldnode, olddiff, revid = oldmap.get(ctx.node(), (None, None, None))
if oldnode != ctx.node():
# Create or update Differential Revision
revision = createdifferentialrevision(ctx, revid, lastrevid,