Skip to content

Commit 0dff969

Browse files
committed
improve identity handling
motivation: package identity is critical to SwiftPM ability to dedup package reliably changes: * use package identity instead of manifest name to flatten the dependencies list * add more test for duplicate identity permutations across identity derived from URL and manifest name
1 parent aa681bd commit 0dff969

File tree

3 files changed

+331
-9
lines changed

3 files changed

+331
-9
lines changed

Bar-1.2.3.zip

476 Bytes
Binary file not shown.

Sources/PackageGraph/PackageGraph+Loading.swift

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ extension PackageGraph {
7474
// Sort all manifests toplogically.
7575
allManifests = try topologicalSort(inputManifests, successors: successors)
7676
}
77-
var flattenedManifests: [String: GraphLoadingNode] = [:]
77+
/*var flattenedManifests: [String: GraphLoadingNode] = [:]
7878
for node in allManifests {
7979
if let existing = flattenedManifests[node.manifest.name] {
8080
let merged = GraphLoadingNode(
@@ -85,9 +85,23 @@ extension PackageGraph {
8585
} else {
8686
flattenedManifests[node.manifest.name] = node
8787
}
88-
}
89-
allManifests = flattenedManifests.values.sorted(by: { $0.manifest.name < $1.manifest.name })
88+
}*/
9089

90+
var flattenedManifests: [PackageIdentity: GraphLoadingNode] = [:]
91+
for node in allManifests {
92+
let packageIdentity = identityResolver.resolveIdentity(for: node.manifest.packageLocation)
93+
if let existing = flattenedManifests[packageIdentity] {
94+
let merged = GraphLoadingNode(
95+
manifest: node.manifest,
96+
productFilter: existing.productFilter.union(node.productFilter)
97+
)
98+
flattenedManifests[packageIdentity] = merged
99+
} else {
100+
flattenedManifests[packageIdentity] = node
101+
}
102+
}
103+
allManifests = flattenedManifests.values.sorted(by: { identityResolver.resolveIdentity(for: $0.manifest.packageLocation) < identityResolver.resolveIdentity(for: $1.manifest.packageLocation) })
104+
91105
// Create the packages.
92106
var manifestToPackage: [Manifest: Package] = [:]
93107
for node in allManifests {
@@ -179,7 +193,7 @@ private func checkAllDependenciesAreUsed(_ rootPackages: [ResolvedPackage], _ di
179193
}
180194

181195
let dependencyIsUsed = dependency.products.contains(where: productDependencies.contains)
182-
if !dependencyIsUsed {
196+
if !dependencyIsUsed && !diagnostics.hasErrors {
183197
diagnostics.emit(.unusedDependency(dependency.name))
184198
}
185199
}
@@ -211,11 +225,18 @@ private func createResolvedPackages(
211225
}
212226

213227
// Create a map of package builders keyed by the package identity.
228+
// This is guaranteed to be unique so we can use spm_createDictionary
214229
let packageMapByIdentity: [PackageIdentity: ResolvedPackageBuilder] = packageBuilders.spm_createDictionary{
215230
let identity = identityResolver.resolveIdentity(for: $0.package.manifest.packageLocation)
216231
return (identity, $0)
217232
}
218-
let packageMapByName: [String: ResolvedPackageBuilder] = packageBuilders.spm_createDictionary{ ($0.package.name, $0) }
233+
234+
// in case packages have same manifest name this map can miss packages which will lead to missing product errors
235+
// our plan is to deprecate the use of manifest + dependency explicit name in target dependency lookup and instead lean 100% on identity
236+
// which means this map would go away too
237+
let packageMapByNameForTargetDependencyResolutionOnly = packageBuilders.reduce(into: [String: ResolvedPackageBuilder](), { partial, item in
238+
partial[item.package.name] = item
239+
})
219240

220241
// Scan and validate the dependencies
221242
for packageBuilder in packageBuilders {
@@ -235,7 +256,7 @@ private func createResolvedPackages(
235256
}
236257

237258
// Use the package name to lookup the dependency. The package name will be present in packages with tools version >= 5.2.
238-
if let explicitDependencyName = dependency.explicitNameForTargetDependencyResolutionOnly, let resolvedPackage = packageMapByName[explicitDependencyName] {
259+
if let explicitDependencyName = dependency.explicitNameForTargetDependencyResolutionOnly, let resolvedPackage = packageMapByNameForTargetDependencyResolutionOnly[explicitDependencyName] {
239260
guard !dependencies.contains(resolvedPackage) else {
240261
// check if this resolvedPackage already listed in the dependencies
241262
// this means that the dependencies share the same name
@@ -409,7 +430,7 @@ private func createResolvedPackages(
409430
if let packageName = productRef.package {
410431
// Find the declared package and check that it contains
411432
// the product we found above.
412-
guard let dependencyPackage = packageMapByName[packageName], dependencyPackage.products.contains(product) else {
433+
guard let dependencyPackage = packageMapByNameForTargetDependencyResolutionOnly[packageName], dependencyPackage.products.contains(product) else {
413434
let error = PackageGraphError.productDependencyIncorrectPackage(
414435
name: productRef.name,
415436
package: packageName

0 commit comments

Comments
 (0)