Merge pull request #1186 from kevinhendricks/master

updates to the _find_lcas routine for improved efficiency and robustness

dulwich/graph.py

@@ -32,50 +32,55 @@ def _find_lcas(lookup_parents, c1, c2s):
     _ANC_OF_1 = 1  # ancestor of commit 1
     _ANC_OF_2 = 2  # ancestor of commit 2
     _DNC = 4  # Do Not Consider
-    _LCA = 8  # potential LCA
+    _LCA = 8  # potential LCA (Lowest Common Ancestor)
 
     def _has_candidates(wlst, cstates):
         for cmt in wlst:
             if cmt in cstates:
-                if not (cstates[cmt] & _DNC):
+                if not ((cstates[cmt] & _DNC) == _DNC):
                     return True
         return False
 
-    # initialize the working list
-    wlst: Deque[int] = deque()
+    # initialize the working list states with ancestry info
+    # note possibility of c1 being one of c2s should be handled
+    wlst: Deque[bytes] = deque()
     cstates[c1] = _ANC_OF_1
     wlst.append(c1)
     for c2 in c2s:
-        cstates[c2] = _ANC_OF_2
+        cflags = cstates.get(c2, 0)
+        cstates[c2] = cflags | _ANC_OF_2
         wlst.append(c2)
-
-    # loop until no other LCA candidates are viable in working list
+    
+    # loop while at least one working list commit is still viable (not marked as _DNC)
     # adding any parents to the list in a breadth first manner
     while _has_candidates(wlst, cstates):
         cmt = wlst.popleft()
-        flags = cstates[cmt]
-        if flags == (_ANC_OF_1 | _ANC_OF_2):
-            # potential common ancestor
-            if not (flags & _LCA):
-                flags = flags | _LCA
-                cstates[cmt] = flags
+        # Look only at ANCESTRY and _DNC flags so that already
+        # found _LCAs can still be marked _DNC by lower _LCAS
+        cflags = cstates[cmt] & (_ANC_OF_1 | _ANC_OF_2 | _DNC)
+        if cflags == (_ANC_OF_1 | _ANC_OF_2):
+            # potential common ancestor if not already in candidates add it
+            if not (cstates[cmt] & _LCA) == _LCA:
+                cstates[cmt] = cstates[cmt] | _LCA
                 cands.append(cmt)
-                # mark any parents of this node _DNC as all parents
-                # would be one level further removed common ancestors
-                flags = flags | _DNC
+            # mark any parents of this node _DNC as all parents
+            # would be one generation further removed common ancestors
+            cflags = cflags | _DNC
         parents = lookup_parents(cmt)
         if parents:
             for pcmt in parents:
-                if pcmt in cstates:
-                    cstates[pcmt] = cstates[pcmt] | flags
-                else:
-                    cstates[pcmt] = flags
+                pflags = cstates.get(pcmt, 0)
+                # if this parent was already visited with no new ancestry/flag information
+                # do not add it to the working list again
+                if ((pflags & cflags) == cflags):
+                    continue
+                cstates[pcmt] = pflags | cflags
                 wlst.append(pcmt)
 
-    # walk final candidates removing any superseded by _DNC by later lower LCAs
+    # walk final candidates removing any superseded by _DNC by later lower _LCAs
     results = []
     for cmt in cands:
-        if not (cstates[cmt] & _DNC):
+        if not ((cstates[cmt] & _DNC) == _DNC):
             results.append(cmt)
     return results