clarfiy reveresedVersions/versions in pubgrub

Sources/PackageGraph/LocalPackageContainer.swift

@@ -89,11 +89,11 @@ public class LocalPackageContainer: PackageContainer {
         fatalError("This should never be called")
     }
 
-    public func versions(filter isIncluded: (Version) -> Bool) throws -> AnySequence<Version> {
+    public func toolsVersionsAppropriateVersionsDescending() throws -> [Version] {
         fatalError("This should never be called")
     }
 
-    public func reversedVersions() throws -> [Version] {
+    public func versionsAscending() throws -> [Version] {
         fatalError("This should never be called")
     }
 

Sources/PackageGraph/PackageContainer.swift

@@ -49,10 +49,19 @@ public protocol PackageContainer {
     /// The list will be returned in sorted order, with the latest version *first*.
     /// All versions will not be requested at once. Resolver will request the next one only
     /// if the previous one did not satisfy all constraints.
-    func versions(filter isIncluded: (Version) -> Bool) throws -> AnySequence<Version>
+    func toolsVersionsAppropriateVersionsDescending() throws -> [Version]
+
+    /// Get the list of versions in the repository sorted in the ascending order, that is the earliest
+    /// version appears first.
+    func versionsAscending() throws -> [Version]
+
+    /// Get the list of versions in the repository sorted in the descending order, that is the latest
+    /// version appears first.
+    func versionsDescending() throws -> [Version]
 
     /// Get the list of versions in the repository sorted in the reverse order, that is the latest
     /// version appears first.
+    @available(*, deprecated, message: "use versionsDescending instead")
     func reversedVersions() throws -> [Version]
 
     // FIXME: We should perhaps define some particularly useful error codes
@@ -90,6 +99,17 @@ public protocol PackageContainer {
     func getUpdatedIdentifier(at boundVersion: BoundVersion) throws -> PackageReference
 }
 
+extension PackageContainer {
+    @available(*, deprecated, message: "use versionsDescending instead")
+    public func reversedVersions() throws -> [Version] {
+        try self.versionsDescending()
+    }
+
+    public func versionsDescending() throws -> [Version] {
+        try self.versionsAscending().reversed()
+    }
+}
+
 // MARK: -
 
 /// An individual constraint onto a container.

Sources/PackageGraph/Pubgrub/PubgrubDependencyResolver.swift

@@ -1094,7 +1094,7 @@ private final class PubGrubPackageContainer {
         if let pinnedVersion = self.pinnedVersion, requirement.contains(pinnedVersion) {
             return 1
         }
-        return try self.underlying.reversedVersions().filter(requirement.contains).count
+        return try self.underlying.versionsDescending().filter(requirement.contains).count
     }
 
     /// Computes the bounds of the given range against the versions available in the package.
@@ -1105,7 +1105,7 @@ private final class PubGrubPackageContainer {
         var includeLowerBound = true
         var includeUpperBound = true
 
-        let versions = try self.underlying.reversedVersions()
+        let versions = try self.underlying.versionsDescending()
 
         if let last = versions.last, range.lowerBound < last {
             includeLowerBound = false
@@ -1131,13 +1131,13 @@ private final class PubGrubPackageContainer {
         }
 
         // Return the highest version that is allowed by the input requirement.
-        return try self.underlying.reversedVersions().first { versionSet.contains($0) }
+        return try self.underlying.versionsDescending().first { versionSet.contains($0) }
     }
 
     /// Compute the bounds of incompatible tools version starting from the given version.
     private func computeIncompatibleToolsVersionBounds(fromVersion: Version) throws -> VersionSetSpecifier {
         assert(!self.underlying.isToolsVersionCompatible(at: fromVersion))
-        let versions: [Version] = try self.underlying.reversedVersions().reversed()
+        let versions: [Version] = try self.underlying.versionsAscending()
 
         // This is guaranteed to be present.
         let idx = versions.firstIndex(of: fromVersion)!
@@ -1358,7 +1358,7 @@ private final class PubGrubPackageContainer {
             return result
         }
 
-        let versions: [Version] = try self.underlying.reversedVersions().reversed()
+        let versions: [Version] = try self.underlying.versionsAscending()
 
         guard let idx = versions.firstIndex(of: firstVersion) else {
             assertionFailure()

Sources/PackageGraph/RepositoryPackageContainer.swift

@@ -67,7 +67,6 @@ public class RepositoryPackageContainer: PackageContainer, CustomStringConvertib
     private var dependenciesCacheLock = Lock()
 
     private var knownVersionsCache = ThreadSafeBox<[Version: String]>()
-    private var reversedVersionsCache = ThreadSafeBox<[Version]>()
     private var manifestsCache = ThreadSafeKeyValueStore<Revision, Manifest>()
     private var toolsVersionsCache = ThreadSafeKeyValueStore<Version, ToolsVersion>()
 
@@ -106,17 +105,15 @@ public class RepositoryPackageContainer: PackageContainer, CustomStringConvertib
             })
         }
     }
-
-    public func reversedVersions() throws -> [Version] {
-        try self.reversedVersionsCache.memoize() {
-            [Version](try self.knownVersions().keys).sorted().reversed()
-        }
+
+    public func versionsAscending() throws -> [Version] {
+        [Version](try self.knownVersions().keys).sorted()
     }
 
     /// The available version list (in reverse order).
-    public func versions(filter isIncluded: (Version) -> Bool) throws -> AnySequence<Version> {
-        let reversedVersions = try self.reversedVersions()
-        return AnySequence(reversedVersions.filter(isIncluded).lazy.filter({
+    public func toolsVersionsAppropriateVersionsDescending() throws -> [Version] {
+        let reversedVersions = try self.versionsDescending()
+        return reversedVersions.lazy.filter({
             // If we have the result cached, return that.
             if let result = self.validToolsVersionsCache[$0] {
                 return result
@@ -126,7 +123,7 @@ public class RepositoryPackageContainer: PackageContainer, CustomStringConvertib
             let isValid = (try? self.toolsVersion(for: $0)).flatMap(self.isValidToolsVersion(_:)) ?? false
             self.validToolsVersionsCache[$0] = isValid
             return isValid
-        }))
+        })
     }
 
     public func getTag(for version: Version) -> String? {

Sources/SPMTestSupport/MockPackageContainer.swift

@@ -38,11 +38,11 @@ public class MockPackageContainer: PackageContainer {
     }
 
     public let _versions: [Version]
-    public func versions(filter isIncluded: (Version) -> Bool) throws -> AnySequence<Version> {
-        return AnySequence(_versions.filter(isIncluded))
+    public func toolsVersionsAppropriateVersionsDescending() throws -> [Version] {
+        return try self.versionsDescending()
     }
 
-    public func reversedVersions() throws -> [Version] {
+    public func versionsAscending() throws -> [Version] {
         return _versions
     }
 
@@ -98,8 +98,7 @@ public class MockPackageContainer: PackageContainer {
         dependencies: [String: [Dependency]] = [:]
     ) {
         self.name = name
-        let versions = dependencies.keys.compactMap(Version.init(string:))
-        self._versions = versions.sorted().reversed()
+        self._versions = dependencies.keys.compactMap(Version.init(string:)).sorted()
         self.dependencies = dependencies
     }
 }

Sources/Workspace/ResolverPrecomputationProvider.swift

@@ -117,7 +117,7 @@ private struct LocalPackageContainer: PackageContainer {
         }
     }
 
-    func reversedVersions() throws -> [Version] {
+    func versionsAscending() throws -> [Version] {
         if let version = dependency?.state.checkout?.version {
             return [version]
         } else {
@@ -138,8 +138,8 @@ private struct LocalPackageContainer: PackageContainer {
         return currentToolsVersion
     }
 
-    func versions(filter isIncluded: (Version) -> Bool) throws -> AnySequence<Version> {
-        return AnySequence(try self.reversedVersions())
+    func toolsVersionsAppropriateVersionsDescending() throws -> [Version] {
+        return try self.versionsDescending()
     }
 
     func getDependencies(at version: Version, productFilter: ProductFilter) throws -> [PackageContainerConstraint] {

Sources/Workspace/Workspace.swift

@@ -2391,7 +2391,7 @@ extension Workspace {
             // FIXME: this should not block
             let container = try temp_await { containerProvider.getContainer(for: package, skipUpdate: true, on: self.queue, completion: $0) } as! RepositoryPackageContainer
             guard let tag = container.getTag(for: version) else {
-                throw StringError("Internal error: please file a bug at https://bugs.swift.org with this info -- unable to get tag for \(package) \(version); available versions \(try container.reversedVersions())")
+                throw StringError("Internal error: please file a bug at https://bugs.swift.org with this info -- unable to get tag for \(package) \(version); available versions \(try container.versionsDescending())")
             }
             let revision = try container.getRevision(forTag: tag)
             checkoutState = CheckoutState(revision: revision, version: version)

Tests/PackageGraphTests/PubgrubTests.swift

@@ -1926,20 +1926,19 @@ public class MockContainer: PackageContainer {
     var toolsVersion: ToolsVersion = ToolsVersion.currentToolsVersion
     var versionsToolsVersions = [Version: ToolsVersion]()
 
-    public var _versions: [BoundVersion]
+    private var _versions: [BoundVersion]
 
-    public func versions(filter isIncluded: (Version) -> Bool) throws -> AnySequence<Version> {
+    // TODO: this does not actually do anything with the tools-version
+    public func toolsVersionsAppropriateVersionsDescending() throws -> [Version] {
         var versions: [Version] = []
-        for version in self._versions {
+        for version in self._versions.reversed() {
             guard case .version(let v) = version else { continue }
-            if isIncluded(v) {
-                versions.append(v)
-            }
+            versions.append(v)
         }
-        return AnySequence(versions)
+        return versions
     }
 
-    public func reversedVersions() throws ->  [Version] {
+    public func versionsAscending() throws ->  [Version] {
         var versions: [Version] = []
         for version in self._versions {
             guard case .version(let v) = version else { continue }
@@ -1954,7 +1953,7 @@ public class MockContainer: PackageContainer {
             return self.toolsVersion == toolsVersion
         }
 
-        return (try? self.versions(filter: { _ in true }).contains(version)) ?? false
+        return (try? self.toolsVersionsAppropriateVersionsDescending().contains(version)) ?? false
     }
 
     public func toolsVersion(for version: Version) throws -> ToolsVersion {
@@ -1999,6 +1998,17 @@ public class MockContainer: PackageContainer {
         return name
     }
 
+    func appendVersion(_ version: BoundVersion) {
+        self._versions.append(version)
+        self._versions = self._versions
+            .sorted(by: { lhs, rhs -> Bool in
+                guard case .version(let lv) = lhs, case .version(let rv) = rhs else {
+                    return true
+                }
+                return lv < rv
+            })
+    }
+
     public convenience init(
         name: PackageReference,
         unversionedDependencies: [(package: PackageReference, requirement: PackageRequirement, productFilter: ProductFilter)]
@@ -2038,7 +2048,6 @@ public class MockContainer: PackageContainer {
         let versions = dependencies.keys.compactMap(Version.init(string:))
         self._versions = versions
             .sorted()
-            .reversed()
             .map(BoundVersion.version)
     }
 }
@@ -2114,15 +2123,7 @@ class DependencyGraphBuilder {
             container.versionsToolsVersions[v] = toolsVersion ?? container.toolsVersion
         }
 
-        container._versions.append(version)
-        container._versions = container._versions
-            .sorted(by: { lhs, rhs -> Bool in
-                guard case .version(let lv) = lhs, case .version(let rv) = rhs else {
-                    return true
-                }
-                return lv < rv
-            })
-            .reversed()
+        container.appendVersion(version)
 
         if container.dependencies[version.description] == nil {
             container.dependencies[version.description] = [:]

Tests/PackageGraphTests/RepositoryPackageContainerProviderTests.swift

@@ -180,7 +180,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
 
         let ref = PackageReference(identity: PackageIdentity(path: repoPath), path: repoPath.pathString)
         let container = try provider.getContainer(for: ref, skipUpdate: false)
-        let v = try container.versions(filter: { _ in true }).map { $0 }
+        let v = try container.toolsVersionsAppropriateVersionsDescending().map { $0 }
         XCTAssertEqual(v, ["2.0.3", "1.0.3", "1.0.2", "1.0.1", "1.0.0"])
     }
 
@@ -235,7 +235,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
             let provider = createProvider(ToolsVersion(version: "4.0.0"))
             let ref = PackageReference(identity: PackageIdentity(url: specifier.url), path: specifier.url)
             let container = try provider.getContainer(for: ref, skipUpdate: false)
-            let v = try container.versions(filter: { _ in true }).map { $0 }
+            let v = try container.toolsVersionsAppropriateVersionsDescending().map { $0 }
             XCTAssertEqual(v, ["1.0.1"])
         }
 
@@ -244,7 +244,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
             let ref = PackageReference(identity: PackageIdentity(url: specifier.url), path: specifier.url)
             let container = try provider.getContainer(for: ref, skipUpdate: false) as! RepositoryPackageContainer
             XCTAssertTrue(container.validToolsVersionsCache.isEmpty)
-            let v = try container.versions(filter: { _ in true }).map { $0 }
+            let v = try container.toolsVersionsAppropriateVersionsDescending().map { $0 }
             XCTAssertEqual(container.validToolsVersionsCache["1.0.0"], false)
             XCTAssertEqual(container.validToolsVersionsCache["1.0.1"], true)
             XCTAssertEqual(container.validToolsVersionsCache["1.0.2"], true)
@@ -256,7 +256,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
             let provider = createProvider(ToolsVersion(version: "3.0.0"))
             let ref = PackageReference(identity: PackageIdentity(url: specifier.url), path: specifier.url)
             let container = try provider.getContainer(for: ref, skipUpdate: false)
-            let v = try container.versions(filter: { _ in true }).map { $0 }
+            let v = try container.toolsVersionsAppropriateVersionsDescending().map { $0 }
             XCTAssertEqual(v, [])
         }
 
@@ -312,7 +312,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
         )
         let ref = PackageReference(identity: PackageIdentity(path: repoPath), path: repoPath.pathString)
         let container = try provider.getContainer(for: ref, skipUpdate: false)
-        let v = try container.versions(filter: { _ in true }).map { $0 }
+        let v = try container.toolsVersionsAppropriateVersionsDescending().map { $0 }
         XCTAssertEqual(v, ["1.0.4-alpha", "1.0.2-dev.2", "1.0.2-dev", "1.0.1", "1.0.0", "1.0.0-beta.1", "1.0.0-alpha.1"])
     }
 
@@ -352,7 +352,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
         )
         let ref = PackageReference(identity: PackageIdentity(path: repoPath), path: repoPath.pathString)
         let container = try provider.getContainer(for: ref, skipUpdate: false)
-        let v = try container.versions(filter: { _ in true }).map { $0 }
+        let v = try container.toolsVersionsAppropriateVersionsDescending().map { $0 }
         XCTAssertEqual(v, ["2.0.1", "1.0.4", "1.0.2", "1.0.1", "1.0.0"])
     }