Patchwork D6514: phabricator: use parents.set to always set dependencies

login
register
mail settings
Submitter phabricator
Date June 12, 2019, 4:54 p.m.
Message ID <900051265a0df3937e9c70b12e7f40f0@localhost.localdomain>
Download mbox | patch
Permalink /patch/40449/
State Not Applicable
Headers show

Comments

phabricator - June 12, 2019, 4:54 p.m.
Closed by commit rHGc19d259fd6ad: phabricator: use parents.set to always set dependencies (authored by Kwan).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D6514?vs=15452&id=15459

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6514/new/

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

AFFECTED FILES
  hgext/phabricator.py
  tests/phabricator/phabsend-create-public.json
  tests/phabricator/phabsend-update-alpha-create-beta.json

CHANGE DETAILS




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

Patch

diff --git a/tests/phabricator/phabsend-update-alpha-create-beta.json b/tests/phabricator/phabsend-update-alpha-create-beta.json
--- a/tests/phabricator/phabsend-update-alpha-create-beta.json
+++ b/tests/phabricator/phabsend-update-alpha-create-beta.json
@@ -768,7 +768,7 @@ 
                         "phab.mercurial-scm.org"
                     ]
                 },
-                "body": "api.token=cli-hahayouwish&transactions%5B0%5D%5Btype%5D=update&transactions%5B0%5D%5Bvalue%5D=PHID-DIFF-uhbyhoejzbniwwzj2q5c&transactions%5B1%5D%5Btype%5D=summary&transactions%5B1%5D%5Bvalue%5D=Depends+on+D1190&transactions%5B2%5D%5Btype%5D=summary&transactions%5B2%5D%5Bvalue%5D=+&transactions%5B3%5D%5Btype%5D=title&transactions%5B3%5D%5Bvalue%5D=create+beta+for+phabricator+test",
+                "body": "api.token=cli-hahayouwish&transactions%5B0%5D%5Btype%5D=update&transactions%5B0%5D%5Bvalue%5D=PHID-DIFF-uhbyhoejzbniwwzj2q5c&transactions%5B1%5D%5Btype%5D=parents.set&transactions%5B1%5D%5Bvalue%5D%5B0%5D=PHID-DREV-kikesmfxhzpfaxbzgj3l&transactions%5B2%5D%5Btype%5D=title&transactions%5B2%5D%5Bvalue%5D=create+beta+for+phabricator+test",
                 "uri": "https://phab.mercurial-scm.org//api/differential.revision.edit",
                 "method": "POST"
             },
diff --git a/tests/phabricator/phabsend-create-public.json b/tests/phabricator/phabsend-create-public.json
--- a/tests/phabricator/phabsend-create-public.json
+++ b/tests/phabricator/phabsend-create-public.json
@@ -700,7 +700,7 @@ 
                         "phab.mercurial-scm.org"
                     ]
                 },
-                "body": "api.token=cli-hahayouwish&transactions%5B0%5D%5Btype%5D=update&transactions%5B0%5D%5Bvalue%5D=PHID-DIFF-4pugk2zedyh2xm27uuvh&transactions%5B1%5D%5Btype%5D=summary&transactions%5B1%5D%5Bvalue%5D=Depends+on+D1192&transactions%5B2%5D%5Btype%5D=summary&transactions%5B2%5D%5Bvalue%5D=+&transactions%5B3%5D%5Btype%5D=title&transactions%5B3%5D%5Bvalue%5D=create+draft+change+for+phabricator+testing",
+                "body": "api.token=cli-hahayouwish&transactions%5B0%5D%5Btype%5D=update&transactions%5B0%5D%5Bvalue%5D=PHID-DIFF-4pugk2zedyh2xm27uuvh&transactions%5B1%5D%5Btype%5D=parents.set&transactions%5B1%5D%5Bvalue%5D%5B0%5D=PHID-DREV-qb4xy3abx7eu4puizvjl&transactions%5B2%5D%5Btype%5D=title&transactions%5B2%5D%5Bvalue%5D=create+draft+change+for+phabricator+testing",
                 "uri": "https://phab.mercurial-scm.org//api/differential.revision.edit",
                 "method": "POST"
             },
diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -431,12 +431,13 @@ 
     }
     callconduit(ctx.repo().ui, b'differential.setdiffproperty', params)
 
-def createdifferentialrevision(ctx, revid=None, parentrevid=None, oldnode=None,
-                               olddiff=None, actions=None, comment=None):
+def createdifferentialrevision(ctx, revid=None, parentrevphid=None,
+                               oldnode=None, olddiff=None, actions=None,
+                               comment=None):
     """create or update a Differential Revision
 
     If revid is None, create a new Differential Revision, otherwise update
-    revid. If parentrevid is not None, set it as a dependency.
+    revid. If parentrevphid is not None, set it as a dependency.
 
     If oldnode is not None, check if the patch content (without commit message
     and metadata) has changed before creating another diff.
@@ -465,14 +466,10 @@ 
         diff = olddiff
     writediffproperties(ctx, diff)
 
-    # Use a temporary summary to set dependency. There might be better ways but
-    # I cannot find them for now. But do not do that if we are updating an
-    # existing revision (revid is not None) since that introduces visible
-    # churns (someone edited "Summary" twice) on the web page.
-    if parentrevid and revid is None:
-        summary = b'Depends on D%d' % parentrevid
-        transactions += [{b'type': b'summary', b'value': summary},
-                         {b'type': b'summary', b'value': b' '}]
+    # Set the parent Revision every time, so commit re-ordering is picked-up
+    if parentrevphid:
+        transactions.append({b'type': b'parents.set',
+                             b'value': [parentrevphid]})
 
     if actions:
         transactions += actions
@@ -583,9 +580,9 @@ 
     drevids = [] # [int]
     diffmap = {} # {newnode: diff}
 
-    # Send patches one by one so we know their Differential Revision IDs and
+    # Send patches one by one so we know their Differential Revision PHIDs and
     # can provide dependency relationship
-    lastrevid = None
+    lastrevphid = None
     for rev in revs:
         ui.debug(b'sending rev %d\n' % rev)
         ctx = repo[rev]
@@ -595,10 +592,11 @@ 
         if oldnode != ctx.node() or opts.get(b'amend'):
             # Create or update Differential Revision
             revision, diff = createdifferentialrevision(
-                ctx, revid, lastrevid, oldnode, olddiff, actions,
+                ctx, revid, lastrevphid, oldnode, olddiff, actions,
                 opts.get(b'comment'))
             diffmap[ctx.node()] = diff
             newrevid = int(revision[b'object'][b'id'])
+            newrevphid = revision[b'object'][b'phid']
             if revid:
                 action = b'updated'
             else:
@@ -612,8 +610,9 @@ 
                 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.
+            # Nothing changed. But still set "newrevphid" so the next revision
+            # could depend on this one and "newrevid" for the summary line.
+            newrevphid = querydrev(repo, str(revid))[0][b'phid']
             newrevid = revid
             action = b'skipped'
 
@@ -628,7 +627,7 @@ 
         ui.write(_(b'%s - %s - %s: %s\n') % (drevdesc, actiondesc, nodedesc,
                                              desc))
         drevids.append(newrevid)
-        lastrevid = newrevid
+        lastrevphid = newrevphid
 
     # Update commit messages and remove tags
     if opts.get(b'amend'):