Skip to content

Commit 8a4b37f

Browse files
authored
[6.2][Commands] Migrate: Avoid duplicate fix-its and manifest updates when… (#8929)
… building target for both host and destination - Explanation: This is a limited cherry-pick of #8888 For example if a target is a dependency of a plugin we build it for host and destination and produce the same fix-it twice and attempt to update manifest twice because uniquing is done on the ModuleBuildDescriptions. The changes do the following: - Coalesce all the diagnostic files for the same target regardless of host or destination build to make it possible for`SwiftFixIt` to filter duplicate fix-its; - Unique based on module identity to prevent duplicate setting insertion. - Resolves: rdar://155569285 - Main Branch PR: #8888 - Risk: Very Low. This is a narrow fix for `swift package migrate` command. - Reviewed By: Me and @AnthonyLatsis reviewed the work in the main PR. - Testing: Added new test-cases to the suite.
1 parent ec998ad commit 8a4b37f

File tree

13 files changed

+189
-10
lines changed

13 files changed

+189
-10
lines changed
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// swift-tools-version:5.8
2+
3+
import PackageDescription
4+
5+
let package = Package(
6+
name: "ExistentialAnyMigration",
7+
targets: [
8+
.target(name: "Library", dependencies: ["CommonLibrary"], plugins: [.plugin(name: "Plugin")]),
9+
.plugin(name: "Plugin", capability: .buildTool, dependencies: ["Tool"]),
10+
.executableTarget(name: "Tool", dependencies: ["CommonLibrary"]),
11+
.target(name: "CommonLibrary"),
12+
]
13+
)
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import PackagePlugin
2+
import Foundation
3+
4+
@main struct Plugin: BuildToolPlugin {
5+
func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] {
6+
let tool = try context.tool(named: "Tool")
7+
let output = context.pluginWorkDirectory.appending(["generated.swift"])
8+
return [
9+
.buildCommand(
10+
displayName: "Plugin",
11+
executable: tool.path,
12+
arguments: [output],
13+
inputFiles: [],
14+
outputFiles: [output])
15+
]
16+
}
17+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
public func common() {}
2+
3+
4+
protocol P {}
5+
6+
func needsMigration(_ p: P) {}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import CommonLibrary
2+
3+
func bar() {
4+
generatedFunction()
5+
common()
6+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import Foundation
2+
import CommonLibrary
3+
4+
@main struct Entry {
5+
public static func main() async throws {
6+
common()
7+
let outputPath = CommandLine.arguments[1]
8+
let contents = """
9+
func generatedFunction() {}
10+
"""
11+
FileManager.default.createFile(atPath: outputPath, contents: contents.data(using: .utf8))
12+
}
13+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// swift-tools-version:5.8
2+
3+
import PackageDescription
4+
5+
let package = Package(
6+
name: "ExistentialAnyMigration",
7+
targets: [
8+
.target(name: "Library", plugins: [.plugin(name: "Plugin")]),
9+
.plugin(name: "Plugin", capability: .buildTool, dependencies: ["Tool"]),
10+
.executableTarget(name: "Tool"),
11+
]
12+
)
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import PackagePlugin
2+
import Foundation
3+
4+
@main struct Plugin: BuildToolPlugin {
5+
func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] {
6+
let tool = try context.tool(named: "Tool")
7+
let output = context.pluginWorkDirectory.appending(["generated.swift"])
8+
return [
9+
.buildCommand(
10+
displayName: "Plugin",
11+
executable: tool.path,
12+
arguments: [output],
13+
inputFiles: [],
14+
outputFiles: [output])
15+
]
16+
}
17+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
protocol P {
2+
}
3+
4+
func test1(_: P) {
5+
}
6+
7+
func test2(_: P.Protocol) {
8+
}
9+
10+
func test3() {
11+
let _: [P?] = []
12+
}
13+
14+
func test4() {
15+
var x = 42
16+
}
17+
18+
func bar() {
19+
generatedFunction()
20+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import Foundation
2+
3+
@main struct Entry {
4+
public static func main() async throws {
5+
let outputPath = CommandLine.arguments[1]
6+
let contents = """
7+
func generatedFunction() {}
8+
func dontmodifyme(_: P) {}
9+
"""
10+
FileManager.default.createFile(atPath: outputPath, contents: contents.data(using: .utf8))
11+
}
12+
}

Sources/Commands/PackageCommands/Migrate.swift

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,11 @@ extension SwiftPackageCommand {
113113
// Determine all of the targets we need up update.
114114
let buildPlan = try buildSystem.buildPlan
115115

116-
var modules: [any ModuleBuildDescription] = []
116+
var modules = [String: [AbsolutePath]]()
117117
if !targets.isEmpty {
118118
for buildDescription in buildPlan.buildModules
119119
where targets.contains(buildDescription.module.name) {
120-
modules.append(buildDescription)
120+
modules[buildDescription.module.name, default: []].append(contentsOf: buildDescription.diagnosticFiles)
121121
}
122122
} else {
123123
let graph = try await buildSystem.getPackageGraph()
@@ -128,7 +128,7 @@ extension SwiftPackageCommand {
128128
guard module.type != .plugin, !module.implicit else {
129129
continue
130130
}
131-
modules.append(buildDescription)
131+
modules[buildDescription.module.name, default: []].append(contentsOf: buildDescription.diagnosticFiles)
132132
}
133133
}
134134

@@ -139,10 +139,11 @@ extension SwiftPackageCommand {
139139

140140
var summary = SwiftFixIt.Summary(numberOfFixItsApplied: 0, numberOfFilesChanged: 0)
141141
let fixItDuration = try ContinuousClock().measure {
142-
for module in modules {
142+
for (_, diagnosticFiles) in modules {
143143
let fixit = try SwiftFixIt(
144-
diagnosticFiles: module.diagnosticFiles,
144+
diagnosticFiles: diagnosticFiles,
145145
categories: Set(features.flatMap(\.categories)),
146+
excludedSourceDirectories: [swiftCommandState.scratchDirectory],
146147
fileSystem: swiftCommandState.fileSystem
147148
)
148149
summary += try fixit.applyFixIts()
@@ -176,10 +177,10 @@ extension SwiftPackageCommand {
176177
// manifest with newly adopted feature settings.
177178

178179
print("> Updating manifest")
179-
for module in modules.map(\.module) {
180-
swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(module.name)'")
180+
for (name, _) in modules {
181+
swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(name)'")
181182
try self.updateManifest(
182-
for: module.name,
183+
for: name,
183184
add: features,
184185
using: swiftCommandState
185186
)

0 commit comments

Comments
 (0)