From ac3ebf260cde4f9042b633de21300bff597b5d72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=86=80=E5=8D=93=E7=96=8C?= <55120045+WowbaggersLiquidLunch@users.noreply.github.com> Date: Sat, 19 Jun 2021 12:59:06 -0400 Subject: [PATCH 1/2] [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. --- Tests/PackageGraphTests/PubgrubTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/PackageGraphTests/PubgrubTests.swift b/Tests/PackageGraphTests/PubgrubTests.swift index e4c83ee87a2..42b81674741 100644 --- a/Tests/PackageGraphTests/PubgrubTests.swift +++ b/Tests/PackageGraphTests/PubgrubTests.swift @@ -2166,7 +2166,7 @@ class DependencyGraphBuilder { _ package: String, at version: Version, toolsVersion: ToolsVersion? = nil, - with dependencies: OrderedDictionary> = [:] + with dependencies: KeyValuePairs> = [:] ) { serve(package, at: .version(version), toolsVersion: toolsVersion, with: dependencies) } @@ -2175,7 +2175,7 @@ class DependencyGraphBuilder { _ package: String, at version: BoundVersion, toolsVersion: ToolsVersion? = nil, - with dependencies: OrderedDictionary> = [:] + with dependencies: KeyValuePairs> = [:] ) { let packageReference = reference(for: package) let container = self.containers[package] ?? MockContainer(package: packageReference) From dfd42178bc8ab5a475c578c9fb6213d830350d50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=86=80=E5=8D=93=E7=96=8C?= <55120045+WowbaggersLiquidLunch@users.noreply.github.com> Date: Mon, 21 Jun 2021 19:39:47 -0400 Subject: [PATCH 2/2] [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. --- Tests/PackageGraphTests/PubgrubTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/PackageGraphTests/PubgrubTests.swift b/Tests/PackageGraphTests/PubgrubTests.swift index 42b81674741..29c7dec57e4 100644 --- a/Tests/PackageGraphTests/PubgrubTests.swift +++ b/Tests/PackageGraphTests/PubgrubTests.swift @@ -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 }