Patchwork D8308: phabricator: record all local commits used to create a Differential revision

login
register
mail settings
Submitter phabricator
Date March 20, 2020, 9:24 p.m.
Message ID <differential-rev-PHID-DREV-6qmpzmapypif376h7sex-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45845/
State Superseded
Headers show

Comments

phabricator - March 20, 2020, 9:24 p.m.
mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Arcanist records all of the commits that it squashes into a single review, and
  that info will be helpful when adding similar functionality.  This info is used
  when submitting an updated review, so that the extension can recalculate the old
  diff and see if a new one is necessary, or if it is just a property update.  It
  also shows on the `commits` tab in the `Revision Contents` section.
  
  When submitting in the usual 1:1 commit to review mode, the wire protocol is
  unchanged.
  
  The content of `hg:meta` is a bit odd, but such is the problem when folding
  several commits.  The choice for the parent node is obvious, but the `node`
  value uses the tip commit because that seems more natural, and is used elsewhere
  to look up the previous diff when updating.  The rest of the attributes follow
  from there.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS




To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel

Patch

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -968,42 +968,49 @@ 
     return diff
 
 
-def writediffproperties(ctx, diff):
-    """write metadata to diff so patches could be applied losslessly"""
+def writediffproperties(ctxs, diff):
+    """write metadata to diff so patches could be applied losslessly
+
+    ``ctxs`` is the list of commits that created the diff, in ascending order.
+    The list is generally a single commit, but may be several when using
+    ``phabsend --fold``.
+    """
     # creatediff returns with a diffid but query returns with an id
     diffid = diff.get(b'diffid', diff.get(b'id'))
+    basectx = ctxs[0]
+    tipctx = ctxs[-1]
+
     params = {
         b'diff_id': diffid,
         b'name': b'hg:meta',
         b'data': templatefilters.json(
             {
-                b'user': ctx.user(),
-                b'date': b'%d %d' % ctx.date(),
-                b'branch': ctx.branch(),
-                b'node': ctx.hex(),
-                b'parent': ctx.p1().hex(),
+                b'user': tipctx.user(),
+                b'date': b'%d %d' % tipctx.date(),
+                b'branch': tipctx.branch(),
+                b'node': tipctx.hex(),
+                b'parent': basectx.p1().hex(),
             }
         ),
     }
-    callconduit(ctx.repo().ui, b'differential.setdiffproperty', params)
+    callconduit(basectx.repo().ui, b'differential.setdiffproperty', params)
 
+    commits = {}
+    for ctx in ctxs:
+        commits[ctx.hex()] = {
+            b'author': stringutil.person(ctx.user()),
+            b'authorEmail': stringutil.email(ctx.user()),
+            b'time': int(ctx.date()[0]),
+            b'commit': ctx.hex(),
+            b'parents': [ctx.p1().hex()],
+            b'branch': ctx.branch(),
+        }
     params = {
         b'diff_id': diffid,
         b'name': b'local:commits',
-        b'data': templatefilters.json(
-            {
-                ctx.hex(): {
-                    b'author': stringutil.person(ctx.user()),
-                    b'authorEmail': stringutil.email(ctx.user()),
-                    b'time': int(ctx.date()[0]),
-                    b'commit': ctx.hex(),
-                    b'parents': [ctx.p1().hex()],
-                    b'branch': ctx.branch(),
-                },
-            }
-        ),
+        b'data': templatefilters.json(commits),
     }
-    callconduit(ctx.repo().ui, b'differential.setdiffproperty', params)
+    callconduit(basectx.repo().ui, b'differential.setdiffproperty', params)
 
 
 def createdifferentialrevision(
@@ -1049,7 +1056,7 @@ 
         # pushers could know the correct node metadata.
         assert olddiff
         diff = olddiff
-    writediffproperties(ctx, diff)
+    writediffproperties([ctx], diff)
 
     # Set the parent Revision every time, so commit re-ordering is picked-up
     if parentrevphid:
@@ -1289,7 +1296,9 @@ 
                     # If it fails just warn and keep going, otherwise the DREV
                     # associations will be lost
                     try:
-                        writediffproperties(unfi[newnode], diffmap[old.node()])
+                        writediffproperties(
+                            [unfi[newnode]], diffmap[old.node()]
+                        )
                     except util.urlerr.urlerror:
                         ui.warnnoi18n(
                             b'Failed to update metadata for D%d\n' % drevid