Mercurial > public > mercurial-scm > hg
comparison contrib/phabricator.py @ 33691:1664406a44d9
phabricator: use Phabricator's last node information
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 upload a new diff)
```
The old diff object is now returned, which could be useful in the next
patch.
Differential Revision: https://phab.mercurial-scm.org/D121
author | Jun Wu <quark@fb.com> |
---|---|
date | Mon, 17 Jul 2017 19:52:50 -0700 |
parents | 40cfe3197bc1 |
children | f100354cce52 |
comparison
equal
deleted
inserted
replaced
33690:40cfe3197bc1 | 33691:1664406a44d9 |
---|---|
136 repo.ui.setconfig('phabricator', 'repophid', repophid) | 136 repo.ui.setconfig('phabricator', 'repophid', repophid) |
137 return repophid | 137 return repophid |
138 | 138 |
139 _differentialrevisiontagre = re.compile('\AD([1-9][0-9]*)\Z') | 139 _differentialrevisiontagre = re.compile('\AD([1-9][0-9]*)\Z') |
140 _differentialrevisiondescre = re.compile( | 140 _differentialrevisiondescre = re.compile( |
141 '^Differential Revision:\s*(.*)D([1-9][0-9]*)$', re.M) | 141 '^Differential Revision:\s*(?:.*)D([1-9][0-9]*)$', re.M) |
142 | 142 |
143 def getoldnodedrevmap(repo, nodelist): | 143 def getoldnodedrevmap(repo, nodelist): |
144 """find previous nodes that has been sent to Phabricator | 144 """find previous nodes that has been sent to Phabricator |
145 | 145 |
146 return {node: (oldnode or None, Differential Revision ID)} | 146 return {node: (oldnode, Differential diff, Differential Revision ID)} |
147 for node in nodelist with known previous sent versions, or associated | 147 for node in nodelist with known previous sent versions, or associated |
148 Differential Revision IDs. | 148 Differential Revision IDs. ``oldnode`` and ``Differential diff`` could |
149 | 149 be ``None``. |
150 Examines all precursors and their tags. Tags with format like "D1234" are | 150 |
151 considered a match and the node with that tag, and the number after "D" | 151 Examines commit messages like "Differential Revision:" to get the |
152 (ex. 1234) will be returned. | 152 association information. |
153 | 153 |
154 If tags are not found, examine commit message. The "Differential Revision:" | 154 If such commit message line is not found, examines all precursors and their |
155 line could associate this changeset to a Differential Revision. | 155 tags. Tags with format like "D1234" are considered a match and the node |
156 with that tag, and the number after "D" (ex. 1234) will be returned. | |
157 | |
158 The ``old node``, if not None, is guaranteed to be the last diff of | |
159 corresponding Differential Revision, and exist in the repo. | |
156 """ | 160 """ |
157 url, token = readurltoken(repo) | 161 url, token = readurltoken(repo) |
158 unfi = repo.unfiltered() | 162 unfi = repo.unfiltered() |
159 nodemap = unfi.changelog.nodemap | 163 nodemap = unfi.changelog.nodemap |
160 | 164 |
161 result = {} # {node: (oldnode or None, drev)} | 165 result = {} # {node: (oldnode?, lastdiff?, drev)} |
162 toconfirm = {} # {node: (oldnode, {precnode}, drev)} | 166 toconfirm = {} # {node: (force, {precnode}, drev)} |
163 for node in nodelist: | 167 for node in nodelist: |
164 ctx = unfi[node] | 168 ctx = unfi[node] |
165 # For tags like "D123", put them into "toconfirm" to verify later | 169 # For tags like "D123", put them into "toconfirm" to verify later |
166 precnodes = list(obsolete.allprecursors(unfi.obsstore, [node])) | 170 precnodes = list(obsolete.allprecursors(unfi.obsstore, [node])) |
167 for n in precnodes: | 171 for n in precnodes: |
168 if n in nodemap: | 172 if n in nodemap: |
169 for tag in unfi.nodetags(n): | 173 for tag in unfi.nodetags(n): |
170 m = _differentialrevisiontagre.match(tag) | 174 m = _differentialrevisiontagre.match(tag) |
171 if m: | 175 if m: |
172 toconfirm[node] = (n, set(precnodes), int(m.group(1))) | 176 toconfirm[node] = (0, set(precnodes), int(m.group(1))) |
173 continue | 177 continue |
174 | 178 |
175 # Check commit message (make sure URL matches) | 179 # Check commit message |
176 m = _differentialrevisiondescre.search(ctx.description()) | 180 m = _differentialrevisiondescre.search(ctx.description()) |
177 if m: | 181 if m: |
178 if m.group(1).rstrip('/') == url.rstrip('/'): | 182 toconfirm[node] = (1, set(precnodes), int(m.group(1))) |
179 result[node] = (None, int(m.group(2))) | |
180 else: | |
181 unfi.ui.warn(_('%s: Differential Revision URL ignored - host ' | |
182 'does not match config\n') % ctx) | |
183 | 183 |
184 # Double check if tags are genuine by collecting all old nodes from | 184 # Double check if tags are genuine by collecting all old nodes from |
185 # Phabricator, and expect precursors overlap with it. | 185 # Phabricator, and expect precursors overlap with it. |
186 if toconfirm: | 186 if toconfirm: |
187 confirmed = {} # {drev: {oldnode}} | 187 drevs = [drev for force, precs, drev in toconfirm.values()] |
188 drevs = [drev for n, precs, drev in toconfirm.values()] | 188 alldiffs = callconduit(unfi, 'differential.querydiffs', |
189 diffs = callconduit(unfi, 'differential.querydiffs', | 189 {'revisionIDs': drevs}) |
190 {'revisionIDs': drevs}) | 190 getnode = lambda d: bin(encoding.unitolocal( |
191 for diff in diffs.values(): | 191 getdiffmeta(d).get(r'node', ''))) or None |
192 drev = int(diff[r'revisionID']) | 192 for newnode, (force, precset, drev) in toconfirm.items(): |
193 oldnode = bin(encoding.unitolocal(getdiffmeta(diff).get(r'node'))) | 193 diffs = [d for d in alldiffs.values() |
194 if node: | 194 if int(d[r'revisionID']) == drev] |
195 confirmed.setdefault(drev, set()).add(oldnode) | 195 |
196 for newnode, (oldnode, precset, drev) in toconfirm.items(): | 196 # "precursors" as known by Phabricator |
197 if bool(precset & confirmed.get(drev, set())): | 197 phprecset = set(getnode(d) for d in diffs) |
198 result[newnode] = (oldnode, drev) | 198 |
199 else: | 199 # Ignore if precursors (Phabricator and local repo) do not overlap, |
200 # and force is not set (when commit message says nothing) | |
201 if not force and not bool(phprecset & precset): | |
200 tagname = 'D%d' % drev | 202 tagname = 'D%d' % drev |
201 tags.tag(repo, tagname, nullid, message=None, user=None, | 203 tags.tag(repo, tagname, nullid, message=None, user=None, |
202 date=None, local=True) | 204 date=None, local=True) |
203 unfi.ui.warn(_('D%s: local tag removed - does not match ' | 205 unfi.ui.warn(_('D%s: local tag removed - does not match ' |
204 'Differential history\n') % drev) | 206 'Differential history\n') % drev) |
207 continue | |
208 | |
209 # Find the last node using Phabricator metadata, and make sure it | |
210 # exists in the repo | |
211 oldnode = lastdiff = None | |
212 if diffs: | |
213 lastdiff = max(diffs, key=lambda d: int(d[r'id'])) | |
214 oldnode = getnode(lastdiff) | |
215 if oldnode and oldnode not in nodemap: | |
216 oldnode = None | |
217 | |
218 result[newnode] = (oldnode, lastdiff, drev) | |
205 | 219 |
206 return result | 220 return result |
207 | 221 |
208 def getdiff(ctx, diffopts): | 222 def getdiff(ctx, diffopts): |
209 """plain-text diff without header (user, commit message, etc)""" | 223 """plain-text diff without header (user, commit message, etc)""" |
352 reviewers = opts.get('reviewer', []) | 366 reviewers = opts.get('reviewer', []) |
353 if reviewers: | 367 if reviewers: |
354 phids = userphids(repo, reviewers) | 368 phids = userphids(repo, reviewers) |
355 actions.append({'type': 'reviewers.add', 'value': phids}) | 369 actions.append({'type': 'reviewers.add', 'value': phids}) |
356 | 370 |
357 oldnodedrev = getoldnodedrevmap(repo, [repo[r].node() for r in revs]) | 371 # {newnode: (oldnode, olddiff, olddrev} |
372 oldmap = getoldnodedrevmap(repo, [repo[r].node() for r in revs]) | |
358 | 373 |
359 # Send patches one by one so we know their Differential Revision IDs and | 374 # Send patches one by one so we know their Differential Revision IDs and |
360 # can provide dependency relationship | 375 # can provide dependency relationship |
361 lastrevid = None | 376 lastrevid = None |
362 for rev in revs: | 377 for rev in revs: |
363 ui.debug('sending rev %d\n' % rev) | 378 ui.debug('sending rev %d\n' % rev) |
364 ctx = repo[rev] | 379 ctx = repo[rev] |
365 | 380 |
366 # Get Differential Revision ID | 381 # Get Differential Revision ID |
367 oldnode, revid = oldnodedrev.get(ctx.node(), (None, None)) | 382 oldnode, olddiff, revid = oldmap.get(ctx.node(), (None, None, None)) |
368 if oldnode != ctx.node(): | 383 if oldnode != ctx.node(): |
369 # Create or update Differential Revision | 384 # Create or update Differential Revision |
370 revision = createdifferentialrevision(ctx, revid, lastrevid, | 385 revision = createdifferentialrevision(ctx, revid, lastrevid, |
371 oldnode, actions) | 386 oldnode, actions) |
372 newrevid = int(revision[r'object'][r'id']) | 387 newrevid = int(revision[r'object'][r'id']) |