Skip to content

Commit c708d0d

Browse files
authored
Do not use product identifier based lookup for older tools-versions (#6793)
When we introduced module aliasing, we tried to paper over the fact that there are no product aliases by changing the lookup under certain conditions. It looks like one of these conditions is "one package in the graph uses aliasing" which breaks if the graph contains any packages with pre-5.2 tools-versions because they lack the information to do a identity based lookup. This changes opts packages with older tools-versions out of the identity based lookup. This implies that it is not possible to use aliasing in certain situations, but that seems preferrable to not being to use aliasing at all in affected package graphs. rdar://113506853
1 parent 8aeb5e7 commit c708d0d

File tree

2 files changed

+225
-21
lines changed

2 files changed

+225
-21
lines changed

Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,11 @@ private func createResolvedPackages(
402402
}
403403
}
404404

405-
let dupProductsChecker = DuplicateProductsChecker(packageBuilders: packageBuilders)
405+
let dupProductsChecker = DuplicateProductsChecker(
406+
packageBuilders: packageBuilders,
407+
moduleAliasingUsed: moduleAliasingUsed,
408+
observabilityScope: observabilityScope
409+
)
406410
try dupProductsChecker.run(lookupByProductIDs: moduleAliasingUsed, observabilityScope: observabilityScope)
407411

408412
// The set of all target names.
@@ -430,7 +434,8 @@ private func createResolvedPackages(
430434
return false
431435
})
432436

433-
let lookupByProductIDs = packageBuilder.package.manifest.disambiguateByProductIDs || moduleAliasingUsed
437+
let packageDoesNotSupportProductAliases = packageBuilder.package.doesNotSupportProductAliases
438+
let lookupByProductIDs = !packageDoesNotSupportProductAliases && (packageBuilder.package.manifest.disambiguateByProductIDs || moduleAliasingUsed)
434439

435440
// Get all the products from dependencies of this package.
436441
let productDependencies = packageBuilder.dependencies
@@ -457,9 +462,11 @@ private func createResolvedPackages(
457462
productDependencies.map { ($0.product.name, $0) },
458463
uniquingKeysWith: { lhs, _ in
459464
let duplicates = productDependencies.filter { $0.product.name == lhs.product.name }
460-
throw PackageGraphError.duplicateProduct(
461-
product: lhs.product.name,
462-
packages: duplicates.map(\.packageBuilder.package)
465+
throw emitDuplicateProductDiagnostic(
466+
productName: lhs.product.name,
467+
packages: duplicates.map(\.packageBuilder.package),
468+
moduleAliasingUsed: moduleAliasingUsed,
469+
observabilityScope: observabilityScope
463470
)
464471
}
465472
)
@@ -600,6 +607,31 @@ private func createResolvedPackages(
600607
return try packageBuilders.map{ try $0.construct() }
601608
}
602609

610+
private func emitDuplicateProductDiagnostic(
611+
productName: String,
612+
packages: [Package],
613+
moduleAliasingUsed: Bool,
614+
observabilityScope: ObservabilityScope
615+
) -> PackageGraphError {
616+
if moduleAliasingUsed {
617+
packages.filter { $0.doesNotSupportProductAliases }.forEach {
618+
// Emit an additional warning about product aliasing in case of older tools-versions.
619+
observabilityScope.emit(warning: "product aliasing requires tools-version 5.2 or later, so it is not supported by '\($0.identity.description)'")
620+
}
621+
}
622+
return PackageGraphError.duplicateProduct(
623+
product: productName,
624+
packages: packages
625+
)
626+
}
627+
628+
fileprivate extension Package {
629+
var doesNotSupportProductAliases: Bool {
630+
// We can never use the identity based lookup for older packages because they lack the necessary information.
631+
return self.manifest.toolsVersion < .v5_2
632+
}
633+
}
634+
603635
fileprivate struct Pair: Hashable {
604636
let package1: Package
605637
let package2: Package
@@ -625,11 +657,16 @@ private class DuplicateProductsChecker {
625657
var packageIDToBuilder = [PackageIdentity: ResolvedPackageBuilder]()
626658
var checkedPkgIDs = [PackageIdentity]()
627659

628-
init(packageBuilders: [ResolvedPackageBuilder]) {
660+
let moduleAliasingUsed: Bool
661+
let observabilityScope: ObservabilityScope
662+
663+
init(packageBuilders: [ResolvedPackageBuilder], moduleAliasingUsed: Bool, observabilityScope: ObservabilityScope) {
629664
for packageBuilder in packageBuilders {
630665
let pkgID = packageBuilder.package.identity
631666
self.packageIDToBuilder[pkgID] = packageBuilder
632667
}
668+
self.moduleAliasingUsed = moduleAliasingUsed
669+
self.observabilityScope = observabilityScope
633670
}
634671

635672
func run(lookupByProductIDs: Bool = false, observabilityScope: ObservabilityScope) throws {
@@ -657,9 +694,11 @@ private class DuplicateProductsChecker {
657694
}
658695
for (depIDOrName, depPkgs) in productToPkgMap.filter({Set($0.value).count > 1}) {
659696
let name = depIDOrName.components(separatedBy: "_").dropFirst().joined(separator: "_")
660-
throw PackageGraphError.duplicateProduct(
661-
product: name.isEmpty ? depIDOrName : name,
662-
packages: depPkgs.compactMap{ packageIDToBuilder[$0]?.package }
697+
throw emitDuplicateProductDiagnostic(
698+
productName: name.isEmpty ? depIDOrName : name,
699+
packages: depPkgs.compactMap{ packageIDToBuilder[$0]?.package },
700+
moduleAliasingUsed: self.moduleAliasingUsed,
701+
observabilityScope: self.observabilityScope
663702
)
664703
}
665704
}
@@ -683,9 +722,11 @@ private class DuplicateProductsChecker {
683722

684723
let duplicates = productToPkgMap.filter{ $0.value.count > 1 }
685724
for (productName, pkgs) in duplicates {
686-
throw PackageGraphError.duplicateProduct(
687-
product: productName,
688-
packages: pkgs.compactMap{ packageIDToBuilder[$0]?.package }
725+
throw emitDuplicateProductDiagnostic(
726+
productName: productName,
727+
packages: pkgs.compactMap{ packageIDToBuilder[$0]?.package },
728+
moduleAliasingUsed: self.moduleAliasingUsed,
729+
observabilityScope: self.observabilityScope
689730
)
690731
}
691732
}

0 commit comments

Comments
 (0)