Patchwork D3824: cleanupnodes: preserve phase of parents of new nodes

login
register
mail settings
Submitter phabricator
Date June 21, 2018, 3:56 p.m.
Message ID <differential-rev-PHID-DREV-cjdpvzvw4dcaq7c5nuhi-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/32364/
State Superseded
Headers show

Comments

phabricator - June 21, 2018, 3:56 p.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  As Yuya noted in the review of https://phab.mercurial-scm.org/D3818, passing in
  targetphase=phases.draft would result in advancing the phase boundary
  of a secret-phase parent. We never pass targetphase=phases.draft so
  far, but it's a bug waiting to happen.
  
  I tried to refactor it so max(parentphase, X) happened in one place,
  but I couldn't come up with good variables names and I ended up with a
  "newphase = max(newphase, parentphase)" line, which made the whole
  block not look any better to me.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/scmutil.py

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -842,13 +842,13 @@ 
             return newphases.get(ctx.node(), ctx.phase())
         for newnode in allnewnodes:
             ctx = unfi[newnode]
+            parentphase = max(phase(p) for p in ctx.parents())
             if targetphase is None:
                 oldphase = max(unfi[oldnode].phase()
                                for oldnode in precursors[newnode])
-                parentphase = max(phase(p) for p in ctx.parents())
                 newphase = max(oldphase, parentphase)
             else:
-                newphase = targetphase
+                newphase = max(targetphase, parentphase)
             newphases[newnode] = newphase
             if newphase > ctx.phase():
                 toretract.setdefault(newphase, []).append(newnode)