changeset 43847:16b607e9f714

phabricator: fix processing of tags/desc in getoldnodedrevmap() It seems that the previous logic was wrong (it essentially comes from changeset 3ab0d5767b54 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. Differential Revision: https://phab.mercurial-scm.org/D7513
author Denis Laxalde <denis.laxalde@logilab.fr>
date Thu, 21 Nov 2019 18:10:12 +0100
parents 36444dddaeb4
children 6331a6fc3304
files hgext/phabricator.py
diffstat 1 files changed, 9 insertions(+), 6 deletions(-) [+]
line wrap: on
line diff
--- a/hgext/phabricator.py	Tue Dec 10 10:31:18 2019 -0500
+++ b/hgext/phabricator.py	Thu Nov 21 18:10:12 2019 +0100
@@ -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.