Skip to content

Commit 4cf8e7e

Browse files
elshelsh
authored
Allow duplicate automatic library product names when module aliasing is used. (#5578)
This change is for automatic library products only. No product names are altered. Add an ID property to Product and ProductReference: combine package ID and product name to make it fully qualified. Pass packageID to Product/Reference initializers. Use product IDs to look up duplicates if the product is an automatic library type. Use product IDs to compute aliasing related logic in ModuleAliasTracker. Resolves rdar://89836609.
1 parent 6e798dc commit 4cf8e7e

File tree

6 files changed

+536
-62
lines changed

6 files changed

+536
-62
lines changed

‎Sources/PackageGraph/ModuleAliasTracker.swift

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@ class ModuleAliasTracker {
77
var idToProductToAllTargets = [PackageIdentity: [String: [Target]]]()
88
var productToDirectTargets = [String: [Target]]()
99
var productToAllTargets = [String: [Target]]()
10-
var childToParentProducts = [String: [String]]()
1110
var parentToChildProducts = [String: [String]]()
12-
var cachedChildToParentProducts = [String: [String]]()
11+
var childToParentProducts = [String: [String]]()
1312
var parentToChildIDs = [PackageIdentity: [PackageIdentity]]()
1413
var childToParentID = [PackageIdentity: PackageIdentity]()
1514

@@ -25,7 +24,8 @@ class ModuleAliasTracker {
2524
if let aliasList = productRef.moduleAliases {
2625
// Track aliases for this product
2726
try addAliases(aliasList,
28-
product: productRef.name,
27+
productID: productRef.ID,
28+
productName: productRef.name,
2929
originPackage: productPkgID,
3030
consumingPackage: package)
3131
}
@@ -34,7 +34,8 @@ class ModuleAliasTracker {
3434
}
3535

3636
func addAliases(_ aliases: [String: String],
37-
product: String,
37+
productID: String,
38+
productName: String,
3839
originPackage: PackageIdentity,
3940
consumingPackage: PackageIdentity) throws {
4041
if let aliasDict = aliasMap[originPackage] {
@@ -43,14 +44,14 @@ class ModuleAliasTracker {
4344
if let newAlias = aliases[existingAlias.name], newAlias != existingAlias.alias {
4445
// Error if there are multiple different aliases specified for
4546
// targets in this product
46-
throw PackageGraphError.multipleModuleAliases(target: existingAlias.name, product: product, package: originPackage.description, aliases: existingAliases.map{$0.alias} + [newAlias])
47+
throw PackageGraphError.multipleModuleAliases(target: existingAlias.name, product: productName, package: originPackage.description, aliases: existingAliases.map{$0.alias} + [newAlias])
4748
}
4849
}
4950
}
5051

5152
for (originalName, newName) in aliases {
5253
let model = ModuleAliasModel(name: originalName, alias: newName, originPackage: originPackage, consumingPackage: consumingPackage)
53-
aliasMap[originPackage, default: [:]][product, default: []].append(model)
54+
aliasMap[originPackage, default: [:]][productID, default: []].append(model)
5455
}
5556
}
5657

@@ -73,25 +74,25 @@ class ModuleAliasTracker {
7374
allTargetDeps.append(contentsOf: targetDeps)
7475
for dep in allTargetDeps {
7576
if case let .product(depRef, _) = dep {
76-
childToParentProducts[depRef.name, default: []].append(product.name)
77-
parentToChildProducts[product.name, default: []].append(depRef.name)
77+
childToParentProducts[depRef.ID, default: []].append(product.ID)
78+
parentToChildProducts[product.ID, default: []].append(depRef.ID)
7879
}
7980
}
8081

8182
var allTargetsInProduct = targetDeps.compactMap{$0.target}
8283
allTargetsInProduct.append(contentsOf: product.targets)
83-
idToProductToAllTargets[package, default: [:]][product.name] = allTargetsInProduct
84-
productToDirectTargets[product.name] = product.targets
85-
productToAllTargets[product.name] = allTargetsInProduct
84+
idToProductToAllTargets[package, default: [:]][product.ID] = allTargetsInProduct
85+
productToDirectTargets[product.ID] = product.targets
86+
productToAllTargets[product.ID] = allTargetsInProduct
8687
}
8788

88-
func validateAndApplyAliases(product: String,
89+
func validateAndApplyAliases(product: Product,
8990
package: PackageIdentity) throws {
90-
guard let targets = idToProductToAllTargets[package]?[product] else { return }
91+
guard let targets = idToProductToAllTargets[package]?[product.ID] else { return }
9192
let targetsWithAliases = targets.filter{ $0.moduleAliases != nil }
9293
for target in targetsWithAliases {
9394
if target.sources.containsNonSwiftFiles {
94-
throw PackageGraphError.invalidSourcesForModuleAliasing(target: target.name, product: product, package: package.description)
95+
throw PackageGraphError.invalidSourcesForModuleAliasing(target: target.name, product: product.name, package: package.description)
9596
}
9697
target.applyAlias()
9798
}
@@ -113,8 +114,8 @@ class ModuleAliasTracker {
113114
// Now merge overriden upstream aliases and add them to
114115
// downstream targets
115116
if let productToAllTargets = idToProductToAllTargets[rootPkg] {
116-
for productName in productToAllTargets.keys {
117-
mergeAliases(product: productName)
117+
for productID in productToAllTargets.keys {
118+
mergeAliases(productID: productID)
118119
}
119120
}
120121
}
@@ -176,23 +177,23 @@ class ModuleAliasTracker {
176177
}
177178

178179
// Merge overriden upstream aliases and add them to downstream targets
179-
func mergeAliases(product: String) {
180-
guard let childProducts = parentToChildProducts[product] else { return }
180+
func mergeAliases(productID: String) {
181+
guard let childProducts = parentToChildProducts[productID] else { return }
181182
for child in childProducts {
182-
mergeAliases(product: child)
183+
mergeAliases(productID: child)
183184
// Filter out targets in the current product with names that are
184185
// aliased with different values in the child products since they
185186
// should either not be aliased or their existing aliases if any
186187
// should not be overridden.
187-
let allTargetNamesInCurProduct = productToAllTargets[product]?.compactMap{$0.name} ?? []
188+
let allTargetNamesInCurProduct = productToAllTargets[productID]?.compactMap{$0.name} ?? []
188189
let childTargetsAliases = productToDirectTargets[child]?.compactMap{$0.moduleAliases}.flatMap{$0}.filter{ !allTargetNamesInCurProduct.contains($0.key) }
189190

190191
if let childTargetsAliases = childTargetsAliases,
191-
let directTargets = productToDirectTargets[product] {
192+
let directTargets = productToDirectTargets[productID] {
192193
// Keep track of all targets in this product that directly
193194
// or indirectly depend on the child product
194-
let directRelevantTargets = directTargets.filter {$0.dependsOn(product: child)}
195-
var relevantTargets = directTargets.map{$0.dependentTargets()}.flatMap{$0}.filter {$0.dependsOn(product: child)}
195+
let directRelevantTargets = directTargets.filter {$0.dependsOn(productID: child)}
196+
var relevantTargets = directTargets.map{$0.dependentTargets()}.flatMap{$0}.filter {$0.dependsOn(productID: child)}
196197
relevantTargets.append(contentsOf: directTargets)
197198
relevantTargets.append(contentsOf: directRelevantTargets)
198199
let relevantTargetSet = Set(relevantTargets)
@@ -259,10 +260,10 @@ class ModuleAliasModel {
259260
}
260261

261262
extension Target {
262-
func dependsOn(product: String) -> Bool {
263+
func dependsOn(productID: String) -> Bool {
263264
return dependencies.contains { dep in
264265
if case let .product(prodRef, _) = dep {
265-
return prodRef.name == product
266+
return prodRef.ID == productID
266267
}
267268
return false
268269
}

‎Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 77 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ private func createResolvedPackages(
246246

247247
// Resolve module aliases, if specified, for targets and their dependencies
248248
// across packages. Aliasing will result in target renaming.
249-
try resolveModuleAliases(packageBuilders: packageBuilders)
249+
let moduleAliasingUsed = try resolveModuleAliases(packageBuilders: packageBuilders)
250250

251251
// Scan and validate the dependencies
252252
for packageBuilder in packageBuilders {
@@ -383,25 +383,67 @@ private func createResolvedPackages(
383383
}
384384

385385
// Find duplicate products in the package graph.
386-
let duplicateProducts = packageBuilders
387-
.flatMap({ $0.products })
388-
.map({ $0.product })
389-
.spm_findDuplicateElements(by: \.name)
390-
.map({ $0[0].name })
391-
392-
// Emit diagnostics for duplicate products.
393-
for productName in duplicateProducts {
394-
let packages = packageBuilders
395-
.filter({ $0.products.contains(where: { $0.product.name == productName }) })
396-
.map{ $0.package.identity.description }
397-
.sorted()
398-
399-
observabilityScope.emit(PackageGraphError.duplicateProduct(product: productName, packages: packages))
400-
}
401-
402-
// Remove the duplicate products from the builders.
403-
for packageBuilder in packageBuilders {
404-
packageBuilder.products = packageBuilder.products.filter({ !duplicateProducts.contains($0.product.name) })
386+
let productList = packageBuilders.flatMap({ $0.products }).map({ $0.product })
387+
388+
if moduleAliasingUsed {
389+
// FIXME: If moduleAliasingUsed, we want to allow duplicate product names
390+
// from different packages as often times the product name of a package is
391+
// same as its target name which might have a conflict with the name of the
392+
// product itself or its target from another package.
393+
// The following is a workaround; eventually we want to allow duplicate product
394+
// names even when module aliasing is not used, which is a cleaner solution.
395+
// Ref rdar://94744134.
396+
397+
// We first divide the products by type which determines whether to use
398+
// the product ID (unique, fully qualified name) or name to look up duplicates.
399+
400+
// There are no shared dirs/files created for automatic library products, so look
401+
// up duplicates with the ID property for those products.
402+
let autoLibProducts = productList
403+
.filter{ $0.isDefaultLibrary }
404+
.spm_findDuplicateElements(by: \.ID)
405+
.map({ $0[0] })
406+
407+
// Building other products, i.e. static libs, dylibs, executables, result in
408+
// shared dirs/files, e.g. Foo.product (dir), libFoo.dylib, etc., so we want
409+
// to keep the original product names for those. Thus, use the name property
410+
// to look up duplicates.
411+
let otherProducts = productList
412+
.filter{ !$0.isDefaultLibrary }
413+
.spm_findDuplicateElements(by: \.name)
414+
.map({ $0[0] })
415+
416+
let allProducts = autoLibProducts + otherProducts
417+
// Emit diagnostics for duplicate products.
418+
for dupProduct in allProducts {
419+
let packages = packageBuilders
420+
.filter({ $0.products.contains(where: { $0.product.isDefaultLibrary ? $0.product.ID == dupProduct.ID : $0.product.name == dupProduct.name }) })
421+
.map{ $0.package.identity.description }
422+
.sorted()
423+
observabilityScope.emit(PackageGraphError.duplicateProduct(product: dupProduct.name, packages: packages))
424+
}
425+
// Remove the duplicate products from the builders.
426+
let autoLibProductIDs = autoLibProducts.map{ $0.ID }
427+
let otherProductNames = otherProducts.map{ $0.name }
428+
for packageBuilder in packageBuilders {
429+
packageBuilder.products = packageBuilder.products.filter { $0.product.isDefaultLibrary ? !autoLibProductIDs.contains($0.product.ID) : !otherProductNames.contains($0.product.name) }
430+
}
431+
} else {
432+
let duplicateProducts = productList
433+
.spm_findDuplicateElements(by: \.name)
434+
.map({ $0[0] })
435+
// Emit diagnostics for duplicate products.
436+
for dupProduct in duplicateProducts {
437+
let packages = packageBuilders
438+
.filter({ $0.products.contains(where: { $0.product.name == dupProduct.name }) })
439+
.map{ $0.package.identity.description }
440+
.sorted()
441+
observabilityScope.emit(PackageGraphError.duplicateProduct(product: dupProduct.name, packages: packages))
442+
}
443+
// Remove the duplicate products from the builders.
444+
for packageBuilder in packageBuilders {
445+
packageBuilder.products = packageBuilder.products.filter { !duplicateProducts.map{$0.name}.contains($0.product.name) }
446+
}
405447
}
406448

407449
// The set of all target names.
@@ -436,7 +478,7 @@ private func createResolvedPackages(
436478
let explicit = Set(dependency.package.manifest.products.lazy.map({ $0.name }))
437479
return dependency.products.filter({ explicit.contains($0.product.name) })
438480
})
439-
let productDependencyMap = productDependencies.spm_createDictionary({ ($0.product.name, $0) })
481+
let productDependencyMap = moduleAliasingUsed ? productDependencies.spm_createDictionary({ ($0.product.ID, $0) }) : productDependencies.spm_createDictionary({ ($0.product.name, $0) })
440482

441483
// Establish dependencies in each target.
442484
for targetBuilder in packageBuilder.targets {
@@ -449,7 +491,9 @@ private func createResolvedPackages(
449491
// Establish product dependencies.
450492
for case .product(let productRef, let conditions) in targetBuilder.target.dependencies {
451493
// Find the product in this package's dependency products.
452-
guard let product = productDependencyMap[productRef.name] else {
494+
// Look it up by ID if module aliasing is used, otherwise by name.
495+
let product = moduleAliasingUsed ? productDependencyMap[productRef.ID] : productDependencyMap[productRef.name]
496+
guard let product = product else {
453497
// Only emit a diagnostic if there are no other diagnostics.
454498
// This avoids flooding the diagnostics with product not
455499
// found errors when there are more important errors to
@@ -514,9 +558,16 @@ private func createResolvedPackages(
514558
}
515559
}
516560
}
561+
517562
return try packageBuilders.map{ try $0.construct() }
518563
}
519564

565+
fileprivate extension Product {
566+
var isDefaultLibrary: Bool {
567+
return type == .library(.automatic)
568+
}
569+
}
570+
520571
private func computePlatforms(
521572
package: Package,
522573
usingXCTest: Bool,
@@ -590,7 +641,7 @@ private func computePlatforms(
590641
}
591642

592643
// Track and override module aliases specified for targets in a package graph
593-
private func resolveModuleAliases(packageBuilders: [ResolvedPackageBuilder]) throws {
644+
private func resolveModuleAliases(packageBuilders: [ResolvedPackageBuilder]) throws -> Bool {
594645

595646
// If there are no module aliases specified, return early
596647
let hasAliases = packageBuilders.contains { $0.package.targets.contains {
@@ -603,7 +654,7 @@ private func resolveModuleAliases(packageBuilders: [ResolvedPackageBuilder]) thr
603654
}
604655
}
605656

606-
guard hasAliases else { return }
657+
guard hasAliases else { return false }
607658
let aliasTracker = ModuleAliasTracker()
608659
for packageBuilder in packageBuilders {
609660
try aliasTracker.addTargetAliases(targets: packageBuilder.package.targets,
@@ -626,10 +677,11 @@ private func resolveModuleAliases(packageBuilders: [ResolvedPackageBuilder]) thr
626677
// upstream can be overriden.
627678
for packageBuilder in packageBuilders {
628679
for product in packageBuilder.package.products {
629-
try aliasTracker.validateAndApplyAliases(product: product.name,
680+
try aliasTracker.validateAndApplyAliases(product: product,
630681
package: packageBuilder.package.identity)
631682
}
632683
}
684+
return true
633685
}
634686

635687
/// A generic builder for `Resolved` models.

‎Sources/PackageLoading/PackageBuilder.swift

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ public final class PackageBuilder {
731731
}
732732

733733
// Check for duplicate target dependencies
734-
dependencies.spm_findDuplicateElements(by: \.nameAndType).map(\.[0].name).forEach {
734+
dependencies.filter{$0.product?.moduleAliases == nil}.spm_findDuplicateElements(by: \.nameAndType).map(\.[0].name).forEach {
735735
self.observabilityScope.emit(.duplicateTargetDependency(dependency: $0, target: potentialModule.name, package: self.identity.description))
736736
}
737737

@@ -1070,7 +1070,7 @@ public final class PackageBuilder {
10701070
// If enabled, create one test product for each test target.
10711071
if self.shouldCreateMultipleTestProducts {
10721072
for testTarget in testModules {
1073-
let product = try Product(name: testTarget.name, type: .test, targets: [testTarget])
1073+
let product = try Product(package: self.identity, name: testTarget.name, type: .test, targets: [testTarget])
10741074
append(product)
10751075
}
10761076
} else if !testModules.isEmpty {
@@ -1083,7 +1083,7 @@ public final class PackageBuilder {
10831083
let productName = self.manifest.displayName + "PackageTests"
10841084
let testManifest = try self.findTestManifest(in: testModules)
10851085

1086-
let product = try Product(name: productName, type: .test, targets: testModules, testManifest: testManifest)
1086+
let product = try Product(package: self.identity, name: productName, type: .test, targets: testModules, testManifest: testManifest)
10871087
append(product)
10881088
}
10891089

@@ -1140,7 +1140,7 @@ public final class PackageBuilder {
11401140
}
11411141
}
11421142

1143-
try append(Product(name: product.name, type: product.type, targets: targets))
1143+
try append(Product(package: self.identity, name: product.name, type: product.type, targets: targets))
11441144
}
11451145

11461146
// Add implicit executables - for root packages and for dependency plugins.
@@ -1184,7 +1184,7 @@ public final class PackageBuilder {
11841184
} else {
11851185
if self.manifest.packageKind.isRoot || implicitPlugInExecutables.contains(target.name) {
11861186
// Generate an implicit product for the executable target
1187-
let product = try Product(name: target.name, type: .executable, targets: [target])
1187+
let product = try Product(package: self.identity, name: target.name, type: .executable, targets: [target])
11881188
append(product)
11891189
}
11901190
}
@@ -1198,6 +1198,7 @@ public final class PackageBuilder {
11981198
self.observabilityScope.emit(.noLibraryTargetsForREPL)
11991199
} else {
12001200
let replProduct = try Product(
1201+
package: self.identity,
12011202
name: self.identity.description + Product.replProductSuffix,
12021203
type: .library(.dynamic),
12031204
targets: libraryTargets
@@ -1209,7 +1210,7 @@ public final class PackageBuilder {
12091210
// Create implicit snippet products
12101211
try targets
12111212
.filter { $0.type == .snippet }
1212-
.map { try Product(name: $0.name, type: .snippet, targets: [$0]) }
1213+
.map { try Product(package: self.identity, name: $0.name, type: .snippet, targets: [$0]) }
12131214
.forEach(append)
12141215

12151216
return products.map{ $0.item }

‎Sources/PackageModel/Product.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ public class Product: Codable {
1919
/// The name of the product.
2020
public let name: String
2121

22+
/// Fully qualified name for this product: package ID + name of this product
23+
public let ID: String
24+
2225
/// The type of product to create.
2326
public let type: ProductType
2427

@@ -35,7 +38,7 @@ public class Product: Codable {
3538
/// The suffix for REPL product name.
3639
public static let replProductSuffix: String = "__REPL"
3740

38-
public init(name: String, type: ProductType, targets: [Target], testManifest: AbsolutePath? = nil) throws {
41+
public init(package: PackageIdentity, name: String, type: ProductType, targets: [Target], testManifest: AbsolutePath? = nil) throws {
3942
guard !targets.isEmpty else {
4043
throw InternalError("Targets cannot be empty")
4144
}
@@ -51,6 +54,7 @@ public class Product: Codable {
5154
}
5255
self.name = name
5356
self.type = type
57+
self.ID = package.description.lowercased() + "_" + name
5458
self._targets = .init(wrappedValue: targets)
5559
self.testManifest = testManifest
5660
}

0 commit comments

Comments
 (0)