Merge pull request #670 from aciidb0mb3r/package-builder-fixes

Package builder fixes

Author: aciidgh

Sources/PackageLoading/PackageBuilder.swift

@@ -34,6 +34,9 @@ public enum ModuleError: Swift.Error {
 
     /// The manifest has invalid configuration wrt type of the module.
     case invalidManifestConfig(String, String)
+
+    /// The target dependency declaration has cycle in it.
+    case cycleDetected((path: [Module], cycle: [Module]))
 }
 
 extension ModuleError: FixableError {
@@ -47,6 +50,10 @@ extension ModuleError: FixableError {
             return "the target \(module) cannot have the executable \(dependency) as a dependency"
         case .invalidManifestConfig(let package, let message):
             return "invalid configuration in '\(package)': \(message)"
+        case .cycleDetected(let cycle):
+            return "found cyclic dependency declaration: " +
+                (cycle.path + cycle.cycle).map{$0.name}.joined(separator: " -> ") +
+                " -> " + cycle.cycle[0].name
         }
     }
 
@@ -60,6 +67,8 @@ extension ModuleError: FixableError {
             return "move the shared logic inside a library, which can be referenced from both the target and the executable"
         case .invalidManifestConfig(_):
             return nil
+        case .cycleDetected(_):
+            return nil
         }
     }
 }
@@ -452,6 +461,11 @@ public struct PackageBuilder {
                 module.dependencies = [baseModule]
             }
         }
+
+        // Look for any cycle in the dependencies.
+        if let cycle = findCycle(modules.sorted { $0.name < $1.name }, successors: { $0.dependencies}) {
+            throw ModuleError.cycleDetected(cycle)
+        }
     }
 
     /// Private function that checks whether a module name is valid.  This method doesn't return anything, but rather, if there's a problem, it throws an error describing what the problem is.

Tests/PackageLoadingTests/ConventionTests.swift

@@ -579,6 +579,36 @@ class ConventionTests: XCTestCase {
             result.checkDiagnostic("these referenced modules could not be found: Foo fix: reference only valid modules")
         }
 
+        // Reference self in dependencies.
+        package = PackageDescription.Package(name: "pkg", targets: [Target(name: "pkg", dependencies: ["pkg"])])
+        PackageBuilderTester(package, in: fs) { result in
+            result.checkDiagnostic("found cyclic dependency declaration: pkg -> pkg")
+        }
+
+        fs = InMemoryFileSystem(emptyFiles:
+            "/Sources/pkg1/Foo.swift",
+            "/Sources/pkg2/Foo.swift",
+            "/Sources/pkg3/Foo.swift"
+        )
+        // Cyclic dependency.
+        package = PackageDescription.Package(name: "pkg", targets: [
+            Target(name: "pkg1", dependencies: ["pkg2"]),
+            Target(name: "pkg2", dependencies: ["pkg3"]),
+            Target(name: "pkg3", dependencies: ["pkg1"]),
+        ])
+        PackageBuilderTester(package, in: fs) { result in
+            result.checkDiagnostic("found cyclic dependency declaration: pkg1 -> pkg2 -> pkg3 -> pkg1")
+        }
+
+        package = PackageDescription.Package(name: "pkg", targets: [
+            Target(name: "pkg1", dependencies: ["pkg2"]),
+            Target(name: "pkg2", dependencies: ["pkg3"]),
+            Target(name: "pkg3", dependencies: ["pkg2"]),
+        ])
+        PackageBuilderTester(package, in: fs) { result in
+            result.checkDiagnostic("found cyclic dependency declaration: pkg1 -> pkg2 -> pkg3 -> pkg2")
+        }
+
         // Executable as dependency.
         // FIXME: maybe should support this and condiser it as build order dependency.
         fs = InMemoryFileSystem(emptyFiles: