mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The tricky part here is reasoning through all of the possible predecessor
  scenarios.  In the typical case of submitting a folded range and then
  resubmitting it (also folded), filtering the list of commits for the diff 
stored
  on Phabricator through the local predecessor list for each single node will
  result in the typical 1:1 mapping to the old node.
  
  There are edge cases like using `hg fold` within the range prior to
  resubmitting, that will result in mapping to multiple old nodes.  In that 
case,
  the first direct predecessor is needed for the base of the diff, and the last
  direct predecessor is needed for the head of the diff in order to make sure 
that
  the entire range is included in the diff content.  And none of this matters 
for
  commits in the middle of the range, as they are never used.
  
  Fortunately the only crucial thing here is the `drev` number for each node.  
For
  these complicated cases where there are multiple old nodes, simply ignore them
  all.  This will cause `createdifferentialrevision()` to generate a new diff
  (within the same Differential), and avoids complicating the code.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -483,18 +483,23 @@
         alldiffs = callconduit(
             unfi.ui, b'differential.querydiffs', {b'revisionIDs': drevs}
         )
-        getnode = lambda d: bin(getdiffmeta(d).get(b'node', b'')) or None
+
+        def getnodes(d, precset):
+            # Ignore other nodes that were combined into the Differential
+            # that aren't predecessors of the current local node.
+            return [n for n in getlocalcommits(d) if n in precset]
+
         for newnode, (force, precset, drev) in toconfirm.items():
             diffs = [
                 d for d in alldiffs.values() if int(d[b'revisionID']) == drev
             ]
 
-            # "precursors" as known by Phabricator
-            phprecset = {getnode(d) for d in diffs}
+            # local predecessors known by Phabricator
+            phprecset = {n for d in diffs for n in getnodes(d, precset)}
 
             # Ignore if precursors (Phabricator and local repo) do not overlap,
             # and force is not set (when commit message says nothing)
-            if not force and not bool(phprecset & precset):
+            if not force and not phprecset:
                 tagname = b'D%d' % drev
                 tags.tag(
                     repo,
@@ -519,7 +524,15 @@
             oldnode = lastdiff = None
             if diffs:
                 lastdiff = max(diffs, key=lambda d: int(d[b'id']))
-                oldnode = getnode(lastdiff)
+                oldnodes = getnodes(lastdiff, precset)
+
+                # If this commit was the result of `hg fold` after submission,
+                # and now resubmitted with --fold, the easiest thing to do is
+                # to leave the node clear.  This only results in creating a new
+                # diff for the _same_ Differential Revision if this commit is
+                # the first or last in the selected range.
+                if len(oldnodes) == 1:
+                    oldnode = oldnodes[0]
                 if oldnode and not has_node(oldnode):
                     oldnode = None
 
@@ -1667,6 +1680,21 @@
     return _differentialrevisiondescre.sub(uri, ctx.description())
 
 
+def getlocalcommits(diff):
+    """get the set of local commits from a diff object
+
+    See ``getdiffmeta()`` for an example diff object.
+    """
+    props = diff.get(b'properties') or {}
+    commits = props.get(b'local:commits') or {}
+    if len(commits) > 1:
+        return {bin(c) for c in commits.keys()}
+
+    # Storing the diff metadata predates storing `local:commits`, so continue
+    # to use that in the --no-fold case.
+    return {bin(getdiffmeta(diff).get(b'node', b'')) or None}
+
+
 def getdiffmeta(diff):
     """get commit metadata (date, node, user, p1) from a diff object
 



To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to