Skip to content

Commit

Permalink
[PubgrubTests] Handle duplicate product keys in `DependencyGraphBuild…
Browse files Browse the repository at this point in the history
…er.serve` (#3564)

* [PubgrubTests] Replace `OrderedDictionary` with `KeyValuePairs` in `DependencyGraphBuilder.serve`

[One of the callers](https://github.com/apple/swift-package-manager/blob/20eba126ffa32088abb46d642bd0481dbae212ef/Tests/PackageGraphTests/PubgrubTests.swift#L472-L475) of the function passes in a dictionary literal with duplicate keys. Although the logic in this function suggests that duplicate keys are allowed, `TSCBasic.OrderedDictionary` preserves only [the final](https://github.com/apple/swift-tools-support-core/blob/21a79185b2ea8f89b9253ed8cdf2338bf564c499/Sources/TSCBasic/OrderedDictionary.swift#L97-L99) of all duplicate keys' values in a dictionary literal. In addition, with #3533, Swift Collections' `OrderedDictionary` [traps](https://github.com/apple/swift-collections/blob/c0549b6284aadd5fd13156316f43fcb43c7fca77/Sources/OrderedCollections/OrderedDictionary/OrderedDictionary%2BInitializers.swift#L88-L91) on duplicate keys. `KeyValuePairs` seems like the right replacement that allows duplicate keys.

* [PubgrubTests] append, not assign, `packageDependencies` to value in dictionary keyed by `product`

`dependencies` passed to `DependencyGraphBuilder.serve` can have duplicate product keys, so when `dependencies` are iterated, the same `product` can appear more than once, each time with possibly different `filteredDependencies`. Assigning `packageDependencies` mapped from a `product`'s `filteredDependencies` to the value in a different dictionary keyed by the same `product` overrides any existing value from a previous assignment. Appending `packageDependencies` instead preserves all previous values.
  • Loading branch information
WowbaggersLiquidLunch authored Jun 22, 2021
1 parent 26275b6 commit 065536e
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions Tests/PackageGraphTests/PubgrubTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2166,7 +2166,7 @@ class DependencyGraphBuilder {
_ package: String,
at version: Version,
toolsVersion: ToolsVersion? = nil,
with dependencies: OrderedDictionary<String, OrderedDictionary<String, (PackageRequirement, ProductFilter)>> = [:]
with dependencies: KeyValuePairs<String, OrderedDictionary<String, (PackageRequirement, ProductFilter)>> = [:]
) {
serve(package, at: .version(version), toolsVersion: toolsVersion, with: dependencies)
}
Expand All @@ -2175,7 +2175,7 @@ class DependencyGraphBuilder {
_ package: String,
at version: BoundVersion,
toolsVersion: ToolsVersion? = nil,
with dependencies: OrderedDictionary<String, OrderedDictionary<String, (PackageRequirement, ProductFilter)>> = [:]
with dependencies: KeyValuePairs<String, OrderedDictionary<String, (PackageRequirement, ProductFilter)>> = [:]
) {
let packageReference = reference(for: package)
let container = self.containers[package] ?? MockContainer(package: packageReference)
Expand All @@ -2193,7 +2193,7 @@ class DependencyGraphBuilder {
let packageDependencies: [MockContainer.Dependency] = filteredDependencies.map {
(container: reference(for: $0), requirement: $1.0, products: $1.1)
}
container.dependencies[version.description, default: [:]][product] = packageDependencies
container.dependencies[version.description, default: [:]][product, default: []] += packageDependencies
}
self.containers[package] = container
}
Expand Down

0 comments on commit 065536e

Please sign in to comment.