comparison contrib/phabricator.py @ 33808:fa3aa6c98bb7

phabricator: add --amend option to phabsend Previously `hg phabsend` was imitating `hg email` and won't mutate changesets. That works fine with reviewer-push workflow, reviewers run `phabread`, `import`. However, it does not work well with author-push workflow. Namely, the author needs to run extra commands to get the right commit message, and remove the local tag after push. This patch solves those issues by adding the `--amend` option, so local changesets will have the right commit message, and tags become unnecessary. Test Plan: Given the following DAG: o 17 o 16 | o 15 | @ 14 |/ o 13 o 12 Run `hg phabsend '(13::)-17' --amend`, check the new DAG looks like: o 21 | o 20 | @ 19 |/ o 18 | o 17 | x 16 | x 13 |/ o 12 And commit messages are updated to contain the `Differential Revision` lines. Use `phabread` to make sure Phabricator has the amended node recorded. Also check `phabsend .` followed by a `phabsend . --amend`, the commit message will be updated and the tag will be removed. Differential Revision: https://phab.mercurial-scm.org/D122
author Jun Wu <quark@fb.com>
date Fri, 04 Aug 2017 12:39:29 -0700
parents f7d6978a4da9
children 75fdaf851e83
comparison
equal deleted inserted replaced
33807:0975506120fb 33808:fa3aa6c98bb7
36 import re 36 import re
37 37
38 from mercurial.node import bin, nullid 38 from mercurial.node import bin, nullid
39 from mercurial.i18n import _ 39 from mercurial.i18n import _
40 from mercurial import ( 40 from mercurial import (
41 cmdutil,
42 context,
41 encoding, 43 encoding,
42 error, 44 error,
43 mdiff, 45 mdiff,
44 obsutil, 46 obsutil,
45 patch, 47 patch,
313 315
314 revision = callconduit(repo, 'differential.revision.edit', params) 316 revision = callconduit(repo, 'differential.revision.edit', params)
315 if not revision: 317 if not revision:
316 raise error.Abort(_('cannot create revision for %s') % ctx) 318 raise error.Abort(_('cannot create revision for %s') % ctx)
317 319
318 return revision 320 return revision, diff
319 321
320 def userphids(repo, names): 322 def userphids(repo, names):
321 """convert user names to PHIDs""" 323 """convert user names to PHIDs"""
322 query = {'constraints': {'usernames': names}} 324 query = {'constraints': {'usernames': names}}
323 result = callconduit(repo, 'user.search', query) 325 result = callconduit(repo, 'user.search', query)
331 % ' '.join(sorted(unresolved))) 333 % ' '.join(sorted(unresolved)))
332 return [entry[r'phid'] for entry in data] 334 return [entry[r'phid'] for entry in data]
333 335
334 @command('phabsend', 336 @command('phabsend',
335 [('r', 'rev', [], _('revisions to send'), _('REV')), 337 [('r', 'rev', [], _('revisions to send'), _('REV')),
338 ('', 'amend', False, _('update commit messages')),
336 ('', 'reviewer', [], _('specify reviewers')), 339 ('', 'reviewer', [], _('specify reviewers')),
337 ('', 'confirm', None, _('ask for confirmation before sending'))], 340 ('', 'confirm', None, _('ask for confirmation before sending'))],
338 _('REV [OPTIONS]')) 341 _('REV [OPTIONS]'))
339 def phabsend(ui, repo, *revs, **opts): 342 def phabsend(ui, repo, *revs, **opts):
340 """upload changesets to Phabricator 343 """upload changesets to Phabricator
346 For the first time uploading changesets, local tags will be created to 349 For the first time uploading changesets, local tags will be created to
347 maintain the association. After the first time, phabsend will check 350 maintain the association. After the first time, phabsend will check
348 obsstore and tags information so it can figure out whether to update an 351 obsstore and tags information so it can figure out whether to update an
349 existing Differential Revision, or create a new one. 352 existing Differential Revision, or create a new one.
350 353
354 If --amend is set, update commit messages so they have the
355 ``Differential Revision`` URL, remove related tags. This is similar to what
356 arcanist will do, and is more desired in author-push workflows. Otherwise,
357 use local tags to record the ``Differential Revision`` association.
358
351 The --confirm option lets you confirm changesets before sending them. You 359 The --confirm option lets you confirm changesets before sending them. You
352 can also add following to your configuration file to make it default 360 can also add following to your configuration file to make it default
353 behaviour. 361 behaviour.
354 362
355 [phabsend] 363 [phabsend]
356 confirm = true 364 confirm = true
365
366 phabsend will check obsstore and the above association to decide whether to
367 update an existing Differential Revision, or create a new one.
357 """ 368 """
358 revs = list(revs) + opts.get('rev', []) 369 revs = list(revs) + opts.get('rev', [])
359 revs = scmutil.revrange(repo, revs) 370 revs = scmutil.revrange(repo, revs)
360 371
361 if not revs: 372 if not revs:
362 raise error.Abort(_('phabsend requires at least one changeset')) 373 raise error.Abort(_('phabsend requires at least one changeset'))
374 if opts.get('amend'):
375 cmdutil.checkunfinished(repo)
363 376
364 confirm = ui.configbool('phabsend', 'confirm') 377 confirm = ui.configbool('phabsend', 'confirm')
365 confirm |= bool(opts.get('confirm')) 378 confirm |= bool(opts.get('confirm'))
366 if confirm: 379 if confirm:
367 confirmed = _confirmbeforesend(repo, revs) 380 confirmed = _confirmbeforesend(repo, revs)
375 actions.append({'type': 'reviewers.add', 'value': phids}) 388 actions.append({'type': 'reviewers.add', 'value': phids})
376 389
377 # {newnode: (oldnode, olddiff, olddrev} 390 # {newnode: (oldnode, olddiff, olddrev}
378 oldmap = getoldnodedrevmap(repo, [repo[r].node() for r in revs]) 391 oldmap = getoldnodedrevmap(repo, [repo[r].node() for r in revs])
379 392
393 drevids = [] # [int]
394 diffmap = {} # {newnode: diff}
395
380 # Send patches one by one so we know their Differential Revision IDs and 396 # Send patches one by one so we know their Differential Revision IDs and
381 # can provide dependency relationship 397 # can provide dependency relationship
382 lastrevid = None 398 lastrevid = None
383 for rev in revs: 399 for rev in revs:
384 ui.debug('sending rev %d\n' % rev) 400 ui.debug('sending rev %d\n' % rev)
385 ctx = repo[rev] 401 ctx = repo[rev]
386 402
387 # Get Differential Revision ID 403 # Get Differential Revision ID
388 oldnode, olddiff, revid = oldmap.get(ctx.node(), (None, None, None)) 404 oldnode, olddiff, revid = oldmap.get(ctx.node(), (None, None, None))
389 if oldnode != ctx.node(): 405 if oldnode != ctx.node() or opts.get('amend'):
390 # Create or update Differential Revision 406 # Create or update Differential Revision
391 revision = createdifferentialrevision(ctx, revid, lastrevid, 407 revision, diff = createdifferentialrevision(
392 oldnode, olddiff, actions) 408 ctx, revid, lastrevid, oldnode, olddiff, actions)
409 diffmap[ctx.node()] = diff
393 newrevid = int(revision[r'object'][r'id']) 410 newrevid = int(revision[r'object'][r'id'])
394 if revid: 411 if revid:
395 action = _('updated') 412 action = _('updated')
396 else: 413 else:
397 action = _('created') 414 action = _('created')
398 415
399 # Create a local tag to note the association 416 # Create a local tag to note the association, if commit message
400 tagname = 'D%d' % newrevid 417 # does not have it already
401 tags.tag(repo, tagname, ctx.node(), message=None, user=None, 418 m = _differentialrevisiondescre.search(ctx.description())
402 date=None, local=True) 419 if not m or int(m.group(1)) != newrevid:
420 tagname = 'D%d' % newrevid
421 tags.tag(repo, tagname, ctx.node(), message=None, user=None,
422 date=None, local=True)
403 else: 423 else:
404 # Nothing changed. But still set "newrevid" so the next revision 424 # Nothing changed. But still set "newrevid" so the next revision
405 # could depend on this one. 425 # could depend on this one.
406 newrevid = revid 426 newrevid = revid
407 action = _('skipped') 427 action = _('skipped')
408 428
409 ui.write(_('D%s: %s - %s: %s\n') % (newrevid, action, ctx, 429 ui.write(_('D%s: %s - %s: %s\n') % (newrevid, action, ctx,
410 ctx.description().split('\n')[0])) 430 ctx.description().split('\n')[0]))
431 drevids.append(newrevid)
411 lastrevid = newrevid 432 lastrevid = newrevid
433
434 # Update commit messages and remove tags
435 if opts.get('amend'):
436 unfi = repo.unfiltered()
437 drevs = callconduit(repo, 'differential.query', {'ids': drevids})
438 with repo.wlock(), repo.lock(), repo.transaction('phabsend'):
439 wnode = unfi['.'].node()
440 mapping = {} # {oldnode: [newnode]}
441 for i, rev in enumerate(revs):
442 old = unfi[rev]
443 drevid = drevids[i]
444 drev = [d for d in drevs if int(d[r'id']) == drevid][0]
445 newdesc = getdescfromdrev(drev)
446 # Make sure commit message contain "Differential Revision"
447 if old.description() != newdesc:
448 parents = [
449 mapping.get(old.p1().node(), (old.p1(),))[0],
450 mapping.get(old.p2().node(), (old.p2(),))[0],
451 ]
452 new = context.metadataonlyctx(
453 repo, old, parents=parents, text=newdesc,
454 user=old.user(), date=old.date(), extra=old.extra())
455 newnode = new.commit()
456 mapping[old.node()] = [newnode]
457 # Update diff property
458 writediffproperties(unfi[newnode], diffmap[old.node()])
459 # Remove local tags since it's no longer necessary
460 tagname = 'D%d' % drevid
461 if tagname in repo.tags():
462 tags.tag(repo, tagname, nullid, message=None, user=None,
463 date=None, local=True)
464 scmutil.cleanupnodes(repo, mapping, 'phabsend')
465 if wnode in mapping:
466 unfi.setparents(mapping[wnode][0])
412 467
413 # Map from "hg:meta" keys to header understood by "hg import". The order is 468 # Map from "hg:meta" keys to header understood by "hg import". The order is
414 # consistent with "hg export" output. 469 # consistent with "hg export" output.
415 _metanamemap = util.sortdict([(r'user', 'User'), (r'date', 'Date'), 470 _metanamemap = util.sortdict([(r'user', 'User'), (r'date', 'Date'),
416 (r'node', 'Node ID'), (r'parent', 'Parent ')]) 471 (r'node', 'Node ID'), (r'parent', 'Parent ')])