D7513: phabricator: fix processing of tags/desc in getoldnodedrevmap()

[dlax (Denis Laxalde)]

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

REVISION SUMMARY
  It seems that the previous logic was wrong (it essentially comes
  from changeset 3ab0d5767b54 <https://phab.mercurial-scm.org/rHG3ab0d5767b549d1f56fae5155a1c115a9e10f9e7> where the result got accumulated instead of
  early returned).
  First of all, the "continue" in first "if m:" is useless because we're
  at the end of the loop. Then, the algorithm seems weird because we will
  process all predecessors of a node and possibly override
  `toconfirm[node]` for each of these having a tag (maybe this doesn't
  happen, but still). Finally, we would also override `toconfirm[node]`
  when the "Differential Revision: " is found in changeset description.
  Maybe this is not a big deal when there is no mix of local tag and
  changeset description update?
  
  The logic is changed so that the loop on predecessors stops upon first
  match of a tag and so that the changeset description is only checked if
  no tag was found. Therefore, `toconfirm[node]` is only set once.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

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
@@ -403,12 +403,15 @@
                     m = _differentialrevisiontagre.match(tag)
                     if m:
                         toconfirm[node] = (0, set(precnodes), int(m.group(1)))
-                        continue
-
-        # Check commit message
-        m = _differentialrevisiondescre.search(ctx.description())
-        if m:
-            toconfirm[node] = (1, set(precnodes), int(m.group('id')))
+                        break
+                else:
+                    continue  # move to next predecessor
+                break  # found a tag, stop
+        else:
+            # Check commit message
+            m = _differentialrevisiondescre.search(ctx.description())
+            if m:
+                toconfirm[node] = (1, set(precnodes), int(m.group('id')))
 
     # Double check if tags are genuine by collecting all old nodes from
     # Phabricator, and expect precursors overlap with it.



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