Skip to content

Commit b983d8d

Browse files
AnthonyLatsisAnthonyLatsis
committed
swift-package-migrate: Add more clarity to manifest update error message
(cherry picked from commit e1deb23)
1 parent b571535 commit b983d8d

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
@@ -155,9 +156,12 @@ extension SwiftPackageCommand {
155156

156157
// Once the fix-its were applied, it's time to update the
157158
// manifest with newly adopted feature settings.
159+
//
160+
// Loop over a sorted array to produce deterministic results and
161+
// order of diagnostics.
158162

159163
print("> Updating manifest")
160-
for (name, _) in modules {
164+
for name in modules.keys.sorted() {
161165
swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(name)'")
162166
try self.updateManifest(
163167
for: name,
@@ -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
@@ -2248,6 +2248,100 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
22482248
}
22492249
}
22502250

2251+
func testMigrateCommandUpdateManifestSingleTarget() async throws {
2252+
try XCTSkipIf(
2253+
!UserToolchain.default.supportesSupportedFeatures,
2254+
"skipping because test environment compiler doesn't support `-print-supported-features`"
2255+
)
2256+
2257+
try await fixture(name: "SwiftMigrate/UpdateManifest") { fixturePath in
2258+
_ = try await self.execute(
2259+
[
2260+
"migrate",
2261+
"--to-feature",
2262+
"ExistentialAny,InferIsolatedConformances",
2263+
"--target",
2264+
"A",
2265+
],
2266+
packagePath: fixturePath
2267+
)
2268+
2269+
let updatedManifest = try localFileSystem.readFileContents(
2270+
fixturePath.appending(components: "Package.swift")
2271+
)
2272+
let expectedManifest = try localFileSystem.readFileContents(
2273+
fixturePath.appending(components: "Package.updated.targets-A.swift")
2274+
)
2275+
XCTAssertEqual(updatedManifest, expectedManifest)
2276+
}
2277+
2278+
}
2279+
2280+
func testMigrateCommandUpdateManifest2Targets() async throws {
2281+
try XCTSkipIf(
2282+
!UserToolchain.default.supportesSupportedFeatures,
2283+
"skipping because test environment compiler doesn't support `-print-supported-features`"
2284+
)
2285+
2286+
try await fixture(name: "SwiftMigrate/UpdateManifest") { fixturePath in
2287+
_ = try await self.execute(
2288+
[
2289+
"migrate",
2290+
"--to-feature",
2291+
"ExistentialAny,InferIsolatedConformances",
2292+
"--target",
2293+
"A,B",
2294+
],
2295+
packagePath: fixturePath
2296+
)
2297+
2298+
let updatedManifest = try localFileSystem.readFileContents(
2299+
fixturePath.appending(components: "Package.swift")
2300+
)
2301+
let expectedManifest = try localFileSystem.readFileContents(
2302+
fixturePath.appending(components: "Package.updated.targets-A-B.swift")
2303+
)
2304+
XCTAssertEqual(updatedManifest, expectedManifest)
2305+
}
2306+
}
2307+
2308+
func testMigrateCommandUpdateManifestWithErrors() async throws {
2309+
try XCTSkipIf(
2310+
!UserToolchain.default.supportesSupportedFeatures,
2311+
"skipping because test environment compiler doesn't support `-print-supported-features`"
2312+
)
2313+
2314+
try await fixture(name: "SwiftMigrate/UpdateManifest") { fixturePath in
2315+
try await XCTAssertThrowsCommandExecutionError(
2316+
await self.execute(
2317+
["migrate", "--to-feature", "ExistentialAny,InferIsolatedConformances,StrictMemorySafety"],
2318+
packagePath: fixturePath
2319+
)
2320+
) { error in
2321+
// 'SwiftMemorySafety.strictMemorySafety' was introduced in 6.2.
2322+
XCTAssertMatch(
2323+
error.stderr,
2324+
.contains(
2325+
"""
2326+
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()'
2327+
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()'
2328+
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()'
2329+
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()'
2330+
"""
2331+
)
2332+
)
2333+
}
2334+
2335+
let updatedManifest = try localFileSystem.readFileContents(
2336+
fixturePath.appending(components: "Package.swift")
2337+
)
2338+
let expectedManifest = try localFileSystem.readFileContents(
2339+
fixturePath.appending(components: "Package.updated.targets-all.swift")
2340+
)
2341+
XCTAssertEqual(updatedManifest, expectedManifest)
2342+
}
2343+
}
2344+
22512345
func testBuildToolPlugin() async throws {
22522346
try await testBuildToolPlugin(staticStdlib: false)
22532347
}

0 commit comments

Comments
 (0)