D122: phabricator: add --amend option to phabsend

[quark (Jun Wu)]

quark updated this revision to Diff 485.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D122?vs=233&id=485

REVISION DETAIL
  https://phab.mercurial-scm.org/D122

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
@@ -38,6 +38,8 @@
 from mercurial.node import bin, nullid
 from mercurial.i18n import _
 from mercurial import (
+    cmdutil,
+    context,
     encoding,
     error,
     mdiff,
@@ -324,6 +326,7 @@
 
 @command('phabsend',
          [('r', 'rev', [], _('revisions to send'), _('REV')),
+          ('', 'amend', False, _('update commit messages')),
           ('', 'reviewer', [], _('specify reviewers'))],
          _('REV [OPTIONS]'))
 def phabsend(ui, repo, *revs, **opts):
@@ -333,16 +336,21 @@
     with a linear dependencies relationship using the order specified by the
     revset.
 
-    For the first time uploading changesets, local tags will be created to
-    maintain the association. After the first time, phabsend will check
-    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.
+
+    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)
 
     actions = []
     reviewers = opts.get('reviewer', [])
@@ -354,6 +362,7 @@
 
     # Send patches one by one so we know their Differential Revision IDs and
     # can provide dependency relationship
+    drevids = []
     lastrevid = None
     for rev in revs:
         ui.debug('sending rev %d\n' % rev)
@@ -371,20 +380,55 @@
             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.
             newrevid = revid
             action = _('skipped')
 
         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())
+                    mapping[old.node()] = [new.commit()]
+                # 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'),



To: quark, #hg-reviewers, mitrandir
Cc: mercurial-devel