D121: phabricator: use Phabricator's last node information
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG1664406a44d9: phabricator: use Phabricator's last node
information (authored by quark).
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D121?vs=550=565
REVISION DETAIL
https://phab.mercurial-scm.org/D121
AFFECTED FILES
contrib/phabricator.py
CHANGE DETAILS
diff --git a/contrib/phabricator.py b/contrib/phabricator.py
--- a/contrib/phabricator.py
+++ b/contrib/phabricator.py
@@ -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):
D121: phabricator: use Phabricator's last node information
quark updated this revision to Diff 484.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D121?vs=232=484
REVISION DETAIL
https://phab.mercurial-scm.org/D121
AFFECTED FILES
contrib/phabricator.py
CHANGE DETAILS
diff --git a/contrib/phabricator.py b/contrib/phabricator.py
--- a/contrib/phabricator.py
+++ b/contrib/phabricator.py
@@ -147,19 +147,22 @@
for node in nodelist with known previous sent versions, or associated
Differential Revision IDs.
-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)}
+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 +172,47 @@
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
+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, drev)
return result
To: quark, #hg-reviewers, mitrandir
Cc: mercurial-devel
Re: D121: phabricator: use Phabricator's last node information
quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
This makes it more strict when checking whether or not we should update a
Differential Revision. For example,
a) Alice updates D1 to content 1.
b) Bob updates D1 to content 2.
c) Alice tries to update D1 to content 1.
Previously, `c)` will do nothing because `phabsend` detects the patch is not
changed. A more correct behavior is to override Bob's update here, hence the
patch.
This also makes it possible to return a reaonsable "last node" when there is
no tags but only `Differential Revision` commit messages.
TEST PLAN
for i in A B C; do echo $i > $i; hg ci -m $i -A $i; done
hg phabsend 0::
# D40: created
# D41: created
# D42: created
echo 3 >> C; hg amend; hg phabsend .
# D42: updated
hg tag --local --hidden -r 2 -f D42
# move tag to the previous version
hg phabsend .
# D42: skipped (previously it would be "updated")
rm -rf .hg; hg init
hg phabread --stack D42 | hg import -
hg phabsend .
# D42: updated
hg tag --local --remove D42
hg commit --amend
hg phabsend .
# D42: updated (no new diff uploaded, previously it will update new diff)
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D121
AFFECTED FILES
contrib/phabricator.py
CHANGE DETAILS
diff --git a/contrib/phabricator.py b/contrib/phabricator.py
--- a/contrib/phabricator.py
+++ b/contrib/phabricator.py
@@ -147,19 +147,22 @@
for node in nodelist with known previous sent versions, or associated
Differential Revision IDs.
-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)}
+toconfirm = {} # {node: (force, {precnode}, drev)}
for node in nodelist:
ctx = unfi[node]
# For tags like "D123", put them into "toconfirm" to verify later
@@ -169,35 +172,48 @@
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
+# Check commit message. If "Differential Revision:" exists and matches
+# the URL. Trust it blindly even if precursors do not match.
m = _differentialrevisiondescre.search(ctx.description())
if m:
-result[node] = (None, int(m.group(1)))
+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"
