[PackageGraph] Detect and diagnose cycles in target graph

It cannot be done sooner because `Package` only knows about
package names of products but doesn't attempt to resolve them
and attempting to build resolved packages with cycles would
result in an infinite recusion because builders are reference
types.

Author: xedin

Sources/PackageGraph/ModulesGraph+Loading.swift

@@ -17,6 +17,7 @@ import PackageModel
 
 import struct TSCBasic.KeyedPair
 import func TSCBasic.bestMatch
+import func TSCBasic.findCycle
 
 extension ModulesGraph {
     /// Load the package graph for the given package path.
@@ -638,6 +639,31 @@ private func createResolvedPackages(
         }
     }
 
+    do {
+        let targetBuilders = packageBuilders.flatMap {
+            $0.targets.map {
+                KeyedPair($0, key: $0.target)
+            }
+        }
+        if let cycle = findCycle(targetBuilders, successors: {
+            $0.item.dependencies.flatMap {
+                switch $0 {
+                case .product(let productBuilder, conditions: _):
+                    return productBuilder.targets.map { KeyedPair($0, key: $0.target) }
+                case .target:
+                    return [] // local targets were checked by PackageBuilder.
+                }
+            }
+        }) {
+            observabilityScope.emit(
+                ModuleError.cycleDetected(
+                    (cycle.path.map(\.key.name), cycle.cycle.map(\.key.name))
+                )
+            )
+            return IdentifiableSet()
+        }
+    }
+
     return IdentifiableSet(try packageBuilders.map { try $0.construct() })
 }
 

Tests/PackageGraphTests/ModulesGraphTests.swift

@@ -190,32 +190,77 @@ final class ModulesGraphTests: XCTestCase {
         }
     }
 
-    func testCycle2() throws {
+    func testLocalTargetCycle() throws {
         let fs = InMemoryFileSystem(emptyFiles:
             "/Foo/Sources/Foo/source.swift",
-            "/Bar/Sources/Bar/source.swift",
-            "/Baz/Sources/Baz/source.swift"
+            "/Foo/Sources/Bar/source.swift"
         )
 
         let observability = ObservabilitySystem.makeForTesting()
         _ = try loadModulesGraph(
+            fileSystem: fs,
+            manifests: [
+                Manifest.createRootManifest(
+                    displayName: "Foo",
+                    path: "/Foo",
+                    targets: [
+                        TargetDescription(name: "Foo", dependencies: ["Bar"]),
+                        TargetDescription(name: "Bar", dependencies: ["Foo"])
+                    ]),
+            ],
+            observabilityScope: observability.topScope
+        )
+
+        testDiagnostics(observability.diagnostics) { result in
+            result.check(diagnostic: "cyclic dependency declaration found: Bar -> Foo -> Bar", severity: .error)
+        }
+    }
+
+    func testDependencyCycleWithoutTargetCycle() throws {
+        let fs = InMemoryFileSystem(emptyFiles:
+            "/Foo/Sources/Foo/source.swift",
+            "/Bar/Sources/Bar/source.swift",
+            "/Bar/Sources/Baz/source.swift"
+        )
+
+        let observability = ObservabilitySystem.makeForTesting()
+        let graph = try loadModulesGraph(
             fileSystem: fs,
             manifests: [
                 Manifest.createRootManifest(
                     displayName: "Foo",
                     path: "/Foo",
                     dependencies: [
-                        .localSourceControl(path: "/Foo", requirement: .upToNextMajor(from: "1.0.0"))
+                        .localSourceControl(path: "/Bar", requirement: .upToNextMajor(from: "1.0.0"))
+                    ],
+                    products: [
+                        ProductDescription(name: "Foo", type: .library(.automatic), targets: ["Foo"])
                     ],
                     targets: [
-                        TargetDescription(name: "Foo"),
+                        TargetDescription(name: "Foo", dependencies: ["Bar"]),
                     ]),
+                Manifest.createFileSystemManifest(
+                    displayName: "Bar",
+                    path: "/Bar",
+                    dependencies: [
+                        .localSourceControl(path: "/Foo", requirement: .upToNextMajor(from: "1.0.0"))
+                    ],
+                    products: [
+                        ProductDescription(name: "Bar", type: .library(.automatic), targets: ["Bar"]),
+                        ProductDescription(name: "Baz", type: .library(.automatic), targets: ["Baz"])
+                    ],
+                    targets: [
+                        TargetDescription(name: "Bar"),
+                        TargetDescription(name: "Baz", dependencies: ["Foo"]),
+                    ])
             ],
             observabilityScope: observability.topScope
         )
 
-        testDiagnostics(observability.diagnostics) { result in
-            result.check(diagnostic: "cyclic dependency declaration found: Foo -> Foo", severity: .error)
+        XCTAssertNoDiagnostics(observability.diagnostics)
+        PackageGraphTester(graph) { result in
+            result.check(packages: "Foo", "Bar")
+            result.check(targets: "Bar", "Baz", "Foo")
         }
     }
 

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -12019,7 +12019,7 @@ final class WorkspaceTests: XCTestCase {
                     targets: [
                         .init(name: "Root1Target", dependencies: [
                             .product(name: "FooProduct", package: "foo"),
-                            .product(name: "Root2Target", package: "Root2")
+                            .product(name: "Root2Product", package: "Root2")
                         ]),
                     ],
                     products: [
@@ -12115,7 +12115,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] -> *"),
+                    diagnostic: .regex("cyclic dependency declaration found: Root[1|2]Target -> *"),
                     severity: .error
                 )
             }