[Workspace] Switch manifest loading to use DFS instead of findCycles+topologicalSort

It's too early to diagnose cycles here because cyclic package-level
dependencies don't necessary lead to cycles in build graph, only
target/product element cycles are important.

Author: xedin

Sources/Workspace/Workspace+Manifests.swift

@@ -16,6 +16,7 @@ import struct Basics.InternalError
 import class Basics.ObservabilityScope
 import struct Basics.SwiftVersion
 import func Basics.temp_await
+import func Basics.DFS
 import class Basics.ThreadSafeKeyValueStore
 import class Dispatch.DispatchGroup
 import struct Dispatch.DispatchTime
@@ -494,12 +495,7 @@ extension Workspace {
         // Continue to load the rest of the manifest for this graph
         // Creates a map of loaded manifests. We do this to avoid reloading the shared nodes.
         var loadedManifests = firstLevelManifests
-        // Compute the transitive closure of available dependencies.
-        let topologicalSortInput = topLevelManifests.map { identity, manifest in KeyedPair(
-            manifest,
-            key: Key(identity: identity, productFilter: .everything)
-        ) }
-        let topologicalSortSuccessors: (KeyedPair<Manifest, Key>) throws -> [KeyedPair<Manifest, Key>] = { pair in
+        let successorManifests: (KeyedPair<Manifest, Key>) throws -> [KeyedPair<Manifest, Key>] = { pair in
             // optimization: preload manifest we know about in parallel
             let dependenciesRequired = pair.item.dependenciesRequired(for: pair.key.productFilter)
             let dependenciesToLoad = dependenciesRequired.map(\.packageRef)
@@ -527,36 +523,26 @@ extension Workspace {
             }
         }
 
-        // Look for any cycle in the dependencies.
-        if let cycle = try findCycle(topologicalSortInput, successors: topologicalSortSuccessors) {
-            observabilityScope.emit(
-                error: "cyclic dependency declaration found: " +
-                    (cycle.path + cycle.cycle).map(\.key.identity.description).joined(separator: " -> ") +
-                    " -> " + cycle.cycle[0].key.identity.description
-            )
-            // return partial results
-            return DependencyManifests(
-                root: root,
-                dependencies: [],
-                workspace: self,
-                observabilityScope: observabilityScope
-            )
-        }
-        let allManifestsWithPossibleDuplicates = try topologicalSort(
-            topologicalSortInput,
-            successors: topologicalSortSuccessors
-        )
-
-        // merge the productFilter of the same package (by identity)
-        var deduplication = [PackageIdentity: Int]()
         var allManifests = [(identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter)]()
-        for pair in allManifestsWithPossibleDuplicates {
-            if let index = deduplication[pair.key.identity] {
-                let productFilter = allManifests[index].productFilter.merge(pair.key.productFilter)
-                allManifests[index] = (pair.key.identity, pair.item, productFilter)
-            } else {
+        do {
+            let manifestGraphRoots = topLevelManifests.map { identity, manifest in
+                KeyedPair(
+                    manifest,
+                    key: Key(identity: identity, productFilter: .everything)
+                )
+            }
+
+            var deduplication = [PackageIdentity: Int]()
+            try DFS(
+                manifestGraphRoots,
+                successors: successorManifests
+            ) { pair in
                 deduplication[pair.key.identity] = allManifests.count
                 allManifests.append((pair.key.identity, pair.item, pair.key.productFilter))
+            } onDuplicate: { old, new in
+                let index = deduplication[old.key.identity]!
+                let productFilter = allManifests[index].productFilter.merge(new.key.productFilter)
+                allManifests[index] = (new.key.identity, new.item, productFilter)
             }
         }
 

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -1971,7 +1971,7 @@ final class WorkspaceTests: XCTestCase {
             // Ensure that the order of the manifests is stable.
             XCTAssertEqual(
                 manifests.allDependencyManifests.map(\.value.manifest.displayName),
-                ["Foo", "Baz", "Bam", "Bar"]
+                ["Foo", "Bar", "Baz", "Bam"]
             )
             XCTAssertNoDiagnostics(diagnostics)
         }
@@ -12111,15 +12111,7 @@ final class WorkspaceTests: XCTestCase {
         try workspace.checkPackageGraph(roots: ["Root1", "Root2"]) { _, diagnostics in
             testDiagnostics(diagnostics) { result in
                 result.check(
-                    diagnostic: .regex("cyclic dependency declaration found: root[1|2] -> *"),
-                    severity: .error
-                )
-                result.check(
-                    diagnostic: """
-                    exhausted attempts to resolve the dependencies graph, with the following dependencies unresolved:
-                    * 'bar' from http://scm.com/org/bar
-                    * 'foo' from http://scm.com/org/foo
-                    """,
+                    diagnostic: .regex("cyclic dependency declaration found: Root[1|2] -> *"),
                     severity: .error
                 )
             }