Skip to content

Commit db1a26f

Browse files
xedinxedin
committed
[PackageGraph] Switch ResolvedPackage dependencies to use PackageIdentifier
Another step towards recursive dependency support - `ResolvedPackage` identifies its dependencies but refers back to the `ModulesGraph` to fetch underlying packages. This helps to avoid possible infinite recursion while building `ResolvedPackage` via existing builder pattern. There are really only two places where recursive walk over dependencies is needed - `PluginContextSerializer` and `ShowDependencies` command, both of which have module graph available.
1 parent 0ad8cd5 commit db1a26f

File tree

11 files changed

+70
-37
lines changed

11 files changed

+70
-37
lines changed

‎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: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -383,10 +383,17 @@ struct PluginCommand: SwiftCommand {
383383

384384
static func availableCommandPlugins(in graph: ModulesGraph, limitedTo packageIdentity: String?) -> [PluginTarget] {
385385
// All targets from plugin products of direct dependencies are "available".
386-
let directDependencyPackages = graph.rootPackages.flatMap { $0.dependencies }.filter { $0.matching(identity: packageIdentity) }
386+
let directDependencyPackages = graph.rootPackages.flatMap {
387+
$0.dependencies
388+
}.filter {
389+
$0.matching(identity: packageIdentity)
390+
}.compactMap {
391+
graph.package(for: $0)
392+
}
393+
387394
let directDependencyPluginTargets = directDependencyPackages.flatMap { $0.products.filter { $0.type == .plugin } }.flatMap { $0.targets }
388395
// As well as any plugin targets in root packages.
389-
let rootPackageTargets = graph.rootPackages.filter { $0.matching(identity: packageIdentity) }.flatMap { $0.targets }
396+
let rootPackageTargets = graph.rootPackages.filter { $0.identity.matching(identity: packageIdentity) }.flatMap { $0.targets }
390397
return (directDependencyPluginTargets + rootPackageTargets).compactMap { $0.underlying as? PluginTarget }.filter {
391398
switch $0.capability {
392399
case .buildTool: return false
@@ -470,10 +477,10 @@ extension SandboxNetworkPermission {
470477
}
471478
}
472479

473-
extension ResolvedPackage {
480+
extension PackageIdentity {
474481
fileprivate func matching(identity: String?) -> Bool {
475482
if let identity {
476-
return self.identity == .plain(identity)
483+
return self == .plain(identity)
477484
} else {
478485
return true
479486
}

‎Sources/Commands/PackageCommands/ShowDependencies.swift

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

52-
static func dumpDependenciesOf(rootPackage: ResolvedPackage, mode: ShowDependenciesMode, on stream: OutputByteStream) {
53+
static func dumpDependenciesOf(
54+
graph: ModulesGraph,
55+
rootPackage: ResolvedPackage,
56+
mode: ShowDependenciesMode,
57+
on stream: OutputByteStream
58+
) {
5359
let dumper: DependenciesDumper
5460
switch mode {
5561
case .text:
@@ -61,7 +67,7 @@ extension SwiftPackageCommand {
6167
case .flatlist:
6268
dumper = FlatListDumper()
6369
}
64-
dumper.dump(dependenciesOf: rootPackage, on: stream)
70+
dumper.dump(graph: graph, dependenciesOf: rootPackage, on: stream)
6571
stream.flush()
6672
}
6773

‎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

‎Sources/PackageGraph/ModulesGraph+Loading.swift

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ extension ModulesGraph {
162162
)
163163

164164
let rootPackages = resolvedPackages.filter { root.manifests.values.contains($0.manifest) }
165-
checkAllDependenciesAreUsed(rootPackages, observabilityScope: observabilityScope)
165+
checkAllDependenciesAreUsed(packages: resolvedPackages, rootPackages, observabilityScope: observabilityScope)
166166

167167
return try ModulesGraph(
168168
rootPackages: rootPackages,
@@ -174,7 +174,11 @@ extension ModulesGraph {
174174
}
175175
}
176176

177-
private func checkAllDependenciesAreUsed(_ rootPackages: [ResolvedPackage], observabilityScope: ObservabilityScope) {
177+
private func checkAllDependenciesAreUsed(
178+
packages: IdentifiableSet<ResolvedPackage>,
179+
_ rootPackages: [ResolvedPackage],
180+
observabilityScope: ObservabilityScope
181+
) {
178182
for package in rootPackages {
179183
// List all dependency products dependent on by the package targets.
180184
let productDependencies = IdentifiableSet(package.targets.flatMap { target in
@@ -188,7 +192,12 @@ private func checkAllDependenciesAreUsed(_ rootPackages: [ResolvedPackage], obse
188192
}
189193
})
190194

191-
for dependency in package.dependencies {
195+
for dependencyId in package.dependencies {
196+
guard let dependency = packages[dependencyId] else {
197+
observabilityScope.emit(.error("Unknown package: \(dependencyId)"))
198+
return
199+
}
200+
192201
// We continue if the dependency contains executable products to make sure we don't
193202
// warn on a valid use-case for a lone dependency: swift run dependency executables.
194203
guard !dependency.products.contains(where: { $0.type == .executable }) else {
@@ -249,7 +258,7 @@ private func createResolvedPackages(
249258
platformVersionProvider: PlatformVersionProvider,
250259
fileSystem: FileSystem,
251260
observabilityScope: ObservabilityScope
252-
) throws -> [ResolvedPackage] {
261+
) throws -> IdentifiableSet<ResolvedPackage> {
253262

254263
// Create package builder objects from the input manifests.
255264
let packageBuilders: [ResolvedPackageBuilder] = nodes.compactMap{ node in
@@ -643,7 +652,7 @@ private func createResolvedPackages(
643652
}
644653
}
645654

646-
return try packageBuilders.map { try $0.construct() }
655+
return IdentifiableSet(try packageBuilders.map { try $0.construct() })
647656
}
648657

649658
private func emitDuplicateProductDiagnostic(
@@ -1044,11 +1053,11 @@ private final class ResolvedPackageBuilder: ResolvedBuilder<ResolvedPackage> {
10441053
var targets = products.reduce(into: IdentifiableSet()) { $0.formUnion($1.targets) }
10451054
try targets.formUnion(self.targets.map { try $0.construct() })
10461055

1047-
return try ResolvedPackage(
1056+
return ResolvedPackage(
10481057
underlying: self.package,
10491058
defaultLocalization: self.defaultLocalization,
10501059
supportedPlatforms: self.supportedPlatforms,
1051-
dependencies: self.dependencies.map{ try $0.construct() },
1060+
dependencies: self.dependencies.map { $0.package.identity },
10521061
targets: targets,
10531062
products: products,
10541063
registryMetadata: self.registryMetadata,

‎Sources/PackageGraph/ModulesGraph.swift

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,8 @@ public struct ModulesGraph {
8989
/// The root packages.
9090
public let rootPackages: IdentifiableSet<ResolvedPackage>
9191

92-
/// The complete list of contained packages, in topological order starting
93-
/// with the root packages.
94-
public let packages: [ResolvedPackage]
92+
/// The complete set of contained packages.
93+
public let packages: IdentifiableSet<ResolvedPackage>
9594

9695
/// The list of all targets reachable from root targets.
9796
public private(set) var reachableTargets: IdentifiableSet<ResolvedModule>
@@ -140,6 +139,12 @@ public struct ModulesGraph {
140139
}
141140

142141
private var modulesToPackages: [ResolvedModule.ID: ResolvedPackage]
142+
143+
/// Returns the package based on the given identity, or nil if the package isn't in the graph.
144+
public func package(for identity: PackageIdentity) -> ResolvedPackage? {
145+
packages[identity]
146+
}
147+
143148
/// Returns the package that contains the module, or nil if the module isn't in the graph.
144149
public func package(for module: ResolvedModule) -> ResolvedPackage? {
145150
return self.modulesToPackages[module.id]
@@ -152,6 +157,11 @@ public struct ModulesGraph {
152157
return self.productsToPackages[product.id]
153158
}
154159

160+
/// Returns all of the packages that the given package depends on directly.
161+
public func directDependencies(for package: ResolvedPackage) -> [ResolvedPackage] {
162+
package.dependencies.compactMap { self.package(for: $0) }
163+
}
164+
155165
/// All root and root dependency packages provided as input to the graph.
156166
public let inputPackages: [ResolvedPackage]
157167

@@ -162,7 +172,7 @@ public struct ModulesGraph {
162172
public init(
163173
rootPackages: [ResolvedPackage],
164174
rootDependencies: [ResolvedPackage] = [],
165-
packages: [ResolvedPackage],
175+
packages: IdentifiableSet<ResolvedPackage>,
166176
dependencies requiredDependencies: [PackageReference],
167177
binaryArtifacts: [PackageIdentity: [String: BinaryArtifact]]
168178
) throws {
@@ -171,7 +181,6 @@ public struct ModulesGraph {
171181
self.inputPackages = rootPackages + rootDependencies
172182
self.binaryArtifacts = binaryArtifacts
173183
self.packages = packages
174-
let identitiesToPackages = self.packages.spm_createDictionary { ($0.identity, $0) }
175184

176185
// Create a mapping from targets to the packages that define them. Here
177186
// we include all targets, including tests in non-root packages, since
@@ -209,11 +218,11 @@ public struct ModulesGraph {
209218
case .target(let targetDependency, _):
210219
allTargets.insert(targetDependency)
211220
modulesToPackages[targetDependency.id] =
212-
identitiesToPackages[targetDependency.packageIdentity]
221+
self.packages[targetDependency.packageIdentity]
213222
case .product(let productDependency, _):
214223
allProducts.insert(productDependency)
215224
productsToPackages[productDependency.id] =
216-
identitiesToPackages[productDependency.packageIdentity]
225+
self.packages[productDependency.packageIdentity]
217226
}
218227
}
219228
}

‎Sources/PackageGraph/Resolution/ResolvedPackage.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public struct ResolvedPackage {
4040
public let products: [ResolvedProduct]
4141

4242
/// The dependencies of the package.
43-
public let dependencies: [ResolvedPackage]
43+
public let dependencies: [PackageIdentity]
4444

4545
/// The default localization for resources.
4646
public let defaultLocalization: String?
@@ -57,7 +57,7 @@ public struct ResolvedPackage {
5757
underlying: Package,
5858
defaultLocalization: String?,
5959
supportedPlatforms: [SupportedPlatform],
60-
dependencies: [ResolvedPackage],
60+
dependencies: [PackageIdentity],
6161
targets: IdentifiableSet<ResolvedModule>,
6262
products: [ResolvedProduct],
6363
registryMetadata: RegistryReleaseMetadata?,

‎Sources/SPMBuildCore/Plugins/PluginContextSerializer.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ internal struct PluginContextSerializer {
245245
}
246246

247247
// Serialize the dependencies. It is important to do this before the `let id = package.count` below so the correct wire ID gets assigned.
248-
let dependencies = try package.dependencies.map {
248+
let dependencies = try modulesGraph.directDependencies(for: package).map {
249249
WireInput.Package.Dependency(packageId: try serialize(package: $0))
250250
}
251251

‎Sources/Workspace/Workspace+Signing.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ extension Workspace {
4646
expectedSigningEntities: [PackageIdentity: RegistryReleaseMetadata.SigningEntity]
4747
) throws {
4848
try expectedSigningEntities.forEach { identity, expectedSigningEntity in
49-
if let package = packageGraph.packages.first(where: { $0.identity == identity }) {
49+
if let package = packageGraph.package(for: identity) {
5050
guard let actualSigningEntity = package.registryMetadata?.signature?.signedBy else {
5151
throw SigningError.unsigned(package: identity, expected: expectedSigningEntity)
5252
}
@@ -68,7 +68,7 @@ extension Workspace {
6868
expected: expectedSigningEntity
6969
)
7070
}
71-
guard let package = packageGraph.packages.first(where: { $0.identity == mirroredIdentity }) else {
71+
guard let package = packageGraph.package(for: mirroredIdentity) else {
7272
// Unsure if this case is reachable in practice.
7373
throw SigningError.expectedIdentityNotFound(package: identity)
7474
}

‎Sources/Workspace/Workspace.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1150,7 +1150,7 @@ extension Workspace {
11501150
observabilityScope: ObservabilityScope,
11511151
completion: @escaping (Result<Package, Error>) -> Void
11521152
) {
1153-
guard let previousPackage = packageGraph.packages.first(where: { $0.identity == identity }) else {
1153+
guard let previousPackage = packageGraph.package(for: identity) else {
11541154
return completion(.failure(StringError("could not find package with identity \(identity)")))
11551155
}
11561156

‎Tests/CommandsTests/PackageCommandTests.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,7 @@ final class PackageCommandTests: CommandsTestCase {
647647

648648
let output = BufferedOutputByteStream()
649649
SwiftPackageCommand.ShowDependencies.dumpDependenciesOf(
650+
graph: graph,
650651
rootPackage: graph.rootPackages[graph.rootPackages.startIndex],
651652
mode: .dot,
652653
on: output

0 commit comments

Comments
 (0)