Skip to content

Commit b25e390

Browse files
committed
[PackageGraph] Fix package name validation for product target dependencies
The package name validation was wrong for product target dependencies (`.product(name: "<product>", package: "<package>")`) as we were looking up the package using its identity instead of the name. This completely breaks package loading in 5.2 if a package wants to use a product from a dependency that doesn't match the package name of that dependency. <rdar://problem/59744465>
1 parent ea8e19c commit b25e390

File tree

2 files changed

+52
-7
lines changed

2 files changed

+52
-7
lines changed

Sources/PackageGraph/PackageGraphLoader.swift

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -271,20 +271,26 @@ private func createResolvedPackages(
271271
})
272272

273273
// Create a map of package builders keyed by the package identity.
274-
let packageMap: [String: ResolvedPackageBuilder] = packageBuilders.spm_createDictionary({
275-
// FIXME: This shouldn't be needed once <rdar://problem/33693433> is fixed.
274+
let packageMapByIdentity: [String: ResolvedPackageBuilder] = packageBuilders.spm_createDictionary{
276275
let identity = PackageReference.computeIdentity(packageURL: $0.package.manifest.url)
277276
return (identity, $0)
278-
})
277+
}
278+
let packageMapByName: [String: ResolvedPackageBuilder] = packageBuilders.spm_createDictionary{ ($0.package.name, $0) }
279279

280280
// In the first pass, we wire some basic things.
281281
for packageBuilder in packageBuilders {
282282
let package = packageBuilder.package
283283

284284
// Establish the manifest-declared package dependencies.
285-
packageBuilder.dependencies = package.manifest.allRequiredDependencies.compactMap({ dependency in
285+
packageBuilder.dependencies = package.manifest.allRequiredDependencies.compactMap { dependency in
286+
// Use the package name to lookup the dependency. The package name will be present in packages with tools version >= 5.2.
287+
if let dependencyName = dependency.explicitName, let resolvedPackage = packageMapByName[dependencyName] {
288+
return resolvedPackage
289+
}
290+
291+
// Otherwise, look it up by its identity.
286292
let url = config.mirroredURL(forURL: dependency.url)
287-
let resolvedPackage = packageMap[PackageReference.computeIdentity(packageURL: url)]
293+
let resolvedPackage = packageMapByIdentity[PackageReference.computeIdentity(packageURL: url)]
288294

289295
// We check that the explicit package dependency name matches the package name.
290296
if let resolvedPackage = resolvedPackage,
@@ -300,7 +306,7 @@ private func createResolvedPackages(
300306
}
301307

302308
return resolvedPackage
303-
})
309+
}
304310

305311
// Create target builders for each target in the package.
306312
let targetBuilders = package.targets.map({ ResolvedTargetBuilder(target: $0, diagnostics: diagnostics) })
@@ -403,7 +409,7 @@ private func createResolvedPackages(
403409
if let packageName = productRef.package {
404410
// Find the declared package and check that it contains
405411
// the product we found above.
406-
guard let dependencyPackage = packageMap[packageName.lowercased()], dependencyPackage.products.contains(product) else {
412+
guard let dependencyPackage = packageMapByName[packageName], dependencyPackage.products.contains(product) else {
407413
let error = PackageGraphError.productDependencyIncorrectPackage(
408414
name: productRef.name, package: packageName)
409415
diagnostics.emit(error, location: diagnosticLocation())

Tests/PackageGraphTests/PackageGraphTests.swift

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,45 @@ class PackageGraphTests: XCTestCase {
632632
}
633633
}
634634

635+
func testPackageNameValidationInProductTargetDependency() throws {
636+
let fs = InMemoryFileSystem(emptyFiles:
637+
"/Foo/Sources/Foo/foo.swift",
638+
"/Bar/Sources/Bar/bar.swift"
639+
)
640+
641+
let diagnostics = DiagnosticsEngine()
642+
_ = loadPackageGraph(fs: fs, diagnostics: diagnostics,
643+
manifests: [
644+
Manifest.createManifest(
645+
name: "Foo",
646+
path: "/Foo",
647+
url: "/Foo",
648+
v: .v5_2,
649+
packageKind: .root,
650+
dependencies: [
651+
PackageDependencyDescription(name: "UnBar", url: "/Bar", requirement: .branch("master")),
652+
],
653+
targets: [
654+
TargetDescription(name: "Foo", dependencies: [.product(name: "BarProduct", package: "UnBar")]),
655+
]),
656+
Manifest.createV4Manifest(
657+
name: "UnBar",
658+
path: "/Bar",
659+
url: "/Bar",
660+
packageKind: .remote,
661+
products: [
662+
ProductDescription(name: "BarProduct", targets: ["Bar"])
663+
],
664+
targets: [
665+
TargetDescription(name: "Bar"),
666+
]),
667+
]
668+
)
669+
670+
// Expect no diagnostics.
671+
DiagnosticsEngineTester(diagnostics) { _ in }
672+
}
673+
635674
func testUnusedDependency() throws {
636675
let fs = InMemoryFileSystem(emptyFiles:
637676
"/Foo/Sources/Foo/foo.swift",

0 commit comments

Comments
 (0)