Skip to content

Commit e026c16

Browse files
authored
Revert "refine location comparison when pre-loading package (#7002)" (#7031)
I've verified that the reverted commit removes required packages from `Package.resolved` in certain dependency graphs. This reverts commit 1491d34. ``` # Conflicts: # Sources/Workspace/Workspace+Manifests.swift # Sources/Workspace/Workspace.swift ``` Resolves rdar://117381924.
1 parent a805a37 commit e026c16

File tree

8 files changed

+27
-502
lines changed

8 files changed

+27
-502
lines changed

Sources/PackageGraph/PubGrub/ContainerProvider.swift

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,14 @@ final class ContainerProvider {
6262
completion: @escaping (Result<PubGrubPackageContainer, Error>) -> Void
6363
) {
6464
// Return the cached container, if available.
65-
if let container = self.containersCache[comparingLocation: package] {
65+
if let container = self.containersCache[package], package.equalsIncludingLocation(container.package) {
6666
return completion(.success(container))
6767
}
6868

6969
if let prefetchSync = self.prefetches[package] {
7070
// If this container is already being prefetched, wait for that to complete
7171
prefetchSync.notify(queue: .sharedConcurrent) {
72-
if let container = self.containersCache[comparingLocation: package] {
72+
if let container = self.containersCache[package], package.equalsIncludingLocation(container.package) {
7373
// should be in the cache once prefetch completed
7474
return completion(.success(container))
7575
} else {
@@ -125,12 +125,3 @@ final class ContainerProvider {
125125
}
126126
}
127127
}
128-
129-
extension ThreadSafeKeyValueStore where Key == PackageReference, Value == PubGrubPackageContainer {
130-
subscript(comparingLocation package: PackageReference) -> PubGrubPackageContainer? {
131-
if let container = self[package], container.package.equalsIncludingLocation(package) {
132-
return container
133-
}
134-
return .none
135-
}
136-
}

Sources/PackageModel/PackageIdentity.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ public struct CanonicalPackageLocation: Equatable, CustomStringConvertible {
424424
}
425425

426426
/// Similar to `CanonicalPackageLocation` but differentiates based on the scheme.
427-
public struct CanonicalPackageURL: Equatable, CustomStringConvertible {
427+
public struct CanonicalPackageURL: CustomStringConvertible {
428428
public let description: String
429429
public let scheme: String?
430430

Sources/PackageModel/PackageReference.swift

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -161,24 +161,7 @@ extension PackageReference: Equatable {
161161

162162
// TODO: consider rolling into Equatable
163163
public func equalsIncludingLocation(_ other: PackageReference) -> Bool {
164-
if self.identity != other.identity {
165-
return false
166-
}
167-
if self.canonicalLocation != other.canonicalLocation {
168-
return false
169-
}
170-
switch (self.kind, other.kind) {
171-
case (.remoteSourceControl(let lurl), .remoteSourceControl(let rurl)):
172-
return lurl.canonicalURL == rurl.canonicalURL
173-
default:
174-
return true
175-
}
176-
}
177-
}
178-
179-
extension SourceControlURL {
180-
var canonicalURL: CanonicalPackageURL {
181-
CanonicalPackageURL(self.absoluteString)
164+
return self.identity == other.identity && self.canonicalLocation == other.canonicalLocation
182165
}
183166
}
184167

Sources/Workspace/ManagedDependency.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ extension Workspace {
196196

197197
// When loading manifests in Workspace, there are cases where we must also compare the location
198198
// as it may attempt to load manifests for dependencies that have the same identity but from a different location
199-
// (e.g. dependency is changed to a fork with the same identity)
199+
// (e.g. dependency is changed to a fork with the same identity)
200200
public subscript(comparingLocation package: PackageReference) -> ManagedDependency? {
201201
if let dependency = self.dependencies[package.identity], dependency.packageRef.equalsIncludingLocation(package) {
202202
return dependency

Sources/Workspace/Workspace+Dependencies.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -859,8 +859,9 @@ extension Workspace {
859859
// a required dependency that is already loaded (managed) should be represented in the pins store.
860860
// also comparing location as it may have changed at this point
861861
if requiredDependencies.contains(where: { $0.equalsIncludingLocation(dependency.packageRef) }) {
862+
let pin = pinsStore.pins[dependency.packageRef.identity]
862863
// if pin not found, or location is different (it may have changed at this point) pin it
863-
if pinsStore.pins[comparingLocation: dependency.packageRef] == .none {
864+
if !(pin?.packageRef.equalsIncludingLocation(dependency.packageRef) ?? false) {
864865
pinsStore.pin(dependency)
865866
}
866867
} else if let pin = pinsStore.pins[dependency.packageRef.identity] {

Sources/Workspace/Workspace+Manifests.swift

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,6 @@ extension Workspace {
198198
}
199199
}
200200

201-
let topLevelDependencies = root.packages.flatMap { $1.manifest.dependencies.map(\.packageRef) }
202-
203201
var requiredIdentities: OrderedCollections.OrderedSet<PackageReference> = []
204202
_ = transitiveClosure(inputNodes) { node in
205203
node.manifest.dependenciesRequired(for: node.productFilter).compactMap { dependency in
@@ -211,28 +209,17 @@ extension Workspace {
211209
if existing.canonicalLocation == package.canonicalLocation {
212210
// same literal location is fine
213211
if existing.locationString != package.locationString {
214-
// we prefer the top level dependencies
215-
if topLevelDependencies.contains(where: {
216-
$0.locationString == existing.locationString
217-
}) {
218-
observabilityScope.emit(debug: """
219-
similar variants of package '\(package.identity)' \
220-
found at '\(package.locationString)' and '\(existing.locationString)'. \
221-
using preferred root variant '\(existing.locationString)'
222-
""")
223-
} else {
224-
let preferred = [existing, package].sorted(by: {
225-
$0.locationString > $1.locationString
226-
}).first! // safe
227-
observabilityScope.emit(debug: """
228-
similar variants of package '\(package.identity)' \
229-
found at '\(package.locationString)' and '\(existing.locationString)'. \
230-
using preferred variant '\(preferred.locationString)'
231-
""")
232-
if preferred.locationString != existing.locationString {
233-
requiredIdentities.remove(existing)
234-
requiredIdentities.insert(preferred, at: index)
235-
}
212+
let preferred = [existing, package].sorted(by: {
213+
$0.locationString > $1.locationString
214+
}).first! // safe
215+
observabilityScope.emit(debug: """
216+
similar variants of package '\(package.identity)' \
217+
found at '\(package.locationString)' and '\(existing.locationString)'. \
218+
using preferred variant '\(preferred.locationString)'
219+
""")
220+
if preferred.locationString != existing.locationString {
221+
requiredIdentities.remove(existing)
222+
requiredIdentities.insert(preferred, at: index)
236223
}
237224
}
238225
} else {

Sources/Workspace/Workspace+Pinning.swift

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ extension Workspace {
4949
needsUpdate = true
5050
} else {
5151
for dependency in dependenciesToPin {
52-
if let pin = storedPinStore.pins[comparingLocation: dependency.packageRef] {
53-
if pin.state != PinsStore.Pin(dependency)?.state {
52+
if let pin = storedPinStore.pins.first(where: { $0.value.packageRef.equalsIncludingLocation(dependency.packageRef) }) {
53+
if pin.value.state != PinsStore.Pin(dependency)?.state {
5454
needsUpdate = true
5555
break
5656
}
@@ -180,12 +180,3 @@ extension PinsStore.PinState {
180180
}
181181
}
182182
}
183-
184-
extension PinsStore.Pins {
185-
subscript(comparingLocation package: PackageReference) -> PinsStore.Pin? {
186-
if let pin = self[package.identity], pin.packageRef.equalsIncludingLocation(package) {
187-
return pin
188-
}
189-
return .none
190-
}
191-
}

0 commit comments

Comments
 (0)