[Package/ModuleGraph] Allow cyclic package dependencies if they don't introduce a cycle in a build graph (swiftlang#7530)

### Motivation:

It should be possible for packages to depend on each other if such
dependence doesn't introduce cycles in the build graph.

### Modifications:

- Introduced a new DFS method to walk over graphs that breaks cycles.
- Replaces use of `findCycle` + `topologicalSort` with `DFS` while
building manifest and package graphs. This allows cycles in dependencies
to be modeled correctly.
- Removes some of the redundant data transformations from modules graph.
- Modifies `ResolvedPackage` to carry identities of its dependencies
instead of resolved dependencies themselves. This helps to simplify
logic in `createResolvedPackages`.
- Adds detection of target cycles across packages.

### Result:

Makes it possible for package A to depend on package B and B to depend
on A if their targets don't form a cycle.

Author: xedin

CHANGELOG.md

@@ -1,5 +1,11 @@
 Note: This is in reverse chronological order, so newer entries are added to the top.
 
+* [#7530]
+
+  Makes it possible for packages to depend on each other if such dependency doesn't form any target-level cycles. For example,
+  package `A` can depend on `B` and `B` on `A` unless targets in `B` depend on products of `A` that depend on some of the same
+  targets from `B` and vice versa.
+
 Swift 6.0
 -----------
 

Sources/Basics/CMakeLists.txt

@@ -36,6 +36,7 @@ add_library(Basics
   FileSystem/VFSOverlay.swift
   Graph/AdjacencyMatrix.swift
   Graph/DirectedGraph.swift
+  Graph/GraphAlgorithms.swift
   Graph/UndirectedGraph.swift
   SourceControlURL.swift
   HTTPClient/HTTPClient.swift

Sources/Basics/Graph/GraphAlgorithms.swift

@@ -0,0 +1,57 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift open source project
+//
+// Copyright (c) 2015-2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See http://swift.org/LICENSE.txt for license information
+// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import struct OrderedCollections.OrderedSet
+
+/// Implements a pre-order depth-first search.
+///
+/// The cycles are handled by skipping cycle points but it should be possible to
+/// to extend this in the future to provide a callback for every cycle.
+///
+/// - Parameters:
+///   - nodes: The list of input nodes to sort.
+///   - successors: A closure for fetching the successors of a particular node.
+///   - onUnique: A callback to indicate the the given node is being processed for the first time.
+///   - onDuplicate: A callback to indicate that the node was already processed at least once.
+///
+/// - Complexity: O(v + e) where (v, e) are the number of vertices and edges
+/// reachable from the input nodes via the relation.
+public func depthFirstSearch<T: Hashable>(
+    _ nodes: [T],
+    successors: (T) throws -> [T],
+    onUnique: (T) -> Void,
+    onDuplicate: (T, T) -> Void
+) rethrows {
+    var stack = OrderedSet<T>()
+    var visited = Set<T>()
+
+    for node in nodes {
+        precondition(stack.isEmpty)
+        stack.append(node)
+
+        while !stack.isEmpty {
+            let curr = stack.removeLast()
+
+            let visitResult = visited.insert(curr)
+            if visitResult.inserted {
+                onUnique(curr)
+            } else {
+                onDuplicate(visitResult.memberAfterInsert, curr)
+                continue
+            }
+
+            for succ in try successors(curr) {
+                stack.append(succ)
+            }
+        }
+    }
+}

Sources/Commands/PackageCommands/CompletionCommand.swift

@@ -68,6 +68,7 @@ extension SwiftPackageCommand {
                 // command's result output goes on stdout
                 // ie "swift package list-dependencies" should output to stdout
                 ShowDependencies.dumpDependenciesOf(
+                    graph: graph,
                     rootPackage: graph.rootPackages[graph.rootPackages.startIndex],
                     mode: .flatlist,
                     on: TSCBasic.stdoutStream

Sources/Commands/PackageCommands/PluginCommand.swift

@@ -371,6 +371,7 @@ struct PluginCommand: SwiftCommand {
             pkgConfigDirectories: swiftCommandState.options.locations.pkgConfigDirectories,
             sdkRootPath: buildParameters.toolchain.sdkRootPath,
             fileSystem: swiftCommandState.fileSystem,
+            modulesGraph: packageGraph,
             observabilityScope: swiftCommandState.observabilityScope,
             callbackQueue: delegateQueue,
             delegate: pluginDelegate,
@@ -382,10 +383,17 @@ struct PluginCommand: SwiftCommand {
 
     static func availableCommandPlugins(in graph: ModulesGraph, limitedTo packageIdentity: String?) -> [PluginTarget] {
         // All targets from plugin products of direct dependencies are "available".
-        let directDependencyPackages = graph.rootPackages.flatMap { $0.dependencies }.filter { $0.matching(identity: packageIdentity) }
+        let directDependencyPackages = graph.rootPackages.flatMap {
+            $0.dependencies
+        }.filter {
+            $0.matching(identity: packageIdentity)
+        }.compactMap {
+            graph.package(for: $0)
+        }
+
         let directDependencyPluginTargets = directDependencyPackages.flatMap { $0.products.filter { $0.type == .plugin } }.flatMap { $0.targets }
         // As well as any plugin targets in root packages.
-        let rootPackageTargets = graph.rootPackages.filter { $0.matching(identity: packageIdentity) }.flatMap { $0.targets }
+        let rootPackageTargets = graph.rootPackages.filter { $0.identity.matching(identity: packageIdentity) }.flatMap { $0.targets }
         return (directDependencyPluginTargets + rootPackageTargets).compactMap { $0.underlying as? PluginTarget }.filter {
             switch $0.capability {
             case .buildTool: return false
@@ -469,10 +477,10 @@ extension SandboxNetworkPermission {
     }
 }
 
-extension ResolvedPackage {
+extension PackageIdentity {
     fileprivate func matching(identity: String?) -> Bool {
         if let identity {
-            return self.identity == .plain(identity)
+            return self == .plain(identity)
         } else {
             return true
         }

Sources/Commands/PackageCommands/ShowDependencies.swift

@@ -43,13 +43,19 @@ extension SwiftPackageCommand {
             // ie "swift package show-dependencies" should output to stdout
             let stream: OutputByteStream = try outputPath.map { try LocalFileOutputByteStream($0) } ?? TSCBasic.stdoutStream
             Self.dumpDependenciesOf(
+                graph: graph,
                 rootPackage: graph.rootPackages[graph.rootPackages.startIndex],
                 mode: format,
                 on: stream
             )
         }
 
-        static func dumpDependenciesOf(rootPackage: ResolvedPackage, mode: ShowDependenciesMode, on stream: OutputByteStream) {
+        static func dumpDependenciesOf(
+            graph: ModulesGraph,
+            rootPackage: ResolvedPackage,
+            mode: ShowDependenciesMode,
+            on stream: OutputByteStream
+        ) {
             let dumper: DependenciesDumper
             switch mode {
             case .text:
@@ -61,7 +67,7 @@ extension SwiftPackageCommand {
             case .flatlist:
                 dumper = FlatListDumper()
             }
-            dumper.dump(dependenciesOf: rootPackage, on: stream)
+            dumper.dump(graph: graph, dependenciesOf: rootPackage, on: stream)
             stream.flush()
         }
 

Sources/Commands/Utilities/DependenciesSerializer.swift

@@ -17,11 +17,11 @@ import enum TSCBasic.JSON
 import protocol TSCBasic.OutputByteStream
 
 protocol DependenciesDumper {
-    func dump(dependenciesOf: ResolvedPackage, on: OutputByteStream)
+    func dump(graph: ModulesGraph, dependenciesOf: ResolvedPackage, on: OutputByteStream)
 }
 
 final class PlainTextDumper: DependenciesDumper {
-    func dump(dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
+    func dump(graph: ModulesGraph, dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
         func recursiveWalk(packages: [ResolvedPackage], prefix: String = "") {
             var hanger = prefix + "├── "
 
@@ -39,38 +39,38 @@ final class PlainTextDumper: DependenciesDumper {
                     var childPrefix = hanger
                     let startIndex = childPrefix.index(childPrefix.endIndex, offsetBy: -4)
                     childPrefix.replaceSubrange(startIndex..<childPrefix.endIndex, with: replacement)
-                    recursiveWalk(packages: package.dependencies, prefix: childPrefix)
+                    recursiveWalk(packages: graph.directDependencies(for: package), prefix: childPrefix)
                 }
             }
         }
 
         if !rootpkg.dependencies.isEmpty {
             stream.send(".\n")
-            recursiveWalk(packages: rootpkg.dependencies)
+            recursiveWalk(packages: graph.directDependencies(for: rootpkg))
         } else {
             stream.send("No external dependencies found\n")
         }
     }
 }
 
 final class FlatListDumper: DependenciesDumper {
-    func dump(dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
+    func dump(graph: ModulesGraph, dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
         func recursiveWalk(packages: [ResolvedPackage]) {
             for package in packages {
                 stream.send(package.identity.description).send("\n")
                 if !package.dependencies.isEmpty {
-                    recursiveWalk(packages: package.dependencies)
+                    recursiveWalk(packages: graph.directDependencies(for: package))
                 }
             }
         }
         if !rootpkg.dependencies.isEmpty {
-            recursiveWalk(packages: rootpkg.dependencies)
+            recursiveWalk(packages: graph.directDependencies(for: rootpkg))
         }
     }
 }
 
 final class DotDumper: DependenciesDumper {
-    func dump(dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
+    func dump(graph: ModulesGraph, dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
         var nodesAlreadyPrinted: Set<String> = []
         func printNode(_ package: ResolvedPackage) {
             let url = package.manifest.packageLocation
@@ -87,7 +87,7 @@ final class DotDumper: DependenciesDumper {
         var dependenciesAlreadyPrinted: Set<DependencyURLs> = []
         func recursiveWalk(rootpkg: ResolvedPackage) {
             printNode(rootpkg)
-            for dependency in rootpkg.dependencies {
+            for dependency in graph.directDependencies(for: rootpkg) {
                 let rootURL = rootpkg.manifest.packageLocation
                 let dependencyURL = dependency.manifest.packageLocation
                 let urlPair = DependencyURLs(root: rootURL, dependency: dependencyURL)
@@ -120,15 +120,15 @@ final class DotDumper: DependenciesDumper {
 }
 
 final class JSONDumper: DependenciesDumper {
-    func dump(dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
+    func dump(graph: ModulesGraph, dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
         func convert(_ package: ResolvedPackage) -> JSON {
             return .orderedDictionary([
                 "identity": .string(package.identity.description),
                 "name": .string(package.manifest.displayName), // TODO: remove?
                 "url": .string(package.manifest.packageLocation),
                 "version": .string(package.manifest.version?.description ?? "unspecified"),
                 "path": .string(package.path.pathString),
-                "dependencies": .array(package.dependencies.map(convert)),
+                "dependencies": .array(package.dependencies.compactMap { graph.packages[$0] }.map(convert)),
             ])
         }