Use product IDs to allow duplicate product names in package graph (#5792)

Use product IDs (package id + product name) to allow duplicate product names in a package graph
Resolves rdar://94744134

Author: elsh

Sources/PackageGraph/ModuleAliasTracker.swift

@@ -25,7 +25,7 @@ class ModuleAliasTracker {
                 if let aliasList = productRef.moduleAliases {
                     // Track aliases for this product
                     try addAliases(aliasList,
-                                   productID: productRef.ID,
+                                   productID: productRef.identity,
                                    productName: productRef.name,
                                    originPackage: productPkgID,
                                    consumingPackage: package)
@@ -76,20 +76,20 @@ class ModuleAliasTracker {
         allTargetDeps.append(contentsOf: targetDeps)
         for dep in allTargetDeps {
             if case let .product(depRef, _) = dep {
-                parentToChildProducts[product.ID, default: []].append(depRef.ID)
+                parentToChildProducts[product.identity, default: []].append(depRef.identity)
             }
         }
 
         var allTargetsInProduct = targetDeps.compactMap{$0.target}
         allTargetsInProduct.append(contentsOf: product.targets)
-        idToProductToAllTargets[package, default: [:]][product.ID] = allTargetsInProduct
-        productToDirectTargets[product.ID] = product.targets
-        productToAllTargets[product.ID] = allTargetsInProduct
+        idToProductToAllTargets[package, default: [:]][product.identity] = allTargetsInProduct
+        productToDirectTargets[product.identity] = product.targets
+        productToAllTargets[product.identity] = allTargetsInProduct
     }
 
     func validateAndApplyAliases(product: Product,
                                  package: PackageIdentity) throws {
-        guard let targets = idToProductToAllTargets[package]?[product.ID] else { return }
+        guard let targets = idToProductToAllTargets[package]?[product.identity] else { return }
         let targetsWithAliases = targets.filter{ $0.moduleAliases != nil }
         for target in targetsWithAliases {
             if target.sources.containsNonSwiftFiles {
@@ -376,7 +376,7 @@ class ModuleAliasTracker {
     }
 
     private func dependencyProductTargets(of targets: [Target]) -> [Target] {
-        let result = targets.map{$0.dependencies.compactMap{$0.product?.ID}}.flatMap{$0}.compactMap{productToAllTargets[$0]}.flatMap{$0}
+        let result = targets.map{$0.dependencies.compactMap{$0.product?.identity}}.flatMap{$0}.compactMap{productToAllTargets[$0]}.flatMap{$0}
         return result
     }
 }
@@ -400,7 +400,7 @@ extension Target {
     func dependsOn(productID: String) -> Bool {
         return dependencies.contains { dep in
             if case let .product(prodRef, _) = dep {
-                return prodRef.ID == productID
+                return prodRef.identity == productID
             }
             return false
         }

Sources/PackageGraph/PackageGraph+Loading.swift

@@ -384,69 +384,8 @@ private func createResolvedPackages(
         }
     }
 
-    // Find duplicate products in the package graph.
-    let productList = packageBuilders.flatMap({ $0.products }).map({ $0.product })
-
-    if moduleAliasingUsed {
-        // FIXME: If moduleAliasingUsed, we want to allow duplicate product names
-        // from different packages as often times the product name of a package is
-        // same as its target name which might have a conflict with the name of the
-        // product itself or its target from another package.
-        // The following is a workaround; eventually we want to allow duplicate product
-        // names even when module aliasing is not used, which is a cleaner solution.
-        // Ref rdar://94744134.
-
-        // We first divide the products by type which determines whether to use
-        // the product ID (unique, fully qualified name) or name to look up duplicates.
-
-        // There are no shared dirs/files created for automatic library products, so look
-        // up duplicates with the ID property for those products.
-        let autoLibProducts = productList
-            .filter{ $0.isDefaultLibrary }
-            .spm_findDuplicateElements(by: \.ID)
-            .map({ $0[0] })
-
-        // Building other products, i.e. static libs, dylibs, executables, result in
-        // shared dirs/files, e.g. Foo.product (dir), libFoo.dylib, etc., so we want
-        // to keep the original product names for those. Thus, use the name property
-        // to look up duplicates.
-        let otherProducts = productList
-            .filter{ !$0.isDefaultLibrary }
-            .spm_findDuplicateElements(by: \.name)
-            .map({ $0[0] })
-
-        let allProducts = autoLibProducts + otherProducts
-        // Emit diagnostics for duplicate products.
-        for dupProduct in allProducts {
-            let packages = packageBuilders
-                .filter({ $0.products.contains(where: { $0.product.isDefaultLibrary ? $0.product.ID == dupProduct.ID : $0.product.name == dupProduct.name }) })
-                .map{ $0.package.identity.description }
-                .sorted()
-            observabilityScope.emit(PackageGraphError.duplicateProduct(product: dupProduct.name, packages: packages))
-        }
-        // Remove the duplicate products from the builders.
-        let autoLibProductIDs = autoLibProducts.map{ $0.ID }
-        let otherProductNames = otherProducts.map{ $0.name }
-        for packageBuilder in packageBuilders {
-            packageBuilder.products = packageBuilder.products.filter { $0.product.isDefaultLibrary ? !autoLibProductIDs.contains($0.product.ID) : !otherProductNames.contains($0.product.name) }
-        }
-    } else {
-        let duplicateProducts = productList
-            .spm_findDuplicateElements(by: \.name)
-            .map({ $0[0] })
-        // Emit diagnostics for duplicate products.
-        for dupProduct in duplicateProducts {
-            let packages = packageBuilders
-                .filter({ $0.products.contains(where: { $0.product.name == dupProduct.name }) })
-                .map{ $0.package.identity.description }
-                .sorted()
-            observabilityScope.emit(PackageGraphError.duplicateProduct(product: dupProduct.name, packages: packages))
-        }
-        // Remove the duplicate products from the builders.
-        for packageBuilder in packageBuilders {
-            packageBuilder.products = packageBuilder.products.filter { !duplicateProducts.map{$0.name}.contains($0.product.name) }
-        }
-    }
+    let dupProductsChecker = DuplicateProductsChecker(packageBuilders: packageBuilders)
+    try dupProductsChecker.run(lookupByProductIDs: moduleAliasingUsed, observabilityScope: observabilityScope)
 
     // The set of all target names.
     var allTargetNames = Set<String>()
@@ -473,14 +412,19 @@ private func createResolvedPackages(
                 return false
             })
 
+        let lookupByProductIDs = packageBuilder.package.manifest.disambiguateByProductIDs || moduleAliasingUsed
+
         // Get all the products from dependencies of this package.
         let productDependencies = packageBuilder.dependencies
             .flatMap({ (dependency: ResolvedPackageBuilder) -> [ResolvedProductBuilder] in
                 // Filter out synthesized products such as tests and implicit executables.
-                let explicit = Set(dependency.package.manifest.products.lazy.map({ $0.name }))
-                return dependency.products.filter({ explicit.contains($0.product.name) })
+                // Check if a dependency product is explicitly declared as a product in its package manifest
+                let manifestProducts = dependency.package.manifest.products.lazy.map { $0.name }
+                let explicitProducts = dependency.package.products.filter { manifestProducts.contains($0.name) }
+                let explicitIdsOrNames = Set(explicitProducts.lazy.map({ lookupByProductIDs ? $0.identity : $0.name }))
+                return dependency.products.filter({ lookupByProductIDs ? explicitIdsOrNames.contains($0.product.identity) : explicitIdsOrNames.contains($0.product.name) })
             })
-        let productDependencyMap = moduleAliasingUsed ? productDependencies.spm_createDictionary({ ($0.product.ID, $0) }) : productDependencies.spm_createDictionary({ ($0.product.name, $0) })
+        let productDependencyMap = lookupByProductIDs ? productDependencies.spm_createDictionary({ ($0.product.identity, $0) }) : productDependencies.spm_createDictionary({ ($0.product.name, $0) })
 
         // Establish dependencies in each target.
         for targetBuilder in packageBuilder.targets {
@@ -494,7 +438,7 @@ private func createResolvedPackages(
             for case .product(let productRef, let conditions) in targetBuilder.target.dependencies {
                 // Find the product in this package's dependency products.
                 // Look it up by ID if module aliasing is used, otherwise by name.
-                let product = moduleAliasingUsed ? productDependencyMap[productRef.ID] : productDependencyMap[productRef.name]
+                let product = lookupByProductIDs ? productDependencyMap[productRef.identity] : productDependencyMap[productRef.name]
                 guard let product = product else {
                     // Only emit a diagnostic if there are no other diagnostics.
                     // This avoids flooding the diagnostics with product not
@@ -503,7 +447,7 @@ private func createResolvedPackages(
                     if !observabilityScope.errorsReportedInAnyScope {
                         // Emit error if a product (not target) declared in the package is also a productRef (dependency)
                         let declProductsAsDependency = package.products.filter { product in
-                            product.name == productRef.name
+                            lookupByProductIDs ? product.identity == productRef.identity : product.name == productRef.name
                         }.map {$0.targets}.flatMap{$0}.filter { t in
                             t.name != productRef.name
                         }
@@ -570,6 +514,56 @@ fileprivate extension Product {
     }
 }
 
+private class DuplicateProductsChecker {
+    var packageIDToBuilder = [String: ResolvedPackageBuilder]()
+    var checkedPkgIDs = [String]()
+
+    init(packageBuilders: [ResolvedPackageBuilder]) {
+        for packageBuilder in packageBuilders {
+            let pkgID = packageBuilder.package.identity.description.lowercased()
+            packageIDToBuilder[pkgID] = packageBuilder
+        }
+    }
+
+    func run(lookupByProductIDs: Bool = false, observabilityScope: ObservabilityScope) throws {
+        var productToPkgMap = [String: [String]]()
+        for (_, pkgBuilder) in packageIDToBuilder {
+            let useProductIDs = pkgBuilder.package.manifest.disambiguateByProductIDs || lookupByProductIDs
+            let depProductRefs = pkgBuilder.package.targets.map{$0.dependencies}.flatMap{$0}.compactMap{$0.product}
+            for depRef in depProductRefs {
+                if let depPkg = depRef.package?.lowercased() {
+                    checkedPkgIDs.append(depPkg)
+                    let depProductIDs = packageIDToBuilder[depPkg]?.package.products.filter { $0.identity == depRef.identity }.map { useProductIDs && $0.isDefaultLibrary ? $0.identity : $0.name } ?? []
+                    for depID in depProductIDs {
+                        productToPkgMap[depID, default: []].append(depPkg)
+                    }
+                } else {
+                    let depPkgs = pkgBuilder.dependencies.filter{$0.products.contains{$0.product.name == depRef.name}}.map{$0.package.identity.description.lowercased()}
+                    productToPkgMap[depRef.name, default: []].append(contentsOf: depPkgs)
+                    checkedPkgIDs.append(contentsOf: depPkgs)
+                }
+            }
+            for (depIDOrName, depPkgs) in productToPkgMap.filter({Set($0.value).count > 1}) {
+                let name = depIDOrName.components(separatedBy: "_").dropFirst().joined(separator: "_")
+                throw PackageGraphError.duplicateProduct(product: name.isEmpty ? depIDOrName : name, packages: depPkgs.sorted())
+            }
+        }
+
+        let uncheckedPkgs = packageIDToBuilder.filter{!checkedPkgIDs.contains($0.key)}
+        for (pkgID, pkgBuilder) in uncheckedPkgs {
+            let productIDOrNames = pkgBuilder.products.map { pkgBuilder.package.manifest.disambiguateByProductIDs && $0.product.isDefaultLibrary ? $0.product.identity : $0.product.name }
+            for productIDOrName in productIDOrNames {
+                productToPkgMap[productIDOrName, default: []].append(pkgID)
+            }
+        }
+
+        let duplicates = productToPkgMap.filter({Set($0.value).count > 1})
+        for (productName, pkgs) in duplicates {
+            throw PackageGraphError.duplicateProduct(product: productName, packages: pkgs.sorted())
+        }
+    }
+}
+
 private func computePlatforms(
     package: Package,
     usingXCTest: Bool,

Sources/PackageLoading/Diagnostics.swift

@@ -40,6 +40,10 @@ extension Basics.Diagnostic {
         return .warning("ignoring duplicate product '\(product.name)'\(typeString)")
     }
 
+    static func duplicateProduct(name: String, package: String) -> Self {
+        return .warning("ignoring duplicate product '\(name)' from package '\(package)'")
+    }
+
     static func duplicateTargetDependency(dependency: String, target: String, package: String) -> Self {
         .warning("invalid duplicate target dependency declaration '\(dependency)' in target '\(target)' from package '\(package)'")
     }

Sources/PackageLoading/PackageBuilder.swift

@@ -765,8 +765,23 @@ public final class PackageBuilder {
         }
 
         // Check for duplicate target dependencies
-        dependencies.filter{$0.product?.moduleAliases == nil}.spm_findDuplicateElements(by: \.nameAndType).map(\.[0].name).forEach {
-            self.observabilityScope.emit(.duplicateTargetDependency(dependency: $0, target: potentialModule.name, package: self.identity.description))
+        if self.manifest.disambiguateByProductIDs {
+            let dupProductIDs = dependencies.compactMap{$0.product?.identity}.spm_findDuplicates()
+            for dupProductID in dupProductIDs {
+                let comps = dupProductID.components(separatedBy: "_")
+                let pkg = comps.first ?? ""
+                let name = comps.dropFirst().joined(separator: "_")
+                let dupProductName = name.isEmpty ? dupProductID : name
+                self.observabilityScope.emit(.duplicateProduct(name: dupProductName, package: pkg))
+            }
+            let dupTargetNames = dependencies.compactMap{$0.target?.name}.spm_findDuplicates()
+            for dupTargetName in dupTargetNames {
+                self.observabilityScope.emit(.duplicateTargetDependency(dependency: dupTargetName, target: potentialModule.name, package: self.identity.description))
+            }
+        } else {
+            dependencies.filter{$0.product?.moduleAliases == nil}.spm_findDuplicateElements(by: \.nameAndType).map(\.[0].name).forEach {
+                self.observabilityScope.emit(.duplicateTargetDependency(dependency: $0, target: potentialModule.name, package: self.identity.description))
+            }
         }
 
         // Create the build setting assignment table for this target.

Sources/PackageModel/PackageModel.swift

@@ -122,6 +122,12 @@ extension Package.Error: CustomStringConvertible {
     }
 }
 
+extension Manifest {
+    public var disambiguateByProductIDs: Bool {
+        return self.toolsVersion >= .vNext
+    }
+}
+
 extension ObservabilityMetadata {
     public static func packageMetadata(identity: PackageIdentity, kind: PackageReference.Kind) -> Self {
         var metadata = ObservabilityMetadata()

Sources/PackageModel/Product.swift

@@ -20,7 +20,7 @@ public class Product: Codable {
     public let name: String
 
     /// Fully qualified name for this product: package ID + name of this product
-    public let ID: String
+    public let identity: String
 
     /// The type of product to create.
     public let type: ProductType
@@ -54,7 +54,7 @@ public class Product: Codable {
         }
         self.name = name
         self.type = type
-        self.ID = package.description.lowercased() + "_" + name
+        self.identity = package.description.lowercased() + "_" + name
         self._targets = .init(wrappedValue: targets)
         self.testEntryPointPath = testEntryPointPath
     }

Sources/PackageModel/Target.swift

@@ -50,7 +50,7 @@ public class Target: PolymorphicCodableProtocol {
         public let moduleAliases: [String: String]?
 
         /// Fully qualified name for this product dependency: package ID + name of the product
-        public var ID: String {
+        public var identity: String {
             if let pkg = package {
                 return pkg.lowercased() + "_" + name
             }