This is an archive of the discontinued Mercurial Phabricator instance.

Differential D35

phabricator: sanity check Differential Revision from commit message
ClosedPublic

Authored by quark on Jul 11 2017, 11:56 AM.

Details

Reviewers
durin42
phillco
krbullock
Commits
rHG91e3dcefc9b7: phabricator: sanity check Differential Revision from commit message
rHG9132c5150af0: phabricator: sanity check Differential Revision from commit message
Summary

Previously, we trust Differential Revision in commit message blindly. This
patch adds sanity check so a host name change will be detected and the
commit message will be ignored.

Test Plan

Send patches to a test Phabricator instance. Use phabread and import to
apply them in another repo, and change commit message so the URL does not
match the configured URL. Resend them using phabsend, make sure a warning is
printed saying the changed URL is ignored.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

quark created this revision.Jul 11 2017, 11:56 AM
phillco accepted this revision.Jul 13 2017, 2:06 PM
This revision is now accepted and ready to land.Jul 13 2017, 2:06 PM
krbullock accepted this revision.Jul 18 2017, 2:31 PM
krbullock added a subscriber: krbullock.

Queued, thanks.

Closed by commit rHG9132c5150af0: phabricator: sanity check Differential Revision from commit message (authored by quark). · Explain WhyJul 18 2017, 2:41 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mcontrib/phabricator.py (10 lines)

Diff 282

contrib/phabricator.py

if len(query[r'data']) == 0: if len(query[r'data']) == 0:
return None return None
repophid = encoding.strtolocal(query[r'data'][0][r'phid']) repophid = encoding.strtolocal(query[r'data'][0][r'phid'])
repo.ui.setconfig('phabricator', 'repophid', repophid) repo.ui.setconfig('phabricator', 'repophid', repophid)
return repophid return repophid
_differentialrevisiontagre = re.compile('\AD([1-9][0-9]*)\Z') _differentialrevisiontagre = re.compile('\AD([1-9][0-9]*)\Z')
_differentialrevisiondescre = re.compile( _differentialrevisiondescre = re.compile(
'^Differential Revision:.*D([1-9][0-9]*)$', re.M) '^Differential Revision:\s*(.*)D([1-9][0-9]*)$', re.M)
def getoldnodedrevmap(repo, nodelist): def getoldnodedrevmap(repo, nodelist):
"""find previous nodes that has been sent to Phabricator """find previous nodes that has been sent to Phabricator
return {node: (oldnode or None, Differential Revision ID)} return {node: (oldnode or None, Differential Revision ID)}
for node in nodelist with known previous sent versions, or associated for node in nodelist with known previous sent versions, or associated
Differential Revision IDs. Differential Revision IDs.
for n in precnodes: for n in precnodes:
if n in nodemap: if n in nodemap:
for tag in unfi.nodetags(n): for tag in unfi.nodetags(n):
m = _differentialrevisiontagre.match(tag) m = _differentialrevisiontagre.match(tag)
if m: if m:
toconfirm[node] = (n, set(precnodes), int(m.group(1))) toconfirm[node] = (n, set(precnodes), int(m.group(1)))
continue continue
# Check commit message # Check commit message (make sure URL matches)
m = _differentialrevisiondescre.search(ctx.description()) m = _differentialrevisiondescre.search(ctx.description())
if m: if m:
result[node] = (None, int(m.group(1))) 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)
# Double check if tags are genuine by collecting all old nodes from # Double check if tags are genuine by collecting all old nodes from
# Phabricator, and expect precursors overlap with it. # Phabricator, and expect precursors overlap with it.
if toconfirm: if toconfirm:
confirmed = {} # {drev: {oldnode}} confirmed = {} # {drev: {oldnode}}
drevs = [drev for n, precs, drev in toconfirm.values()] drevs = [drev for n, precs, drev in toconfirm.values()]
diffs = callconduit(unfi, 'differential.querydiffs', diffs = callconduit(unfi, 'differential.querydiffs',
{'revisionIDs': drevs}) {'revisionIDs': drevs})