Skip to content

Commit 9c1bc87

Browse files
hartbithartbit
committed
Only resolve dependencies that are visible from the root package
1 parent 09dc6b8 commit 9c1bc87

32 files changed

+626
-81
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.kind == .root ? 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.kind != .root {
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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public struct PackageGraphRoot {
6161
return PackageReference(
6262
identity: PackageReference.computeIdentity(packageURL: effectiveURL),
6363
path: effectiveURL,
64-
isLocal: (requirement == .localPackage)
64+
kind: requirement == .localPackage ? .local : .remote
6565
)
6666
}
6767

@@ -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, kind: .root)
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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,7 @@ public final class PubgrubDependencyResolver {
898898
identity: "<synthesized-root>",
899899
path: "<synthesized-root-path>",
900900
name: nil,
901-
isLocal: true
901+
kind: .root
902902
)
903903

904904
self.root = root

‎Sources/PackageGraph/RawPackageConstraints.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ extension PackageDependencyDescription {
1818
return PackageReference(
1919
identity: PackageReference.computeIdentity(packageURL: effectiveURL),
2020
path: effectiveURL,
21-
isLocal: (requirement == .localPackage)
21+
kind: requirement == .localPackage ? .local : .remote
2222
)
2323
}
2424
}
@@ -27,7 +27,7 @@ extension Manifest {
2727

2828
/// Constructs constraints of the dependencies in the raw package.
2929
public func dependencyConstraints(config: SwiftPMConfig) -> [RepositoryPackageConstraint] {
30-
return dependencies.map({
30+
return allRequiredDependencies.map({
3131
return RepositoryPackageConstraint(
3232
container: $0.createPackageRef(config: config),
3333
requirement: $0.requirement.toConstraintRequirement())

‎Sources/PackageGraph/RepositoryPackageContainerProvider.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public class RepositoryPackageContainerProvider: PackageContainerProvider {
6262
completion: @escaping (Result<PackageContainer, AnyError>) -> Void
6363
) {
6464
// If the container is local, just create and return a local package container.
65-
if identifier.isLocal {
65+
if identifier.kind != .remote {
6666
callbacksQueue.async {
6767
let container = LocalPackageContainer(identifier,
6868
config: self.config,
@@ -107,7 +107,7 @@ extension PackageReference {
107107
///
108108
/// This should only be accessed when the reference is not local.
109109
public var repository: RepositorySpecifier {
110-
precondition(!isLocal)
110+
precondition(kind == .remote)
111111
return RepositorySpecifier(url: path)
112112
}
113113
}
@@ -210,6 +210,7 @@ public class LocalPackageContainer: BasePackageContainer, CustomStringConvertibl
210210
baseURL: identifier.path,
211211
version: nil,
212212
toolsVersion: toolsVersion,
213+
kind: identifier.kind,
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+
kind: identifier.kind,
472474
fileSystem: fs)
473475
}
474476
}

‎Sources/PackageLoading/ManifestLoader.swift

Lines changed: 20 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+
/// - kind: The kind of package the manifest is from.
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+
kind: PackageReference.Kind,
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+
/// - kind: The kind of package the manifest is from.
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+
kind: PackageReference.Kind,
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+
kind: kind,
96104
fileSystem: fileSystem,
97105
diagnostics: diagnostics
98106
)
@@ -158,17 +166,20 @@ public final class ManifestLoader: ManifestLoaderProtocol {
158166
/// - packagePath: The absolute path of the package root.
159167
/// - swiftCompiler: The absolute path of a `swiftc` executable.
160168
/// Its associated resources will be used by the loader.
169+
/// - kind: The kind of package the manifest is from.
161170
public static func loadManifest(
162171
packagePath: AbsolutePath,
163-
swiftCompiler: AbsolutePath
172+
swiftCompiler: AbsolutePath,
173+
kind: PackageReference.Kind
164174
) throws -> Manifest {
165175
let resources = try UserManifestResources(swiftCompiler: swiftCompiler)
166176
let loader = ManifestLoader(manifestResources: resources)
167177
let toolsVersion = try ToolsVersionLoader().load(at: packagePath, fileSystem: localFileSystem)
168178
return try loader.load(
169179
package: packagePath,
170180
baseURL: packagePath.pathString,
171-
toolsVersion: toolsVersion
181+
toolsVersion: toolsVersion,
182+
kind: kind
172183
)
173184
}
174185

@@ -177,6 +188,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
177188
baseURL: String,
178189
version: Version?,
179190
toolsVersion: ToolsVersion,
191+
kind: PackageReference.Kind,
180192
fileSystem: FileSystem? = nil,
181193
diagnostics: DiagnosticsEngine? = nil
182194
) throws -> Manifest {
@@ -185,6 +197,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
185197
baseURL: baseURL,
186198
version: version,
187199
toolsVersion: toolsVersion,
200+
kind: kind,
188201
fileSystem: fileSystem,
189202
diagnostics: diagnostics
190203
)
@@ -196,12 +209,14 @@ public final class ManifestLoader: ManifestLoaderProtocol {
196209
/// - path: The path to the manifest file (or a package root).
197210
/// - baseURL: The URL the manifest was loaded from.
198211
/// - version: The version the manifest is from, if known.
212+
/// - kind: The kind of package the manifest is from.
199213
/// - fileSystem: If given, the file system to load from (otherwise load from the local file system).
200214
func loadFile(
201215
path inputPath: AbsolutePath,
202216
baseURL: String,
203217
version: Version?,
204218
toolsVersion: ToolsVersion,
219+
kind: PackageReference.Kind,
205220
fileSystem: FileSystem? = nil,
206221
diagnostics: DiagnosticsEngine? = nil
207222
) throws -> Manifest {
@@ -246,6 +261,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
246261
url: baseURL,
247262
version: version,
248263
toolsVersion: toolsVersion,
264+
kind: kind,
249265
pkgConfig: manifestBuilder.pkgConfig,
250266
providers: manifestBuilder.providers,
251267
cLanguageStandard: manifestBuilder.cLanguageStandard,

‎Sources/PackageLoading/PackageBuilder.swift

Lines changed: 16 additions & 19 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
@@ -242,19 +236,21 @@ public final class PackageBuilder {
242236
/// - packagePath: The absolute path of the package root.
243237
/// - swiftCompiler: The absolute path of a `swiftc` executable.
244238
/// Its associated resources will be used by the loader.
239+
/// - kind: The kind of package.
245240
public static func loadPackage(
246241
packagePath: AbsolutePath,
247242
swiftCompiler: AbsolutePath,
248243
diagnostics: DiagnosticsEngine,
249-
isRootPackage: Bool = true) throws -> Package {
250-
244+
kind: PackageReference.Kind = .root
245+
) throws -> Package {
251246
let manifest = try ManifestLoader.loadManifest(
252-
packagePath: packagePath, swiftCompiler: swiftCompiler)
247+
packagePath: packagePath,
248+
swiftCompiler: swiftCompiler,
249+
kind: kind)
253250
let builder = PackageBuilder(
254251
manifest: manifest,
255252
path: packagePath,
256-
diagnostics: diagnostics,
257-
isRootPackage: isRootPackage)
253+
diagnostics: diagnostics)
258254
return try builder.construct()
259255
}
260256

@@ -369,7 +365,7 @@ public final class PackageBuilder {
369365
private func constructTargets() throws -> [Target] {
370366

371367
// Ensure no dupicate target definitions are found.
372-
let duplicateTargetNames: [String] = manifest.targets.map({ $0.name
368+
let duplicateTargetNames: [String] = manifest.allRequiredTargets.map({ $0.name
373369
}).spm_findDuplicates()
374370

375371
if !duplicateTargetNames.isEmpty {
@@ -501,7 +497,7 @@ public final class PackageBuilder {
501497

502498
// Create potential targets.
503499
let potentialTargets: [PotentialModule]
504-
potentialTargets = try manifest.targets.map({ target in
500+
potentialTargets = try manifest.allRequiredTargets.map({ target in
505501
let path = try findPath(for: target)
506502
return PotentialModule(name: target.name, path: path, type: target.type)
507503
})
@@ -511,9 +507,9 @@ public final class PackageBuilder {
511507
// Create targets from the provided potential targets.
512508
private func createModules(_ potentialModules: [PotentialModule]) throws -> [Target] {
513509
// Find if manifest references a target which isn't present on disk.
514-
let allReferencedModules = manifest.allReferencedModules()
510+
let allVisibleModules = manifest.allVisibleModules()
515511
let potentialModulesName = Set(potentialModules.map({ $0.name }))
516-
let missingModules = allReferencedModules.subtracting(potentialModulesName).intersection(allReferencedModules)
512+
let missingModules = allVisibleModules.subtracting(potentialModulesName).intersection(allVisibleModules)
517513
if let missingModule = missingModules.first {
518514
throw ModuleError.moduleNotFound(missingModule, potentialModules.first(where: { $0.name == missingModule })?.type ?? .regular)
519515
}
@@ -1002,7 +998,7 @@ public final class PackageBuilder {
1002998
}
1003999

10041000
// Only create implicit executables for root packages.
1005-
if isRootPackage {
1001+
if manifest.kind == .root {
10061002
// Compute the list of targets which are being used in an
10071003
// executable product so we don't create implicit executables
10081004
// for them.
@@ -1111,14 +1107,15 @@ private struct PotentialModule: Hashable {
11111107
}
11121108

11131109
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
1110+
/// Returns the names of all the visible targets in the manifest.
1111+
func allVisibleModules() -> Set<String> {
1112+
let names = allRequiredTargets.flatMap({ target in
11171113
[target.name] + target.dependencies.compactMap({
11181114
switch $0 {
11191115
case .target(let name):
11201116
return name
11211117
case .byName, .product:
1118+
// TODO: Ask why we don't take into account byName as it could point to another target.
11221119
return nil
11231120
}
11241121
})

0 commit comments

Comments
 (0)