Skip to content

Commit 550650b

Browse files
xedinxedin
committed
[Package/ModuleGraph] Allow cyclic package dependencies if they don't introduce a cycle in a build graph (swiftlang#7530)
It should be possible for packages to depend on each other if such dependence doesn't introduce cycles in the build graph. - 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. 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. (cherry picked from commit 8b12909)
1 parent eb3b9c6 commit 550650b

20 files changed

+277
-200
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@ Note: This is in reverse chronological order, so newer entries are added to the
33
Swift 6.0
44
-----------
55

6+
* [#7530]
7+
8+
Makes it possible for packages to depend on each other if such dependency doesn't form any target-level cycles. For example,
9+
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
10+
targets from `B` and vice versa.
11+
612
* [#7507]
713

814
`swift experimental-sdk` command is deprecated with `swift sdk` command replacing it. `--experimental-swift-sdk` and

‎Sources/Basics/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ add_library(Basics
3434
FileSystem/TemporaryFile.swift
3535
FileSystem/TSCAdapters.swift
3636
FileSystem/VFSOverlay.swift
37+
Graph/GraphAlgorithms.swift
3738
SourceControlURL.swift
3839
HTTPClient/HTTPClient.swift
3940
HTTPClient/HTTPClientConfiguration.swift

‎Sources/Basics/Graph/GraphAlgorithms.swift

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift open source project
4+
//
5+
// Copyright (c) 2015-2024 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See http://swift.org/LICENSE.txt for license information
9+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import struct OrderedCollections.OrderedSet
14+
15+
/// Implements a pre-order depth-first search.
16+
///
17+
/// The cycles are handled by skipping cycle points but it should be possible to
18+
/// to extend this in the future to provide a callback for every cycle.
19+
///
20+
/// - Parameters:
21+
/// - nodes: The list of input nodes to sort.
22+
/// - successors: A closure for fetching the successors of a particular node.
23+
/// - onUnique: A callback to indicate the the given node is being processed for the first time.
24+
/// - onDuplicate: A callback to indicate that the node was already processed at least once.
25+
///
26+
/// - Complexity: O(v + e) where (v, e) are the number of vertices and edges
27+
/// reachable from the input nodes via the relation.
28+
public func depthFirstSearch<T: Hashable>(
29+
_ nodes: [T],
30+
successors: (T) throws -> [T],
31+
onUnique: (T) -> Void,
32+
onDuplicate: (T, T) -> Void
33+
) rethrows {
34+
var stack = OrderedSet<T>()
35+
var visited = Set<T>()
36+
37+
for node in nodes {
38+
precondition(stack.isEmpty)
39+
stack.append(node)
40+
41+
while !stack.isEmpty {
42+
let curr = stack.removeLast()
43+
44+
let visitResult = visited.insert(curr)
45+
if visitResult.inserted {
46+
onUnique(curr)
47+
} else {
48+
onDuplicate(visitResult.memberAfterInsert, curr)
49+
continue
50+
}
51+
52+
for succ in try successors(curr) {
53+
stack.append(succ)
54+
}
55+
}
56+
}
57+
}

‎Sources/Commands/PackageCommands/CompletionCommand.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ extension SwiftPackageCommand {
6868
// command's result output goes on stdout
6969
// ie "swift package list-dependencies" should output to stdout
7070
ShowDependencies.dumpDependenciesOf(
71+
graph: graph,
7172
rootPackage: graph.rootPackages[graph.rootPackages.startIndex],
7273
mode: .flatlist,
7374
on: TSCBasic.stdoutStream

‎Sources/Commands/PackageCommands/PluginCommand.swift

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ struct PluginCommand: SwiftCommand {
360360
pkgConfigDirectories: swiftCommandState.options.locations.pkgConfigDirectories,
361361
sdkRootPath: buildParameters.toolchain.sdkRootPath,
362362
fileSystem: swiftCommandState.fileSystem,
363+
modulesGraph: packageGraph,
363364
observabilityScope: swiftCommandState.observabilityScope,
364365
callbackQueue: delegateQueue,
365366
delegate: pluginDelegate,
@@ -371,10 +372,17 @@ struct PluginCommand: SwiftCommand {
371372

372373
static func availableCommandPlugins(in graph: ModulesGraph, limitedTo packageIdentity: String?) -> [PluginTarget] {
373374
// All targets from plugin products of direct dependencies are "available".
374-
let directDependencyPackages = graph.rootPackages.flatMap { $0.dependencies }.filter { $0.matching(identity: packageIdentity) }
375+
let directDependencyPackages = graph.rootPackages.flatMap {
376+
$0.dependencies
377+
}.filter {
378+
$0.matching(identity: packageIdentity)
379+
}.compactMap {
380+
graph.package(for: $0)
381+
}
382+
375383
let directDependencyPluginTargets = directDependencyPackages.flatMap { $0.products.filter { $0.type == .plugin } }.flatMap { $0.targets }
376384
// As well as any plugin targets in root packages.
377-
let rootPackageTargets = graph.rootPackages.filter { $0.matching(identity: packageIdentity) }.flatMap { $0.targets }
385+
let rootPackageTargets = graph.rootPackages.filter { $0.identity.matching(identity: packageIdentity) }.flatMap { $0.targets }
378386
return (directDependencyPluginTargets + rootPackageTargets).compactMap { $0.underlying as? PluginTarget }.filter {
379387
switch $0.capability {
380388
case .buildTool: return false
@@ -458,10 +466,10 @@ extension SandboxNetworkPermission {
458466
}
459467
}
460468

461-
extension ResolvedPackage {
469+
extension PackageIdentity {
462470
fileprivate func matching(identity: String?) -> Bool {
463471
if let identity {
464-
return self.identity == .plain(identity)
472+
return self == .plain(identity)
465473
} else {
466474
return true
467475
}

‎Sources/Commands/PackageCommands/ShowDependencies.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,19 @@ extension SwiftPackageCommand {
4141
// ie "swift package show-dependencies" should output to stdout
4242
let stream: OutputByteStream = try outputPath.map { try LocalFileOutputByteStream($0) } ?? TSCBasic.stdoutStream
4343
Self.dumpDependenciesOf(
44+
graph: graph,
4445
rootPackage: graph.rootPackages[graph.rootPackages.startIndex],
4546
mode: format,
4647
on: stream
4748
)
4849
}
4950

50-
static func dumpDependenciesOf(rootPackage: ResolvedPackage, mode: ShowDependenciesMode, on stream: OutputByteStream) {
51+
static func dumpDependenciesOf(
52+
graph: ModulesGraph,
53+
rootPackage: ResolvedPackage,
54+
mode: ShowDependenciesMode,
55+
on stream: OutputByteStream
56+
) {
5157
let dumper: DependenciesDumper
5258
switch mode {
5359
case .text:
@@ -59,7 +65,7 @@ extension SwiftPackageCommand {
5965
case .flatlist:
6066
dumper = FlatListDumper()
6167
}
62-
dumper.dump(dependenciesOf: rootPackage, on: stream)
68+
dumper.dump(graph: graph, dependenciesOf: rootPackage, on: stream)
6369
stream.flush()
6470
}
6571

‎Sources/Commands/Utilities/DependenciesSerializer.swift

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ import enum TSCBasic.JSON
1717
import protocol TSCBasic.OutputByteStream
1818

1919
protocol DependenciesDumper {
20-
func dump(dependenciesOf: ResolvedPackage, on: OutputByteStream)
20+
func dump(graph: ModulesGraph, dependenciesOf: ResolvedPackage, on: OutputByteStream)
2121
}
2222

2323
final class PlainTextDumper: DependenciesDumper {
24-
func dump(dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
24+
func dump(graph: ModulesGraph, dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
2525
func recursiveWalk(packages: [ResolvedPackage], prefix: String = "") {
2626
var hanger = prefix + "├── "
2727

@@ -39,38 +39,38 @@ final class PlainTextDumper: DependenciesDumper {
3939
var childPrefix = hanger
4040
let startIndex = childPrefix.index(childPrefix.endIndex, offsetBy: -4)
4141
childPrefix.replaceSubrange(startIndex..<childPrefix.endIndex, with: replacement)
42-
recursiveWalk(packages: package.dependencies, prefix: childPrefix)
42+
recursiveWalk(packages: graph.directDependencies(for: package), prefix: childPrefix)
4343
}
4444
}
4545
}
4646

4747
if !rootpkg.dependencies.isEmpty {
4848
stream.send(".\n")
49-
recursiveWalk(packages: rootpkg.dependencies)
49+
recursiveWalk(packages: graph.directDependencies(for: rootpkg))
5050
} else {
5151
stream.send("No external dependencies found\n")
5252
}
5353
}
5454
}
5555

5656
final class FlatListDumper: DependenciesDumper {
57-
func dump(dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
57+
func dump(graph: ModulesGraph, dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
5858
func recursiveWalk(packages: [ResolvedPackage]) {
5959
for package in packages {
6060
stream.send(package.identity.description).send("\n")
6161
if !package.dependencies.isEmpty {
62-
recursiveWalk(packages: package.dependencies)
62+
recursiveWalk(packages: graph.directDependencies(for: package))
6363
}
6464
}
6565
}
6666
if !rootpkg.dependencies.isEmpty {
67-
recursiveWalk(packages: rootpkg.dependencies)
67+
recursiveWalk(packages: graph.directDependencies(for: rootpkg))
6868
}
6969
}
7070
}
7171

7272
final class DotDumper: DependenciesDumper {
73-
func dump(dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
73+
func dump(graph: ModulesGraph, dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
7474
var nodesAlreadyPrinted: Set<String> = []
7575
func printNode(_ package: ResolvedPackage) {
7676
let url = package.manifest.packageLocation
@@ -87,7 +87,7 @@ final class DotDumper: DependenciesDumper {
8787
var dependenciesAlreadyPrinted: Set<DependencyURLs> = []
8888
func recursiveWalk(rootpkg: ResolvedPackage) {
8989
printNode(rootpkg)
90-
for dependency in rootpkg.dependencies {
90+
for dependency in graph.directDependencies(for: rootpkg) {
9191
let rootURL = rootpkg.manifest.packageLocation
9292
let dependencyURL = dependency.manifest.packageLocation
9393
let urlPair = DependencyURLs(root: rootURL, dependency: dependencyURL)
@@ -120,15 +120,15 @@ final class DotDumper: DependenciesDumper {
120120
}
121121

122122
final class JSONDumper: DependenciesDumper {
123-
func dump(dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
123+
func dump(graph: ModulesGraph, dependenciesOf rootpkg: ResolvedPackage, on stream: OutputByteStream) {
124124
func convert(_ package: ResolvedPackage) -> JSON {
125125
return .orderedDictionary([
126126
"identity": .string(package.identity.description),
127127
"name": .string(package.manifest.displayName), // TODO: remove?
128128
"url": .string(package.manifest.packageLocation),
129129
"version": .string(package.manifest.version?.description ?? "unspecified"),
130130
"path": .string(package.path.pathString),
131-
"dependencies": .array(package.dependencies.map(convert)),
131+
"dependencies": .array(package.dependencies.compactMap { graph.packages[$0] }.map(convert)),
132132
])
133133
}
134134

0 commit comments

Comments
 (0)