Properly compute module load order

lib/internal/Magento/Framework/Data/Graph.php

@@ -76,6 +76,26 @@ public function addRelation($fromNode, $toNode)
         return $this;
     }
 
+    /**
+     * Unset a relation between nodes
+     *
+     * @param string|int $fromNode
+     * @param string|int $toNode
+     * @return $this
+     * @throws \InvalidArgumentException
+     */
+    public function removeRelation($fromNode, $toNode)
+    {
+        if ($fromNode == $toNode) {
+            throw new \InvalidArgumentException("Graph node '{$fromNode}' is linked to itself.");
+        }
+        $this->_assertNode($fromNode, true);
+        $this->_assertNode($toNode, true);
+        unset($this->_from[$fromNode][$toNode]);
+        unset($this->_to[$toNode][$fromNode]);
+        return $this;
+    }
+
     /**
      * Export relations between nodes. Can return inverse relations
      *
@@ -181,6 +201,75 @@ public function dfs($fromNode, $toNode, $mode = self::DIRECTIONAL)
         return $this->_dfs($fromNode, $toNode, $this->getRelations($mode));
     }
 
+    /**
+     * Perform a topological sort on the graph. This algorithm is not possible if the graph contains a cycle.
+     *
+     * @return array
+     * @throws \Exception
+     */
+    public function topoSort()
+    {
+        $sortedElements = [];
+        $nodeSet = $this->findStartNodes();
+
+        while (!empty($nodeSet)) {
+            $cur = array_pop($nodeSet);
+            $sortedElements[] = $cur;
+            if (!isset($this->_from[$cur])) {
+                continue;
+            }
+            foreach ($this->_from[$cur] as $parent) {
+                $this->removeRelation($cur, $parent);
+                if (empty($this->_to[$parent])) {
+                    $nodeSet[] = $parent;
+                }
+            }
+        }
+
+        if ($this->hasEdges()) {
+            $cycle = $this->findCycle();
+            $message = "Dependency cycle detected: " . implode(" -> ", $cycle);
+            throw new \Exception($message);
+        }
+
+        return array_reverse($sortedElements);
+    }
+
+    /**
+     * Determines whether the graph has edges.
+     *
+     * Returns true if the graph has edges. Returns false otherwise.
+     *
+     * @return bool
+     */
+    public function hasEdges()
+    {
+        foreach ($this->_from as $from) {
+            if (count($from) > 0) {
+                return true;
+            }
+        }
+
+        foreach ($this->_to as $to) {
+            if (count($to) > 0) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Finds all graph nodes with no incoming edge
+     *
+     * @return array
+     */
+    public function findStartNodes()
+    {
+        $incomingEdgeNodes = array_keys($this->_to);
+        return array_diff($this->_nodes, $incomingEdgeNodes);
+    }
+
     /**
      * Recursive sub-routine of dfs()
      *

lib/internal/Magento/Framework/Module/ModuleList/Loader.php

@@ -126,62 +126,30 @@ private function getModuleConfigs()
      *
      * @param array $origList
      * @return array
-     * @SuppressWarnings(PHPMD.UnusedLocalVariable)
      */
     private function sortBySequence($origList)
     {
-        ksort($origList);
-        $expanded = [];
-        foreach ($origList as $moduleName => $value) {
-            $expanded[] = [
-                'name' => $moduleName,
-                'sequence' => $this->expandSequence($origList, $moduleName),
-            ];
-        }
+        $nodes = [];
+        $relations = [];
 
-        // Use "bubble sorting" because usort does not check each pair of elements and in this case it is important
-        $total = count($expanded);
-        for ($i = 0; $i < $total - 1; $i++) {
-            for ($j = $i; $j < $total; $j++) {
-                if (in_array($expanded[$j]['name'], $expanded[$i]['sequence'])) {
-                    $temp = $expanded[$i];
-                    $expanded[$i] = $expanded[$j];
-                    $expanded[$j] = $temp;
-                }
+        // collect graph nodes and edges
+        foreach ($origList as $module) {
+            $nodes[] = $module['name'];
+            foreach ($module['sequence'] as $dependency) {
+                $relations[] = [$module['name'], $dependency];
             }
         }
 
+        // construct a graph and perform topological sort on it
+        $moduleGraph = new \Magento\Framework\Data\Graph($nodes, $relations);
+        $sortedResult = $moduleGraph->topoSort();
+
+        // re-attach version information to the linear ordering
         $result = [];
-        foreach ($expanded as $pair) {
-            $result[$pair['name']] = $origList[$pair['name']];
+        foreach ($sortedResult as $item) {
+            $result[$item] = $origList[$item];
         }
 
         return $result;
     }
-
-    /**
-     * Accumulate information about all transitive "sequence" references
-     *
-     * @param array $list
-     * @param string $name
-     * @param array $accumulated
-     * @return array
-     * @throws \Exception
-     */
-    private function expandSequence($list, $name, $accumulated = [])
-    {
-        $accumulated[] = $name;
-        $result = $list[$name]['sequence'];
-        foreach ($result as $relatedName) {
-            if (in_array($relatedName, $accumulated)) {
-                throw new \Exception("Circular sequence reference from '{$name}' to '{$relatedName}'.");
-            }
-            if (!isset($list[$relatedName])) {
-                continue;
-            }
-            $relatedResult = $this->expandSequence($list, $relatedName, $accumulated);
-            $result = array_unique(array_merge($result, $relatedResult));
-        }
-        return $result;
-    }
 }

lib/internal/Magento/Framework/Module/Test/Unit/ModuleList/LoaderTest.php

@@ -143,7 +143,7 @@ public function testLoadExclude()
 
     /**
      * @expectedException \Exception
-     * @expectedExceptionMessage Circular sequence reference from 'b' to 'a'
+     * @expectedExceptionMessage Dependency cycle detected: a -> b -> a
      */
     public function testLoadCircular()
     {