Patchwork [03,of,10] phabricator: do not upload new diff if nothing changes

login
register
mail settings
Submitter Jun Wu
Date July 5, 2017, 1:58 a.m.
Message ID <576b9d41cfd1177a32e0.1499219908@x1c>
Download mbox | patch
Permalink /patch/21999/
State Accepted
Headers show

Comments

Jun Wu - July 5, 2017, 1:58 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1499211408 25200
#      Tue Jul 04 16:36:48 2017 -0700
# Node ID 576b9d41cfd1177a32e0d42e134cbb689c1568a7
# Parent  dcb29d9e139b96af5273a91150871a0b8eb0a93e
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 576b9d41cfd1
phabricator: do not upload new diff if nothing changes

Previously, `phabsend` uploads new diffs as long as the commit hash changes.
That's suboptimal because sometimes the diff is exactly the same as before,
the commit hash change is caused by a parent hash change, or commit message
change which do not affect diff content.

This patch adds a check examining actual diff contents to skip uploading new
diffs in that case.

Patch

diff --git a/contrib/phabricator.py b/contrib/phabricator.py
--- a/contrib/phabricator.py
+++ b/contrib/phabricator.py
@@ -203,15 +203,26 @@  def writediffproperties(ctx, diff):
     callconduit(ctx.repo(), 'differential.setdiffproperty', params)
 
-def createdifferentialrevision(ctx, revid=None, parentrevid=None):
+def createdifferentialrevision(ctx, revid=None, parentrevid=None, oldnode=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.
+
+    If oldnode is not None, check if the patch content (without commit message
+    and metadata) has changed before creating another diff.
     """
     repo = ctx.repo()
-    diff = creatediff(ctx)
-    writediffproperties(ctx, diff)
+    if oldnode:
+        diffopts = mdiff.diffopts(git=True, context=1)
+        oldctx = repo.unfiltered()[oldnode]
+        neednewdiff = (getdiff(ctx, diffopts) != getdiff(oldctx, diffopts))
+    else:
+        neednewdiff = True
 
-    transactions = [{'type': 'update', 'value': diff[r'phid']}]
+    transactions = []
+    if neednewdiff:
+        diff = creatediff(ctx)
+        writediffproperties(ctx, diff)
+        transactions.append({'type': 'update', 'value': diff[r'phid']})
 
     # Use a temporary summary to set dependency. There might be better ways but
@@ -272,5 +283,6 @@  def phabsend(ui, repo, *revs, **opts):
         if oldnode != ctx.node():
             # Create or update Differential Revision
-            revision = createdifferentialrevision(ctx, revid, lastrevid)
+            revision = createdifferentialrevision(ctx, revid, lastrevid,
+                                                  oldnode)
             newrevid = int(revision[r'object'][r'id'])
             if revid: