Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// swift-tools-version:5.8

import PackageDescription

let package = Package(
name: "ExistentialAnyMigration",
targets: [
.target(name: "Library", dependencies: ["CommonLibrary"], plugins: [.plugin(name: "Plugin")]),
.plugin(name: "Plugin", capability: .buildTool, dependencies: ["Tool"]),
.executableTarget(name: "Tool", dependencies: ["CommonLibrary"]),
.target(name: "CommonLibrary"),
]
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import PackagePlugin
import Foundation

@main struct Plugin: BuildToolPlugin {
func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] {
let tool = try context.tool(named: "Tool")
let output = context.pluginWorkDirectory.appending(["generated.swift"])
return [
.buildCommand(
displayName: "Plugin",
executable: tool.path,
arguments: [output],
inputFiles: [],
outputFiles: [output])
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
public func common() {}


protocol P {}

func needsMigration(_ p: P) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import CommonLibrary

func bar() {
generatedFunction()
common()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import Foundation
import CommonLibrary

@main struct Entry {
public static func main() async throws {
common()
let outputPath = CommandLine.arguments[1]
let contents = """
func generatedFunction() {}
"""
FileManager.default.createFile(atPath: outputPath, contents: contents.data(using: .utf8))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// swift-tools-version:5.8

import PackageDescription

let package = Package(
name: "ExistentialAnyMigration",
targets: [
.target(name: "Library", plugins: [.plugin(name: "Plugin")]),
.plugin(name: "Plugin", capability: .buildTool, dependencies: ["Tool"]),
.executableTarget(name: "Tool"),
]
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import PackagePlugin
import Foundation

@main struct Plugin: BuildToolPlugin {
func createBuildCommands(context: PluginContext, target: Target) throws -> [Command] {
let tool = try context.tool(named: "Tool")
let output = context.pluginWorkDirectory.appending(["generated.swift"])
return [
.buildCommand(
displayName: "Plugin",
executable: tool.path,
arguments: [output],
inputFiles: [],
outputFiles: [output])
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
protocol P {
}

func test1(_: P) {
}

func test2(_: P.Protocol) {
}

func test3() {
let _: [P?] = []
}

func test4() {
var x = 42
}

func bar() {
generatedFunction()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import Foundation

@main struct Entry {
public static func main() async throws {
let outputPath = CommandLine.arguments[1]
let contents = """
func generatedFunction() {}
func dontmodifyme(_: P) {}
"""
FileManager.default.createFile(atPath: outputPath, contents: contents.data(using: .utf8))
}
}
21 changes: 17 additions & 4 deletions Sources/Build/BuildOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
}

/// Perform a build using the given build description and subset.
public func build(subset: BuildSubset) async throws {
public func build(subset: BuildSubset) async throws -> BuildResult {
guard !self.config.shouldSkipBuilding(for: .target) else {
return
return BuildResult(serializedDiagnosticPathsByTargetName: .failure(StringError("Building was skipped")))
}

let buildStartTime = DispatchTime.now()
Expand All @@ -422,7 +422,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
// any errors up-front. Returns true if we should proceed with the build
// or false if not. It will already have thrown any appropriate error.
guard try await self.compilePlugins(in: subset) else {
return
return BuildResult(serializedDiagnosticPathsByTargetName: .failure(StringError("Plugin compilation failed")))
}

let configuration = self.config.configuration(for: .target)
Expand Down Expand Up @@ -452,6 +452,17 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
)
guard success else { throw Diagnostics.fatalError }

let serializedDiagnosticResult: Result<[String: [AbsolutePath]], Error>
var serializedDiagnosticPaths: [String: [AbsolutePath]] = [:]
do {
for module in try buildPlan.buildModules {
serializedDiagnosticPaths[module.module.name, default: []].append(contentsOf: module.diagnosticFiles)
}
serializedDiagnosticResult = .success(serializedDiagnosticPaths)
} catch {
serializedDiagnosticResult = .failure(error)
}

// Create backwards-compatibility symlink to old build path.
let oldBuildPath = self.config.dataPath(for: .target).parentDirectory.appending(
component: configuration.dirname
Expand All @@ -463,7 +474,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
warning: "unable to delete \(oldBuildPath), skip creating symbolic link",
underlyingError: error
)
return
return BuildResult(serializedDiagnosticPathsByTargetName: serializedDiagnosticResult)
}
}

Expand All @@ -479,6 +490,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
underlyingError: error
)
}

return BuildResult(serializedDiagnosticPathsByTargetName: serializedDiagnosticResult)
}

/// Compiles any plugins specified or implied by the build subset, returning
Expand Down
50 changes: 15 additions & 35 deletions Sources/Commands/PackageCommands/Migrate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ extension SwiftPackageCommand {
features.append(feature)
}

let targets = self.options.targets
var targets = self.options.targets

let buildSystem = try await createBuildSystem(
swiftCommandState,
Expand All @@ -100,49 +100,29 @@ extension SwiftPackageCommand {

// Next, let's build all of the individual targets or the
// whole project to get diagnostic files.

print("> Starting the build")
var diagnosticsPaths: [String: [AbsolutePath]] = [:]
if !targets.isEmpty {
for target in targets {
try await buildSystem.build(subset: .target(target))
let buildResult = try await buildSystem.build(subset: .target(target))
diagnosticsPaths.merge(try buildResult.serializedDiagnosticPathsByTargetName.get(), uniquingKeysWith: { $0 + $1 })
}
} else {
try await buildSystem.build(subset: .allIncludingTests)
diagnosticsPaths = try await buildSystem.build(subset: .allIncludingTests).serializedDiagnosticPathsByTargetName.get()
}

// Determine all of the targets we need up update.
let buildPlan = try buildSystem.buildPlan

var modules: [any ModuleBuildDescription] = []
if !targets.isEmpty {
for buildDescription in buildPlan.buildModules
where targets.contains(buildDescription.module.name) {
modules.append(buildDescription)
}
} else {
let graph = try await buildSystem.getPackageGraph()
for buildDescription in buildPlan.buildModules
where graph.isRootPackage(buildDescription.package)
{
let module = buildDescription.module
guard module.type != .plugin, !module.implicit else {
continue
}
modules.append(buildDescription)
}
var summary = SwiftFixIt.Summary(numberOfFixItsApplied: 0, numberOfFilesChanged: 0)
let graph = try await buildSystem.getPackageGraph()
if targets.isEmpty {
targets = OrderedSet(graph.rootPackages.flatMap { $0.manifest.targets.filter { $0.type != .plugin }.map(\.name) })
}

// If the build suceeded, let's extract all of the diagnostic
// files from build plan and feed them to the fix-it tool.

print("> Applying fix-its")

var summary = SwiftFixIt.Summary(numberOfFixItsApplied: 0, numberOfFilesChanged: 0)
let fixItDuration = try ContinuousClock().measure {
for module in modules {
for target in targets {
let fixit = try SwiftFixIt(
diagnosticFiles: module.diagnosticFiles,
diagnosticFiles: Array(diagnosticsPaths[target] ?? []),
categories: Set(features.flatMap(\.categories)),
excludedSourceDirectories: [swiftCommandState.scratchDirectory],
fileSystem: swiftCommandState.fileSystem
)
summary += try fixit.applyFixIts()
Expand Down Expand Up @@ -176,10 +156,10 @@ extension SwiftPackageCommand {
// manifest with newly adopted feature settings.

print("> Updating manifest")
for module in modules.map(\.module) {
swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(module.name)'")
for target in targets {
swiftCommandState.observabilityScope.emit(debug: "Adding feature(s) to '\(target)'")
try self.updateManifest(
for: module.name,
for: target,
add: features,
using: swiftCommandState
)
Expand Down
14 changes: 12 additions & 2 deletions Sources/SPMBuildCore/BuildSystem/BuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ public protocol BuildSystem: Cancellable {
/// Builds a subset of the package graph.
/// - Parameters:
/// - subset: The subset of the package graph to build.
func build(subset: BuildSubset) async throws
@discardableResult
func build(subset: BuildSubset) async throws -> BuildResult

var buildPlan: BuildPlan { get throws }

Expand All @@ -59,11 +60,20 @@ public protocol BuildSystem: Cancellable {

extension BuildSystem {
/// Builds the default subset: all targets excluding tests.
public func build() async throws {
@discardableResult
public func build() async throws -> BuildResult {
try await build(subset: .allExcludingTests)
}
}

public struct BuildResult {
package init(serializedDiagnosticPathsByTargetName: Result<[String: [AbsolutePath]], Error>) {
self.serializedDiagnosticPathsByTargetName = serializedDiagnosticPathsByTargetName
}

public var serializedDiagnosticPathsByTargetName: Result<[String: [AbsolutePath]], Error>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a problem with using a "name" here as a string unless we can 100% make sure that it would distinguish between host and target builds of the same module (i.e. in BuildOperation we use module.name which won't make a distinction)...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a test that uses a target as a plugin dependency to make sure that we don't override the diagnostics.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added another test here to ensure only one fix-it is applied even if the sources need to be built twice as a common dependency of host tools and target content. I think using the name here is in line with the fact that the list of targets may be user provided - names uniquely identify targets in the manifest model, just not once they've been lowered to targets building for a specific platform

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is that we do allow passing flags like -Xswiftc-host or whatever it's spelled so there could be a difference in fix-its produced when building for host and destination, if we use simply a name instead of an identifier that includes target information we'd just override diagnostic files in that map.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now they'd be concatenated instead of one overriding the other, so the entry corresponding to "Foo" includes the diagnostic paths for Foo when built for the host and those when built for the target, and we rely on SwiftFixIt to deduplicate them. I'm not sure we want to drop the host diagnostics entirely because that feels arbitrary imo.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, yeah, that sounds okay, SwiftFixIt should be able to handle that!

}

public protocol ProductBuildDescription {
/// The reference to the product.
var package: ResolvedPackage { get }
Expand Down
27 changes: 18 additions & 9 deletions Sources/SwiftBuildSupport/SwiftBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,22 @@ struct SessionFailedError: Error {
var diagnostics: [SwiftBuild.SwiftBuildMessage.DiagnosticInfo]
}

func withService(
func withService<T>(
connectionMode: SWBBuildServiceConnectionMode = .default,
variant: SWBBuildServiceVariant = .default,
serviceBundleURL: URL? = nil,
body: @escaping (_ service: SWBBuildService) async throws -> Void
) async throws {
body: @escaping (_ service: SWBBuildService) async throws -> T
) async throws -> T {
let service = try await SWBBuildService(connectionMode: connectionMode, variant: variant, serviceBundleURL: serviceBundleURL)
let result: T
do {
try await body(service)
result = try await body(service)
} catch {
await service.close()
throw error
}
await service.close()
return result
}

public func createSession(
Expand Down Expand Up @@ -279,20 +281,20 @@ public final class SwiftBuildSystem: SPMBuildCore.BuildSystem {
SwiftLanguageVersion.supportedSwiftLanguageVersions
}

public func build(subset: BuildSubset) async throws {
public func build(subset: BuildSubset) async throws -> BuildResult {
guard !buildParameters.shouldSkipBuilding else {
return
return BuildResult(serializedDiagnosticPathsByTargetName: .failure(StringError("Building was skipped")))
}

try await writePIF(buildParameters: buildParameters)

try await startSWBuildOperation(pifTargetName: subset.pifTargetName)
return try await startSWBuildOperation(pifTargetName: subset.pifTargetName)
}

private func startSWBuildOperation(pifTargetName: String) async throws {
private func startSWBuildOperation(pifTargetName: String) async throws -> BuildResult {
let buildStartTime = ContinuousClock.Instant.now

try await withService(connectionMode: .inProcessStatic(swiftbuildServiceEntryPoint)) { service in
return try await withService(connectionMode: .inProcessStatic(swiftbuildServiceEntryPoint)) { service in
let derivedDataPath = self.buildParameters.dataPath

let progressAnimation = ProgressAnimation.percent(
Expand All @@ -302,6 +304,7 @@ public final class SwiftBuildSystem: SPMBuildCore.BuildSystem {
isColorized: self.buildParameters.outputParameters.isColorized
)

var serializedDiagnosticPathsByTargetName: [String: [Basics.AbsolutePath]] = [:]
do {
try await withSession(service: service, name: self.buildParameters.pifManifest.pathString, toolchainPath: self.buildParameters.toolchain.toolchainDir, packageManagerResourcesDirectory: self.packageManagerResourcesDirectory) { session, _ in
self.outputStream.send("Building for \(self.buildParameters.configuration == .debug ? "debugging" : "production")...\n")
Expand Down Expand Up @@ -451,6 +454,11 @@ public final class SwiftBuildSystem: SPMBuildCore.BuildSystem {
}
let targetInfo = try buildState.target(for: startedInfo)
self.delegate?.buildSystem(self, didFinishCommand: BuildSystemCommand(startedInfo, targetInfo: targetInfo))
if let targetName = targetInfo?.targetName {
serializedDiagnosticPathsByTargetName[targetName, default: []].append(contentsOf: startedInfo.serializedDiagnosticsPaths.compactMap {
try? Basics.AbsolutePath(validating: $0.pathString)
})
}
case .targetStarted(let info):
try buildState.started(target: info)
case .planningOperationStarted, .planningOperationCompleted, .reportBuildDescription, .reportPathMap, .preparedForIndex, .backtraceFrame, .buildStarted, .preparationComplete, .targetUpToDate, .targetComplete, .taskUpToDate:
Expand Down Expand Up @@ -503,6 +511,7 @@ public final class SwiftBuildSystem: SPMBuildCore.BuildSystem {
} catch {
throw error
}
return BuildResult(serializedDiagnosticPathsByTargetName: .success(serializedDiagnosticPathsByTargetName))
}
}

Expand Down
Loading