Skip to content

Commit 61c9db6

Browse files
hartbithartbit
committed
Only resolve dependencies that are visible from the root package
1 parent 3997c95 commit 61c9db6

29 files changed

+578
-62
lines changed

‎Package.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ let package = Package(
236236
dependencies: ["PackageLoading", "SPMTestSupport"]),
237237
.testTarget(
238238
name: "PackageModelTests",
239-
dependencies: ["PackageModel"]),
239+
dependencies: ["PackageModel", "SPMTestSupport"]),
240240
.testTarget(
241241
name: "PackageGraphTests",
242242
dependencies: ["PackageGraph", "SPMTestSupport"]),

‎Sources/Commands/SwiftPackageTool.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,7 @@ public class SwiftPackageTool: SwiftTool<PackageToolOptions> {
112112
let builder = PackageBuilder(
113113
manifest: manifest,
114114
path: try getPackageRoot(),
115-
diagnostics: diagnostics,
116-
isRootPackage: true
115+
diagnostics: diagnostics
117116
)
118117
let package = try builder.construct()
119118

@@ -337,8 +336,7 @@ public class SwiftPackageTool: SwiftTool<PackageToolOptions> {
337336
let builder = PackageBuilder(
338337
manifest: manifest,
339338
path: try getPackageRoot(),
340-
diagnostics: diagnostics,
341-
isRootPackage: true
339+
diagnostics: diagnostics
342340
)
343341
let package = try builder.construct()
344342
describe(package, in: options.describeMode, on: stdoutStream)

‎Sources/PackageGraph/PackageGraphLoader.swift

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public struct PackageGraphLoader {
8282
let manifestMapSequence = (root.manifests + externalManifests).map({ (PackageReference.computeIdentity(packageURL: $0.url), $0) })
8383
let manifestMap = Dictionary(uniqueKeysWithValues: manifestMapSequence)
8484
let successors: (Manifest) -> [Manifest] = { manifest in
85-
manifest.dependencies.compactMap({
85+
manifest.allRequiredDependencies.compactMap({
8686
let url = config.mirroredURL(forURL: $0.url)
8787
return manifestMap[PackageReference.computeIdentity(packageURL: url)]
8888
})
@@ -111,8 +111,6 @@ public struct PackageGraphLoader {
111111
// Create the packages.
112112
var manifestToPackage: [Manifest: Package] = [:]
113113
for manifest in allManifests {
114-
let isRootPackage = rootManifestSet.contains(manifest)
115-
116114
// Derive the path to the package.
117115
//
118116
// FIXME: Lift this out of the manifest.
@@ -124,17 +122,16 @@ public struct PackageGraphLoader {
124122
path: packagePath,
125123
fileSystem: fileSystem,
126124
diagnostics: diagnostics,
127-
isRootPackage: isRootPackage,
128125
shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts,
129-
createREPLProduct: isRootPackage ? createREPLProduct : false
126+
createREPLProduct: manifest.isRoot ? createREPLProduct : false
130127
)
131128

132129
diagnostics.wrap(with: PackageLocation.Local(name: manifest.name, packagePath: packagePath), {
133130
let package = try builder.construct()
134131
manifestToPackage[manifest] = package
135132

136133
// Throw if any of the non-root package is empty.
137-
if package.targets.isEmpty && !isRootPackage {
134+
if package.targets.isEmpty && !manifest.isRoot {
138135
throw PackageGraphError.noModules(package)
139136
}
140137
})
@@ -230,7 +227,7 @@ private func createResolvedPackages(
230227
let package = packageBuilder.package
231228

232229
// Establish the manifest-declared package dependencies.
233-
packageBuilder.dependencies = package.manifest.dependencies.compactMap({
230+
packageBuilder.dependencies = package.manifest.allRequiredDependencies.compactMap({
234231
let url = config.mirroredURL(forURL: $0.url)
235232
return packageMap[PackageReference.computeIdentity(packageURL: url)]
236233
})

‎Sources/PackageGraph/PackageGraphRoot.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public struct PackageGraphRoot {
9696
public init(input: PackageGraphRootInput, manifests: [Manifest]) {
9797
self.packageRefs = zip(input.packages, manifests).map { (path, manifest) in
9898
let identity = PackageReference.computeIdentity(packageURL: manifest.url)
99-
return PackageReference(identity: identity, path: path.pathString, isLocal: true)
99+
return PackageReference(identity: identity, path: path.pathString, isLocal: true, isRoot: true)
100100
}
101101
self.manifests = manifests
102102
self.dependencies = input.dependencies

‎Sources/PackageGraph/PinsStore.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ extension PinsStore.Pin: JSONMappable, JSONSerializable, Equatable {
148148
public init(json: JSON) throws {
149149
let name: String? = json.get("package")
150150
let url: String = try json.get("repositoryURL")
151-
let ref = PackageReference(identity: PackageReference.computeIdentity(packageURL: url), path: url)
151+
let identity = PackageReference.computeIdentity(packageURL: url)
152+
let ref = PackageReference(identity: identity, path: url)
152153
self.packageRef = name.flatMap(ref.with(newName:)) ?? ref
153154
self.state = try json.get("state")
154155
}

‎Sources/PackageGraph/Pubgrub.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,8 @@ public final class PubgrubDependencyResolver {
898898
identity: "<synthesized-root>",
899899
path: "<synthesized-root-path>",
900900
name: nil,
901-
isLocal: true
901+
isLocal: true,
902+
isRoot: true
902903
)
903904

904905
self.root = root

‎Sources/PackageGraph/RawPackageConstraints.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@ import SourceControl
1313

1414
extension PackageDependencyDescription {
1515
/// Create the package reference object for the dependency.
16-
public func createPackageRef(config: SwiftPMConfig) -> PackageReference {
16+
public func createPackageRef(config: SwiftPMConfig, isRoot: Bool = false) -> PackageReference {
1717
let effectiveURL = config.mirroredURL(forURL: self.url)
1818
return PackageReference(
1919
identity: PackageReference.computeIdentity(packageURL: effectiveURL),
2020
path: effectiveURL,
21-
isLocal: (requirement == .localPackage)
21+
isLocal: (requirement == .localPackage),
22+
isRoot: isRoot
2223
)
2324
}
2425
}
@@ -27,7 +28,7 @@ extension Manifest {
2728

2829
/// Constructs constraints of the dependencies in the raw package.
2930
public func dependencyConstraints(config: SwiftPMConfig) -> [RepositoryPackageConstraint] {
30-
return dependencies.map({
31+
return allRequiredDependencies.map({
3132
return RepositoryPackageConstraint(
3233
container: $0.createPackageRef(config: config),
3334
requirement: $0.requirement.toConstraintRequirement())

‎Sources/PackageGraph/RepositoryPackageContainerProvider.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ public class LocalPackageContainer: BasePackageContainer, CustomStringConvertibl
210210
baseURL: identifier.path,
211211
version: nil,
212212
toolsVersion: toolsVersion,
213+
isRoot: identifier.isRoot,
213214
fileSystem: fs)
214215
return _manifest!
215216
}
@@ -469,6 +470,7 @@ public class RepositoryPackageContainer: BasePackageContainer, CustomStringConve
469470
baseURL: packageURL,
470471
version: version,
471472
toolsVersion: toolsVersion,
473+
isRoot: identifier.isRoot,
472474
fileSystem: fs)
473475
}
474476
}

‎Sources/PackageLoading/ManifestLoader.swift

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,16 @@ public protocol ManifestLoaderProtocol {
6060
/// - path: The root path of the package.
6161
/// - baseURL: The URL the manifest was loaded from.
6262
/// - version: The version the manifest is from, if known.
63-
/// - manifestVersion: The version of manifest to load.
63+
/// - toolsVersion: The version of the tools the manifest supports.
64+
/// - isRoot: Whether the manifest is a manifest of a root package.
6465
/// - fileSystem: If given, the file system to load from (otherwise load from the local file system).
66+
/// - diagnostics: The diagnostics engine.
6567
func load(
6668
packagePath path: AbsolutePath,
6769
baseURL: String,
6870
version: Version?,
6971
toolsVersion: ToolsVersion,
72+
isRoot: Bool,
7073
fileSystem: FileSystem?,
7174
diagnostics: DiagnosticsEngine?
7275
) throws -> Manifest
@@ -79,12 +82,16 @@ extension ManifestLoaderProtocol {
7982
/// - path: The root path of the package.
8083
/// - baseURL: The URL the manifest was loaded from.
8184
/// - version: The version the manifest is from, if known.
82-
/// - fileSystem: The file system to load from.
85+
/// - toolsVersion: The version of the tools the manifest supports.
86+
/// - isRoot: Whether the manifest is a manifest of a root package.
87+
/// - fileSystem: If given, the file system to load from (otherwise load from the local file system).
88+
/// - diagnostics: The diagnostics engine.
8389
public func load(
8490
package path: AbsolutePath,
8591
baseURL: String,
8692
version: Version? = nil,
8793
toolsVersion: ToolsVersion,
94+
isRoot: Bool,// = false,
8895
fileSystem: FileSystem? = nil,
8996
diagnostics: DiagnosticsEngine? = nil
9097
) throws -> Manifest {
@@ -93,6 +100,7 @@ extension ManifestLoaderProtocol {
93100
baseURL: baseURL,
94101
version: version,
95102
toolsVersion: toolsVersion,
103+
isRoot: isRoot,
96104
fileSystem: fileSystem,
97105
diagnostics: diagnostics
98106
)
@@ -160,15 +168,17 @@ public final class ManifestLoader: ManifestLoaderProtocol {
160168
/// Its associated resources will be used by the loader.
161169
public static func loadManifest(
162170
packagePath: AbsolutePath,
163-
swiftCompiler: AbsolutePath
171+
swiftCompiler: AbsolutePath,
172+
isRoot: Bool
164173
) throws -> Manifest {
165174
let resources = try UserManifestResources(swiftCompiler: swiftCompiler)
166175
let loader = ManifestLoader(manifestResources: resources)
167176
let toolsVersion = try ToolsVersionLoader().load(at: packagePath, fileSystem: localFileSystem)
168177
return try loader.load(
169178
package: packagePath,
170179
baseURL: packagePath.pathString,
171-
toolsVersion: toolsVersion
180+
toolsVersion: toolsVersion,
181+
isRoot: isRoot
172182
)
173183
}
174184

@@ -177,6 +187,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
177187
baseURL: String,
178188
version: Version?,
179189
toolsVersion: ToolsVersion,
190+
isRoot: Bool,
180191
fileSystem: FileSystem? = nil,
181192
diagnostics: DiagnosticsEngine? = nil
182193
) throws -> Manifest {
@@ -185,6 +196,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
185196
baseURL: baseURL,
186197
version: version,
187198
toolsVersion: toolsVersion,
199+
isRoot: isRoot,
188200
fileSystem: fileSystem,
189201
diagnostics: diagnostics
190202
)
@@ -196,12 +208,14 @@ public final class ManifestLoader: ManifestLoaderProtocol {
196208
/// - path: The path to the manifest file (or a package root).
197209
/// - baseURL: The URL the manifest was loaded from.
198210
/// - version: The version the manifest is from, if known.
211+
/// - isRoot: Whether the manifest is a manifest of a root package.
199212
/// - fileSystem: If given, the file system to load from (otherwise load from the local file system).
200213
func loadFile(
201214
path inputPath: AbsolutePath,
202215
baseURL: String,
203216
version: Version?,
204217
toolsVersion: ToolsVersion,
218+
isRoot: Bool,
205219
fileSystem: FileSystem? = nil,
206220
diagnostics: DiagnosticsEngine? = nil
207221
) throws -> Manifest {
@@ -246,6 +260,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
246260
url: baseURL,
247261
version: version,
248262
toolsVersion: toolsVersion,
263+
isRoot: isRoot,
249264
pkgConfig: manifestBuilder.pkgConfig,
250265
providers: manifestBuilder.providers,
251266
cLanguageStandard: manifestBuilder.cLanguageStandard,

‎Sources/PackageLoading/PackageBuilder.swift

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -192,9 +192,6 @@ public final class PackageBuilder {
192192
/// The diagnostics engine.
193193
private let diagnostics: DiagnosticsEngine
194194

195-
/// True if this is the root package.
196-
private let isRootPackage: Bool
197-
198195
/// Create multiple test products.
199196
///
200197
/// If set to true, one test product will be created for each test target.
@@ -213,7 +210,6 @@ public final class PackageBuilder {
213210
/// - path: The root path of the package.
214211
/// - fileSystem: The file system on which the builder should be run.
215212
/// - diagnostics: The diagnostics engine.
216-
/// - isRootPackage: If this is a root package.
217213
/// - createMultipleTestProducts: If enabled, create one test product for
218214
/// each test target.
219215
public init(
@@ -222,11 +218,9 @@ public final class PackageBuilder {
222218
additionalFileRules: [FileRuleDescription] = [],
223219
fileSystem: FileSystem = localFileSystem,
224220
diagnostics: DiagnosticsEngine,
225-
isRootPackage: Bool,
226221
shouldCreateMultipleTestProducts: Bool = false,
227222
createREPLProduct: Bool = false
228223
) {
229-
self.isRootPackage = isRootPackage
230224
self.manifest = manifest
231225
self.packagePath = path
232226
self.additionalFileRules = additionalFileRules
@@ -249,12 +243,11 @@ public final class PackageBuilder {
249243
isRootPackage: Bool = true) throws -> Package {
250244

251245
let manifest = try ManifestLoader.loadManifest(
252-
packagePath: packagePath, swiftCompiler: swiftCompiler)
246+
packagePath: packagePath, swiftCompiler: swiftCompiler, isRoot: isRootPackage)
253247
let builder = PackageBuilder(
254248
manifest: manifest,
255249
path: packagePath,
256-
diagnostics: diagnostics,
257-
isRootPackage: isRootPackage)
250+
diagnostics: diagnostics)
258251
return try builder.construct()
259252
}
260253

@@ -369,7 +362,7 @@ public final class PackageBuilder {
369362
private func constructTargets() throws -> [Target] {
370363

371364
// Ensure no dupicate target definitions are found.
372-
let duplicateTargetNames: [String] = manifest.targets.map({ $0.name
365+
let duplicateTargetNames: [String] = manifest.allRequiredTargets.map({ $0.name
373366
}).spm_findDuplicates()
374367

375368
if !duplicateTargetNames.isEmpty {
@@ -501,7 +494,7 @@ public final class PackageBuilder {
501494

502495
// Create potential targets.
503496
let potentialTargets: [PotentialModule]
504-
potentialTargets = try manifest.targets.map({ target in
497+
potentialTargets = try manifest.allRequiredTargets.map({ target in
505498
let path = try findPath(for: target)
506499
return PotentialModule(name: target.name, path: path, type: target.type)
507500
})
@@ -511,9 +504,9 @@ public final class PackageBuilder {
511504
// Create targets from the provided potential targets.
512505
private func createModules(_ potentialModules: [PotentialModule]) throws -> [Target] {
513506
// Find if manifest references a target which isn't present on disk.
514-
let allReferencedModules = manifest.allReferencedModules()
507+
let allVisibleModules = manifest.allVisibleModules()
515508
let potentialModulesName = Set(potentialModules.map({ $0.name }))
516-
let missingModules = allReferencedModules.subtracting(potentialModulesName).intersection(allReferencedModules)
509+
let missingModules = allVisibleModules.subtracting(potentialModulesName).intersection(allVisibleModules)
517510
if let missingModule = missingModules.first {
518511
throw ModuleError.moduleNotFound(missingModule, potentialModules.first(where: { $0.name == missingModule })?.type ?? .regular)
519512
}
@@ -1002,7 +995,7 @@ public final class PackageBuilder {
1002995
}
1003996

1004997
// Only create implicit executables for root packages.
1005-
if isRootPackage {
998+
if manifest.isRoot {
1006999
// Compute the list of targets which are being used in an
10071000
// executable product so we don't create implicit executables
10081001
// for them.
@@ -1111,14 +1104,15 @@ private struct PotentialModule: Hashable {
11111104
}
11121105

11131106
private extension Manifest {
1114-
/// Returns the names of all the referenced targets in the manifest.
1115-
func allReferencedModules() -> Set<String> {
1116-
let names = targets.flatMap({ target in
1107+
/// Returns the names of all the visible targets in the manifest.
1108+
func allVisibleModules() -> Set<String> {
1109+
let names = allRequiredTargets.flatMap({ target in
11171110
[target.name] + target.dependencies.compactMap({
11181111
switch $0 {
11191112
case .target(let name):
11201113
return name
11211114
case .byName, .product:
1115+
// TODO: Ask why we don't take into account byName as it could point to another target.
11221116
return nil
11231117
}
11241118
})

0 commit comments

Comments
 (0)