Skip to content

Commit e1deb23

Browse files
AnthonyLatsisAnthonyLatsis
committed
swift-package-migrate: Add more clarity to manifest update error message
1 parent a286293 commit e1deb23

File tree

10 files changed

+245
-13
lines changed

10 files changed

+245
-13
lines changed

‎Fixtures/SwiftMigrate/UpdateManifest/Package.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// swift-tools-version:5.8
2+
3+
import PackageDescription
4+
5+
var swiftSettings: [SwiftSetting] = []
6+
7+
let package = Package(
8+
name: "WithErrors",
9+
targets: [
10+
.target(
11+
name: "CannotFindSettings",
12+
swiftSettings: swiftSettings
13+
),
14+
.target(name: "A"),
15+
.target(name: "B"),
16+
]
17+
)
18+
19+
package.targets.append(
20+
.target(
21+
name: "CannotFindTarget",
22+
swiftSettings: swiftSettings
23+
),
24+
)

‎Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-A-B.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// swift-tools-version:5.8
2+
3+
import PackageDescription
4+
5+
var swiftSettings: [SwiftSetting] = []
6+
7+
let package = Package(
8+
name: "WithErrors",
9+
targets: [
10+
.target(
11+
name: "CannotFindSettings",
12+
swiftSettings: swiftSettings
13+
),
14+
.target(name: "A",swiftSettings: [
15+
.enableUpcomingFeature("ExistentialAny"),
16+
.enableUpcomingFeature("InferIsolatedConformances"),]),
17+
.target(name: "B",swiftSettings: [
18+
.enableUpcomingFeature("ExistentialAny"),
19+
.enableUpcomingFeature("InferIsolatedConformances"),]),
20+
]
21+
)
22+
23+
package.targets.append(
24+
.target(
25+
name: "CannotFindTarget",
26+
swiftSettings: swiftSettings
27+
),
28+
)

‎Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-A.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// swift-tools-version:5.8
2+
3+
import PackageDescription
4+
5+
var swiftSettings: [SwiftSetting] = []
6+
7+
let package = Package(
8+
name: "WithErrors",
9+
targets: [
10+
.target(
11+
name: "CannotFindSettings",
12+
swiftSettings: swiftSettings
13+
),
14+
.target(name: "A",swiftSettings: [
15+
.enableUpcomingFeature("ExistentialAny"),
16+
.enableUpcomingFeature("InferIsolatedConformances"),]),
17+
.target(name: "B"),
18+
]
19+
)
20+
21+
package.targets.append(
22+
.target(
23+
name: "CannotFindTarget",
24+
swiftSettings: swiftSettings
25+
),
26+
)

‎Fixtures/SwiftMigrate/UpdateManifest/Package.updated.targets-all.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// swift-tools-version:5.8
2+
3+
import PackageDescription
4+
5+
var swiftSettings: [SwiftSetting] = []
6+
7+
let package = Package(
8+
name: "WithErrors",
9+
targets: [
10+
.target(
11+
name: "CannotFindSettings",
12+
swiftSettings: swiftSettings
13+
),
14+
.target(name: "A",swiftSettings: [
15+
.enableUpcomingFeature("ExistentialAny"),
16+
.enableUpcomingFeature("InferIsolatedConformances"),]),
17+
.target(name: "B",swiftSettings: [
18+
.enableUpcomingFeature("ExistentialAny"),
19+
.enableUpcomingFeature("InferIsolatedConformances"),]),
20+
]
21+
)
22+
23+
package.targets.append(
24+
.target(
25+
name: "CannotFindTarget",
26+
swiftSettings: swiftSettings
27+
),
28+
)

‎Fixtures/SwiftMigrate/UpdateManifest/Sources/A/File.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
public func foo() {}

‎Fixtures/SwiftMigrate/UpdateManifest/Sources/B/File.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
public func foo() {}

‎Fixtures/SwiftMigrate/UpdateManifest/Sources/CannotFindSettings/File.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
public func foo() {}

‎Fixtures/SwiftMigrate/UpdateManifest/Sources/CannotFindTarget/File.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
public func foo() {}

‎Sources/Commands/PackageCommands/Migrate.swift

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import OrderedCollections
2323

2424
import PackageGraph
2525
import PackageModel
26+
import enum PackageModelSyntax.ManifestEditError
2627

2728
import SPMBuildCore
2829
import SwiftFixIt
@@ -156,8 +157,11 @@ extension SwiftPackageCommand {
156157

157158
// Once the fix-its were applied, it's time to update the
158159
// manifest with newly adopted feature settings.
160+
//
161+
// Loop over a sorted array to produce deterministic results and
162+
// order of diagnostics.
159163
print("> Updating manifest")
160-
for target in targetsToMigrate {
164+
for target in targetsToMigrate.sorted() {
161165
swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(target)'")
162166
try self.updateManifest(
163167
for: target,
@@ -169,8 +173,8 @@ extension SwiftPackageCommand {
169173

170174
/// Resolves the requested feature names.
171175
///
172-
/// - Returns: An array of resolved features, or `nil` if an error was
173-
/// emitted.
176+
/// - Returns: An array of resolved features sorted by name, or `nil`
177+
/// if an error was emitted.
174178
private func resolveRequestedFeatures(
175179
_ swiftCommandState: SwiftCommandState
176180
) throws -> [SwiftCompilerFeature]? {
@@ -207,7 +211,9 @@ extension SwiftPackageCommand {
207211
resolvedFeatures.append(feature)
208212
}
209213

210-
return resolvedFeatures
214+
return resolvedFeatures.sorted { lhs, rhs in
215+
lhs.name < rhs.name
216+
}
211217
}
212218

213219
private func createBuildSystem(
@@ -273,15 +279,37 @@ extension SwiftPackageCommand {
273279
verbose: !self.globalOptions.logging.quiet
274280
)
275281
} catch {
276-
try swiftCommandState.observabilityScope.emit(
277-
error: """
278-
Could not update manifest for '\(target)' (\(error)). \
279-
Please enable '\(
280-
features.map { try $0.swiftSettingDescription }
281-
.joined(separator: ", ")
282-
)' features manually
283-
"""
284-
)
282+
var message =
283+
"Could not update manifest to enable requested features for target '\(target)' (\(error))"
284+
285+
// Do not suggest manual addition if something else is wrong or
286+
// if the error implies that it cannot be done.
287+
if let error = error as? ManifestEditError {
288+
switch error {
289+
case .cannotFindPackage,
290+
.cannotAddSettingsToPluginTarget,
291+
.existingDependency:
292+
break
293+
case .cannotFindArrayLiteralArgument,
294+
// This means the target could not be found
295+
// syntactically, not that it does not exist.
296+
.cannotFindTargets,
297+
.cannotFindTarget,
298+
// This means the swift-tools-version is lower than
299+
// the version where one of the setting was introduced.
300+
.oldManifest:
301+
let settings = try features.map {
302+
try $0.swiftSettingDescription
303+
}.joined(separator: ", ")
304+
305+
message += """
306+
. Please enable them manually by adding the following Swift settings to the target: \
307+
'\(settings)'
308+
"""
309+
}
310+
}
311+
312+
swiftCommandState.observabilityScope.emit(error: message)
285313
}
286314
}
287315

‎Tests/CommandsTests/PackageCommandTests.swift

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2250,6 +2250,100 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
22502250
}
22512251
}
22522252

2253+
func testMigrateCommandUpdateManifestSingleTarget() async throws {
2254+
try XCTSkipIf(
2255+
!UserToolchain.default.supportesSupportedFeatures,
2256+
"skipping because test environment compiler doesn't support `-print-supported-features`"
2257+
)
2258+
2259+
try await fixture(name: "SwiftMigrate/UpdateManifest") { fixturePath in
2260+
_ = try await self.execute(
2261+
[
2262+
"migrate",
2263+
"--to-feature",
2264+
"ExistentialAny,InferIsolatedConformances",
2265+
"--target",
2266+
"A",
2267+
],
2268+
packagePath: fixturePath
2269+
)
2270+
2271+
let updatedManifest = try localFileSystem.readFileContents(
2272+
fixturePath.appending(components: "Package.swift")
2273+
)
2274+
let expectedManifest = try localFileSystem.readFileContents(
2275+
fixturePath.appending(components: "Package.updated.targets-A.swift")
2276+
)
2277+
XCTAssertEqual(updatedManifest, expectedManifest)
2278+
}
2279+
2280+
}
2281+
2282+
func testMigrateCommandUpdateManifest2Targets() async throws {
2283+
try XCTSkipIf(
2284+
!UserToolchain.default.supportesSupportedFeatures,
2285+
"skipping because test environment compiler doesn't support `-print-supported-features`"
2286+
)
2287+
2288+
try await fixture(name: "SwiftMigrate/UpdateManifest") { fixturePath in
2289+
_ = try await self.execute(
2290+
[
2291+
"migrate",
2292+
"--to-feature",
2293+
"ExistentialAny,InferIsolatedConformances",
2294+
"--target",
2295+
"A,B",
2296+
],
2297+
packagePath: fixturePath
2298+
)
2299+
2300+
let updatedManifest = try localFileSystem.readFileContents(
2301+
fixturePath.appending(components: "Package.swift")
2302+
)
2303+
let expectedManifest = try localFileSystem.readFileContents(
2304+
fixturePath.appending(components: "Package.updated.targets-A-B.swift")
2305+
)
2306+
XCTAssertEqual(updatedManifest, expectedManifest)
2307+
}
2308+
}
2309+
2310+
func testMigrateCommandUpdateManifestWithErrors() async throws {
2311+
try XCTSkipIf(
2312+
!UserToolchain.default.supportesSupportedFeatures,
2313+
"skipping because test environment compiler doesn't support `-print-supported-features`"
2314+
)
2315+
2316+
try await fixture(name: "SwiftMigrate/UpdateManifest") { fixturePath in
2317+
try await XCTAssertThrowsCommandExecutionError(
2318+
await self.execute(
2319+
["migrate", "--to-feature", "ExistentialAny,InferIsolatedConformances,StrictMemorySafety"],
2320+
packagePath: fixturePath
2321+
)
2322+
) { error in
2323+
// 'SwiftMemorySafety.strictMemorySafety' was introduced in 6.2.
2324+
XCTAssertMatch(
2325+
error.stderr,
2326+
.contains(
2327+
"""
2328+
error: Could not update manifest to enable requested features for target 'A' (package manifest version 5.8.0 is too old: please update to manifest version 6.2.0 or newer). Please enable them manually by adding the following Swift settings to the target: '.enableUpcomingFeature("ExistentialAny"), .enableUpcomingFeature("InferIsolatedConformances"), .strictMemorySafety()'
2329+
error: Could not update manifest to enable requested features for target 'B' (package manifest version 5.8.0 is too old: please update to manifest version 6.2.0 or newer). Please enable them manually by adding the following Swift settings to the target: '.enableUpcomingFeature("ExistentialAny"), .enableUpcomingFeature("InferIsolatedConformances"), .strictMemorySafety()'
2330+
error: Could not update manifest to enable requested features for target 'CannotFindSettings' (unable to find array literal for 'swiftSettings' argument). Please enable them manually by adding the following Swift settings to the target: '.enableUpcomingFeature("ExistentialAny"), .enableUpcomingFeature("InferIsolatedConformances"), .strictMemorySafety()'
2331+
error: Could not update manifest to enable requested features for target 'CannotFindTarget' (unable to find target named 'CannotFindTarget' in package). Please enable them manually by adding the following Swift settings to the target: '.enableUpcomingFeature("ExistentialAny"), .enableUpcomingFeature("InferIsolatedConformances"), .strictMemorySafety()'
2332+
"""
2333+
)
2334+
)
2335+
}
2336+
2337+
let updatedManifest = try localFileSystem.readFileContents(
2338+
fixturePath.appending(components: "Package.swift")
2339+
)
2340+
let expectedManifest = try localFileSystem.readFileContents(
2341+
fixturePath.appending(components: "Package.updated.targets-all.swift")
2342+
)
2343+
XCTAssertEqual(updatedManifest, expectedManifest)
2344+
}
2345+
}
2346+
22532347
func testBuildToolPlugin() async throws {
22542348
try await testBuildToolPlugin(staticStdlib: false)
22552349
}

0 commit comments

Comments
 (0)