Skip to content

Commit a27bbb5

Browse files
authored
Fix a tools version loading failure that occurred when the regular manifest doesn't contain a tools-version. (#3208)
Usually the main manifest is the latest, but there are examples of packages where it's the opposite: the main version is the old one, and newer ones have version suffixes. In rare cases the main manifest is so old that it doesn't have a tools version, and the new logic for tools version loading incorrectly reported an error in those cases. rdar://73267827
1 parent ee7b68d commit a27bbb5

File tree

2 files changed

+27
-1
lines changed

2 files changed

+27
-1
lines changed

Sources/PackageLoading/ToolsVersionLoader.swift

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,21 @@ extension Manifest {
9999
else {
100100
versionSpecificManifestToolsVersion = try toolsVersionLoader.load(file: versionSpecificManifest, fileSystem: fileSystem)
101101
}
102-
let regularManifestToolsVersion = try toolsVersionLoader.load(file: regularManifest, fileSystem: fileSystem)
102+
103+
// Try to get the tools version of the regular manifest. At the comment marker is missing, we default to
104+
// tools version 3.1.0 (as documented).
105+
let regularManifestToolsVersion: ToolsVersion
106+
do {
107+
regularManifestToolsVersion = try toolsVersionLoader.load(file: regularManifest, fileSystem: fileSystem)
108+
}
109+
catch {
110+
if case ToolsVersionLoader.Error.malformedToolsVersionSpecification(.commentMarker(.isMissing)) = error {
111+
regularManifestToolsVersion = .v3
112+
}
113+
else {
114+
throw error
115+
}
116+
}
103117

104118
// Compare the tools version of this manifest with the regular
105119
// manifest and use the version-specific manifest if it has

Tests/PackageLoadingTests/PD4_2LoadingTests.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
360360
func testVersionSpecificLoadingOfVersion3Manifest() throws {
361361
// Create a temporary FS to hold the package manifests.
362362
let fs = InMemoryFileSystem()
363+
363364
// Write a regular manifest with a tools version comment, and a `[email protected]` manifest without one.
364365
let packageDir = AbsolutePath.root
365366
let manifestContents = "import PackageDescription\nlet package = Package(name: \"Trivial\")"
@@ -372,6 +373,17 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
372373
// Check we can load the manifest.
373374
let manifest = try manifestLoader.load(package: packageDir, baseURL: "/foo", toolsVersion: .v4_2, packageKind: .root, fileSystem: fs)
374375
XCTAssertEqual(manifest.name, "Trivial")
376+
377+
// Switch it around so that the main manifest is now the one that doesn't have a comment.
378+
try fs.writeFileContents(
379+
packageDir.appending(component: Manifest.basename + ".swift"),
380+
bytes: ByteString(encodingAsUTF8: manifestContents))
381+
try fs.writeFileContents(
382+
packageDir.appending(component: Manifest.basename + "@swift-4.swift"),
383+
bytes: ByteString(encodingAsUTF8: "// swift-tools-version:4.0\n" + manifestContents))
384+
// Check we can load the manifest.
385+
let manifest2 = try manifestLoader.load(package: packageDir, baseURL: "/foo", toolsVersion: .v4_2, packageKind: .root, fileSystem: fs)
386+
XCTAssertEqual(manifest2.name, "Trivial")
375387
}
376388

377389
func testRuntimeManifestErrors() throws {

0 commit comments

Comments
 (0)