mutation: public commits are never obsolete

Summary:
The computation of commit obsolescence is inconsistent.  If we compute the full
set of obsolete commits in `mutation.obsoletecache.obsoletenodes`, then we
correctly ignore public commits as they cannot be obsolete.

However, if we compute the obsolescence state for a single public commit with
`mutation.obsoletecache.isobsolete`, and that commit somehow has a visible
successor, then we will incorrectly consider the commit as obsolete.

Similarly, `allpredecessors` and `allsuccessors` should stop when they hit a
public commit.

Reviewed By: quark-zju

Differential Revision: D20892778

fbshipit-source-id: 223cb8b2bc9f2f08124df6ff51c2eb208bb8eb5f

eden/scm/edenscm/mercurial/mutation.py

@@ -182,6 +182,7 @@ def allpredecessors(repo, nodes, startdepth=None, stopdepth=None):
     thislevel = set(nodes)
     nextlevel = set()
     seen = {nullid}
+    ispublic = getispublicfunc(repo)
     while thislevel and (stopdepth is None or depth < stopdepth):
         for current in thislevel:
             if current in seen:
@@ -195,7 +196,7 @@ def allpredecessors(repo, nodes, startdepth=None, stopdepth=None):
                 pred = entry.preds()
             if pred is not None:
                 for nextnode in pred:
-                    if nextnode not in seen:
+                    if nextnode not in seen and not ispublic(nextnode):
                         nextlevel.add(nextnode)
         depth += 1
         thislevel = nextlevel
@@ -210,6 +211,7 @@ def allsuccessors(repo, nodes, startdepth=None, stopdepth=None):
     thislevel = set(nodes)
     nextlevel = set()
     seen = set()
+    ispublic = getispublicfunc(repo)
     while thislevel and (stopdepth is None or depth < stopdepth):
         for current in thislevel:
             if current in seen:
@@ -217,6 +219,8 @@ def allsuccessors(repo, nodes, startdepth=None, stopdepth=None):
             seen.add(current)
             if startdepth is None or depth >= startdepth:
                 yield current
+            if ispublic(current):
+                continue
             succsets = lookupsuccessors(repo, current)
             if succsets:
                 nextlevel = nextlevel.union(*succsets)
@@ -243,6 +247,9 @@ class obsoletecache(object):
             return False
         if node not in repo:
             return False
+        ispublic = getispublicfunc(repo)
+        if ispublic(node):
+            return False
         obsolete = self.obsolete[repo.filtername]
         if node in obsolete:
             return True
@@ -339,8 +346,7 @@ def fate(repo, node):
     This returns a list of ([nodes], operation) pairs, indicating mutations that
     happened to this node that resulted in one or more visible commits.
     """
-    clrev = repo.changelog.rev
+    ispublic = getispublicfunc(repo)
-    phase = repo._phasecache.phase
     fate = []
     if isobsolete(repo, node):
         for succset in successorssets(repo, node, closest=True):
@@ -351,7 +357,7 @@ def fate(repo, node):
             else:
                 succ = succset[0]
                 # Base the default operation name on the successor's phase
-                if succ in repo and phase(repo, clrev(succ)) == phases.public:
+                if ispublic(succ):
                     op = "land"
                 else:
                     op = "rewrite"
@@ -377,6 +383,7 @@ def predecessorsset(repo, startnode, closest=False):
     the start node.
     """
     seen = {startnode}
+    ispublic = getispublicfunc(repo)
 
     def get(node):
         """Get immediate predecessors
@@ -391,7 +398,9 @@ def predecessorsset(repo, startnode, closest=False):
         if entry is not None:
             preds = entry.preds()
             if preds is not None:
-                return [pred for pred in preds if pred not in seen] or [node]
+                return [
+                    pred for pred in preds if pred not in seen and not ispublic(pred)
+                ] or [node]
         return [node]
 
     preds = [startnode]
@@ -480,6 +489,7 @@ def successorssets(repo, startnode, closest=False, cache=None):
     signature-compatible with ``obsutil.successorssets``.
     """
     seen = {startnode}
+    ispublic = getispublicfunc(repo)
 
     def getsets(node):
         """Get immediate successors sets
@@ -490,6 +500,8 @@ def successorssets(repo, startnode, closest=False, cache=None):
         If the node has no successors, returns a list containing a single
         successors set which contains the node itself.
         """
+        if ispublic(node):
+            return [[node]]
         succsets = [
             succset
             for succset in lookupsuccessors(repo, node)
@@ -826,3 +838,20 @@ def getisvisiblefunc(repo):
     else:
         # Use cl.hasnode to test
         return repo.changelog.hasnode
+
+
+def getispublicfunc(repo):
+    """get a (node) -> bool function to test whether a commit is public"""
+    nodemap = repo.changelog.nodemap
+    getphase = repo._phasecache.phase
+
+    def ispublic(
+        node, nodemap=nodemap, getphase=getphase, repo=repo, public=phases.public
+    ):
+        try:
+            rev = nodemap[node]
+        except error.RevlogError:
+            return False
+        return getphase(repo, rev) == public
+
+    return ispublic

eden/scm/tests/test-mutation-phases.t

@@ -0,0 +1,46 @@
+#require py2
+#chg-compatible
+
+  $ enable amend
+  $ setconfig experimental.evolution=obsolete
+  $ setconfig experimental.narrow-heads=false
+  $ setconfig visibility.enabled=true
+  $ setconfig mutation.record=true mutation.enabled=true mutation.date="0 0"
+
+  $ newrepo
+  $ echo "base" > base
+  $ hg commit -Aqm base
+  $ echo 1 > file
+  $ hg commit -Aqm commit1
+  $ echo 2 > file
+  $ hg amend -Aqm commit1-amended
+  $ tglogm --hidden
+  @  2: f9719601f84a 'commit1-amended'
+  |
+  | x  1: e6c779c67aa9 'commit1'  (Rewritten using amend into f9719601f84a)
+  |/
+  o  0: d20a80d4def3 'base'
+  
+  $ hg log -r 'successors(1)' -T '{node} {desc}\n' --hidden
+  e6c779c67aa947c951f334f4f312bd2b21d27e55 commit1
+  f9719601f84ab527273dc915bfb41704b111058c commit1-amended
+  $ hg log -r 'predecessors(2)' -T '{node} {desc}\n' --hidden
+  e6c779c67aa947c951f334f4f312bd2b21d27e55 commit1
+  f9719601f84ab527273dc915bfb41704b111058c commit1-amended
+
+Set the phase of the obsolete commit to public, simulating the older version being landed.
+  $ hg phase -p 1 --hidden
+
+The commit should no longer show up as amended.
+  $ tglogm --hidden
+  @  2: f9719601f84a 'commit1-amended'
+  |
+  | o  1: e6c779c67aa9 'commit1'
+  |/
+  o  0: d20a80d4def3 'base'
+  
+The predecessor and successor relationship has been removed.
+  $ hg log -r 'successors(1)' -T '{node} {desc}\n' --hidden
+  e6c779c67aa947c951f334f4f312bd2b21d27e55 commit1
+  $ hg log -r 'predecessors(2)' -T '{node} {desc}\n' --hidden
+  f9719601f84ab527273dc915bfb41704b111058c commit1-amended