Improve version-specific manifest selection (#6801)

Currently, we only pick a non-exact match for version-specific manifests if its tools-version is *higher* than the regular one. That seems like an edge case only and we should *also* pick it if the regular manifest is incompatible. This both matches intuition as well as our current documentation on the topic. It should also be backwards-compatible with the current behavior since the new behavior only comes into play if we would have previously failed to load a manifest at all.

Co-authored-by: Anders Bertelrud <[email protected]>

Author: neonichu

Sources/PackageLoading/ToolsVersionParser.swift

@@ -583,8 +583,6 @@ extension ManifestLoader {
             }
         }
 
-        // Otherwise, check if there is a version-specific manifest that has
-        // a higher tools version than the main Package.swift file.
         let contents: [String]
         do { contents = try fileSystem.getDirectoryContents(packagePath) } catch {
             throw ToolsVersionParser.Error.inaccessiblePackage(path: packagePath, reason: String(describing: error))
@@ -607,39 +605,47 @@ extension ManifestLoader {
 
         let regularManifest = packagePath.appending(component: Manifest.filename)
 
-        // Find the newest version-specific manifest that is compatible with the the current tools version.
-        if let versionSpecificCandidate = versionSpecificManifests.keys.sorted(by: >).first(where: { $0 <= currentToolsVersion }) {
+        // Try to get the tools version of the regular manifest.  As the comment marker is missing, we default to
+        // tools version 3.1.0 (as documented).
+        let regularManifestToolsVersion: ToolsVersion
+        do {
+            regularManifestToolsVersion = try ToolsVersionParser.parse(manifestPath: regularManifest, fileSystem: fileSystem)
+        }
+        catch let error as UnsupportedToolsVersion where error.packageToolsVersion == .v3 {
+          regularManifestToolsVersion = .v3
+        }
 
-            let versionSpecificManifest = packagePath.appending(component: versionSpecificManifests[versionSpecificCandidate]!)
+        // Find the newest version-specific manifest that is compatible with the current tools version.
+        guard let versionSpecificCandidate = versionSpecificManifests.keys.sorted(by: >).first(where: { $0 <= currentToolsVersion }) else {
+            // Otherwise, return the regular manifest.
+            return regularManifest
+        }
 
-            // SwiftPM 4 introduced tools-version designations; earlier packages default to tools version 3.1.0.
-            // See https://swift.org/blog/swift-package-manager-manifest-api-redesign.
-            let versionSpecificManifestToolsVersion: ToolsVersion
-            if versionSpecificCandidate < .v4 {
-                versionSpecificManifestToolsVersion = .v3
-            }
-            else {
-                versionSpecificManifestToolsVersion = try ToolsVersionParser.parse(manifestPath: versionSpecificManifest, fileSystem: fileSystem)
-            }
+        let versionSpecificManifest = packagePath.appending(component: versionSpecificManifests[versionSpecificCandidate]!)
 
-            // Try to get the tools version of the regular manifest.  At the comment marker is missing, we default to
-            // tools version 3.1.0 (as documented).
-            let regularManifestToolsVersion: ToolsVersion
-            do {
-                regularManifestToolsVersion = try ToolsVersionParser.parse(manifestPath: regularManifest, fileSystem: fileSystem)
-            }
-            catch let error as UnsupportedToolsVersion where error.packageToolsVersion == .v3 {
-              regularManifestToolsVersion = .v3
-            }
+        // SwiftPM 4 introduced tools-version designations; earlier packages default to tools version 3.1.0.
+        // See https://swift.org/blog/swift-package-manager-manifest-api-redesign.
+        let versionSpecificManifestToolsVersion: ToolsVersion
+        if versionSpecificCandidate < .v4 {
+            versionSpecificManifestToolsVersion = .v3
+        }
+        else {
+            versionSpecificManifestToolsVersion = try ToolsVersionParser.parse(manifestPath: versionSpecificManifest, fileSystem: fileSystem)
+        }
 
-            // Compare the tools version of this manifest with the regular
-            // manifest and use the version-specific manifest if it has
-            // a greater tools version.
-            if versionSpecificManifestToolsVersion > regularManifestToolsVersion {
+        // Compare the tools version of this manifest with the regular
+        // manifest and use the version-specific manifest if it has
+        // a greater tools version.
+        if versionSpecificManifestToolsVersion > regularManifestToolsVersion {
+            return versionSpecificManifest
+        } else {
+            // If there's no primary candidate, validate the regular manifest.
+            if regularManifestToolsVersion.validateToolsVersion(currentToolsVersion) {
+                return regularManifest
+            } else {
+                // If that's incompatible, use the closest version-specific manifest we got.
                 return versionSpecificManifest
             }
         }
-
-        return regularManifest
     }
 }

Sources/PackageModel/ToolsVersion.swift

@@ -105,30 +105,53 @@ public struct ToolsVersion: Equatable, Hashable, Codable, Sendable {
         _version = version
     }
 
+    private enum ValidationResult {
+        case valid
+        case unsupportedToolsVersion
+        case requireNewerTools
+    }
+
+    private func _validateToolsVersion(_ currentToolsVersion: ToolsVersion) -> ValidationResult {
+        // We don't want to throw any error when using the special vNext version.
+        if SwiftVersion.current.isDevelopment && self == .vNext {
+            return .valid
+        }
+
+        // Make sure the package has the right minimum tools version.
+        guard self >= .minimumRequired else {
+            return .unsupportedToolsVersion
+        }
+
+        // Make sure the package isn't newer than the current tools version.
+        guard currentToolsVersion >= self else {
+            return .requireNewerTools
+        }
+
+        return .valid
+    }
+
     /// Returns true if the tools version is valid and can be used by this
     /// version of the package manager.
+    public func validateToolsVersion(_ currentToolsVersion: ToolsVersion) -> Bool {
+        return self._validateToolsVersion(currentToolsVersion) == .valid
+    }
+
     public func validateToolsVersion(
         _ currentToolsVersion: ToolsVersion,
         packageIdentity: PackageIdentity,
         packageVersion: String? = .none
     ) throws {
-        // We don't want to throw any error when using the special vNext version.
-        if SwiftVersion.current.isDevelopment && self == .vNext {
-            return
-        }
-
-        // Make sure the package has the right minimum tools version.
-        guard self >= .minimumRequired else {
+        switch self._validateToolsVersion(currentToolsVersion) {
+        case .valid:
+            break
+        case .unsupportedToolsVersion:
             throw UnsupportedToolsVersion(
                 packageIdentity: packageIdentity,
                 packageVersion: packageVersion,
                 currentToolsVersion: currentToolsVersion,
                 packageToolsVersion: self
             )
-        }
-
-        // Make sure the package isn't newer than the current tools version.
-        guard currentToolsVersion >= self else {
+        case .requireNewerTools:
             throw RequireNewerTools(
                 packageIdentity: packageIdentity,
                 packageVersion: packageVersion,

Tests/PackageLoadingTests/ToolsVersionParserTests.swift

@@ -764,4 +764,19 @@ class ToolsVersionParserTests: XCTestCase {
         }
     }
 
+    func testVersionSpecificManifestMostCompatibleIfLower() throws {
+        let fs = InMemoryFileSystem(emptyFiles:
+            "/pkg/foo"
+        )
+        let root = AbsolutePath("/pkg")
+
+        try fs.writeFileContents(root.appending("Package.swift"), string: "// swift-tools-version:6.0.0\n")
+        try fs.writeFileContents(root.appending("[email protected]"), string: "// swift-tools-version:5.0.0\n")
+
+        let currentToolsVersion = ToolsVersion(version: "5.5.0")
+        let manifestPath = try ManifestLoader.findManifest(packagePath: root, fileSystem: fs, currentToolsVersion: currentToolsVersion)
+        let version = try ToolsVersionParser.parse(manifestPath: manifestPath, fileSystem: fs)
+        try version.validateToolsVersion(currentToolsVersion, packageIdentity: .plain("lunch"))
+        XCTAssertEqual(version.description, "5.0.0")
+    }
 }