# HG changeset patch # User Jun Wu # Date 1501875569 25200 # Node ID fa3aa6c98bb79575b87d708654f94bfa2c602eb4 # Parent 0975506120fb1d6fff0378870e78455094a5fbec 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 diff -r 0975506120fb -r fa3aa6c98bb7 contrib/phabricator.py --- a/contrib/phabricator.py Thu Aug 10 21:30:31 2017 -0700 +++ b/contrib/phabricator.py Fri Aug 04 12:39:29 2017 -0700 @@ -38,6 +38,8 @@ from mercurial.node import bin, nullid from mercurial.i18n import _ from mercurial import ( + cmdutil, + context, encoding, error, mdiff, @@ -315,7 +317,7 @@ if not revision: raise error.Abort(_('cannot create revision for %s') % ctx) - return revision + return revision, diff def userphids(repo, names): """convert user names to PHIDs""" @@ -333,6 +335,7 @@ @command('phabsend', [('r', 'rev', [], _('revisions to send'), _('REV')), + ('', 'amend', False, _('update commit messages')), ('', 'reviewer', [], _('specify reviewers')), ('', 'confirm', None, _('ask for confirmation before sending'))], _('REV [OPTIONS]')) @@ -348,18 +351,28 @@ obsstore and tags information so it can figure out whether to update an existing Differential Revision, or create a new one. + If --amend is set, update commit messages so they have the + ``Differential Revision`` URL, remove related tags. This is similar to what + arcanist will do, and is more desired in author-push workflows. Otherwise, + use local tags to record the ``Differential Revision`` association. + The --confirm option lets you confirm changesets before sending them. You can also add following to your configuration file to make it default behaviour. [phabsend] confirm = true + + phabsend will check obsstore and the above association to decide whether to + update an existing Differential Revision, or create a new one. """ revs = list(revs) + opts.get('rev', []) revs = scmutil.revrange(repo, revs) if not revs: raise error.Abort(_('phabsend requires at least one changeset')) + if opts.get('amend'): + cmdutil.checkunfinished(repo) confirm = ui.configbool('phabsend', 'confirm') confirm |= bool(opts.get('confirm')) @@ -377,6 +390,9 @@ # {newnode: (oldnode, olddiff, olddrev} oldmap = getoldnodedrevmap(repo, [repo[r].node() for r in revs]) + drevids = [] # [int] + diffmap = {} # {newnode: diff} + # Send patches one by one so we know their Differential Revision IDs and # can provide dependency relationship lastrevid = None @@ -386,20 +402,24 @@ # Get Differential Revision ID oldnode, olddiff, revid = oldmap.get(ctx.node(), (None, None, None)) - if oldnode != ctx.node(): + if oldnode != ctx.node() or opts.get('amend'): # Create or update Differential Revision - revision = createdifferentialrevision(ctx, revid, lastrevid, - oldnode, olddiff, actions) + revision, diff = createdifferentialrevision( + ctx, revid, lastrevid, oldnode, olddiff, actions) + diffmap[ctx.node()] = diff newrevid = int(revision[r'object'][r'id']) if revid: action = _('updated') else: action = _('created') - # Create a local tag to note the association - tagname = 'D%d' % newrevid - tags.tag(repo, tagname, ctx.node(), message=None, user=None, - date=None, local=True) + # Create a local tag to note the association, if commit message + # does not have it already + m = _differentialrevisiondescre.search(ctx.description()) + if not m or int(m.group(1)) != newrevid: + tagname = 'D%d' % newrevid + tags.tag(repo, tagname, ctx.node(), message=None, user=None, + date=None, local=True) else: # Nothing changed. But still set "newrevid" so the next revision # could depend on this one. @@ -408,8 +428,43 @@ ui.write(_('D%s: %s - %s: %s\n') % (newrevid, action, ctx, ctx.description().split('\n')[0])) + drevids.append(newrevid) lastrevid = newrevid + # Update commit messages and remove tags + if opts.get('amend'): + unfi = repo.unfiltered() + drevs = callconduit(repo, 'differential.query', {'ids': drevids}) + with repo.wlock(), repo.lock(), repo.transaction('phabsend'): + wnode = unfi['.'].node() + mapping = {} # {oldnode: [newnode]} + for i, rev in enumerate(revs): + old = unfi[rev] + drevid = drevids[i] + drev = [d for d in drevs if int(d[r'id']) == drevid][0] + newdesc = getdescfromdrev(drev) + # Make sure commit message contain "Differential Revision" + if old.description() != newdesc: + parents = [ + mapping.get(old.p1().node(), (old.p1(),))[0], + mapping.get(old.p2().node(), (old.p2(),))[0], + ] + new = context.metadataonlyctx( + repo, old, parents=parents, text=newdesc, + user=old.user(), date=old.date(), extra=old.extra()) + newnode = new.commit() + mapping[old.node()] = [newnode] + # Update diff property + writediffproperties(unfi[newnode], diffmap[old.node()]) + # Remove local tags since it's no longer necessary + tagname = 'D%d' % drevid + if tagname in repo.tags(): + tags.tag(repo, tagname, nullid, message=None, user=None, + date=None, local=True) + scmutil.cleanupnodes(repo, mapping, 'phabsend') + if wnode in mapping: + unfi.setparents(mapping[wnode][0]) + # Map from "hg:meta" keys to header understood by "hg import". The order is # consistent with "hg export" output. _metanamemap = util.sortdict([(r'user', 'User'), (r'date', 'Date'),