Skip to content

Implement diagnose-api-breaking-changes support with --build-system swiftbuild #8857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 2, 2025
Merged

Implement diagnose-api-breaking-changes support with --build-system swiftbuild #8857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 7 additions & 2 deletions Sources/Build/BuildOperation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
/// Alternative path to search for pkg-config `.pc` files.
private let pkgConfigDirectories: [AbsolutePath]

public var hasIntegratedAPIDigesterSupport: Bool { false }

public convenience init(
productsBuildParameters: BuildParameters,
toolsBuildParameters: BuildParameters,
Expand Down Expand Up @@ -225,7 +227,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
outputStream: outputStream,
logLevel: logLevel,
fileSystem: fileSystem,
observabilityScope: observabilityScope
observabilityScope: observabilityScope,
delegate: nil
)
}

Expand All @@ -242,7 +245,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
outputStream: OutputByteStream,
logLevel: Basics.Diagnostic.Severity,
fileSystem: Basics.FileSystem,
observabilityScope: ObservabilityScope
observabilityScope: ObservabilityScope,
delegate: SPMBuildCore.BuildSystemDelegate?
) {
/// Checks if stdout stream is tty.
var productsBuildParameters = productsBuildParameters
Expand All @@ -269,6 +273,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
self.additionalFileRules = additionalFileRules
self.pluginConfiguration = pluginConfiguration
self.pkgConfigDirectories = pkgConfigDirectories
self.delegate = delegate
}

public func getPackageGraph() async throws -> ModulesGraph {
Expand Down
210 changes: 186 additions & 24 deletions Sources/Commands/PackageCommands/APIDiff.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ import PackageGraph
import PackageModel
import SourceControl
import SPMBuildCore
import TSCBasic
Copy link
Contributor

Choose a reason for hiding this comment

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

question: What are we using in TSCBasic here? Can we replace it with a symbol that is not in Swift Tool Support Core?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ByteString type is defined there, so this is just formalizing a dependency that already existed now that a couple extension methods trigger MemberImportVisibility requirements

import TSCUtility
import _Concurrency
import Workspace

struct DeprecatedAPIDiff: ParsableCommand {
static let configuration = CommandConfiguration(commandName: "experimental-api-diff",
Expand Down Expand Up @@ -57,7 +59,7 @@ struct APIDiff: AsyncSwiftCommand {
Each ignored breaking change in the file should appear on its own line and contain the exact message \
to be ignored (e.g. 'API breakage: func foo() has been removed').
""")
var breakageAllowlistPath: AbsolutePath?
var breakageAllowlistPath: Basics.AbsolutePath?

@Argument(help: "The baseline treeish to compare to (for example, a commit hash, branch name, tag, and so on).")
var treeish: String
Expand All @@ -75,32 +77,51 @@ struct APIDiff: AsyncSwiftCommand {

@Option(name: .customLong("baseline-dir"),
help: "The path to a directory used to store API baseline files. If unspecified, a temporary directory will be used.")
var overrideBaselineDir: AbsolutePath?
var overrideBaselineDir: Basics.AbsolutePath?

@Flag(help: "Regenerate the API baseline, even if an existing one is available.")
var regenerateBaseline: Bool = false

func run(_ swiftCommandState: SwiftCommandState) async throws {
let apiDigesterPath = try swiftCommandState.getTargetToolchain().getSwiftAPIDigester()
let apiDigesterTool = SwiftAPIDigester(fileSystem: swiftCommandState.fileSystem, tool: apiDigesterPath)

let packageRoot = try globalOptions.locations.packageDirectory ?? swiftCommandState.getPackageRoot()
let repository = GitRepository(path: packageRoot)
let baselineRevision = try repository.resolveRevision(identifier: treeish)

// We turn build manifest caching off because we need the build plan.
let buildSystem = try await swiftCommandState.createBuildSystem(
explicitBuildSystem: .native,
traitConfiguration: .init(traitOptions: self.traits),
cacheBuildManifest: false
)

let packageGraph = try await buildSystem.getPackageGraph()
let modulesToDiff = try determineModulesToDiff(
let baselineDir = try overrideBaselineDir?.appending(component: baselineRevision.identifier) ?? swiftCommandState.productsBuildParameters.apiDiff.appending(component: "\(baselineRevision.identifier)-baselines")
let packageGraph = try await swiftCommandState.loadPackageGraph()
let modulesToDiff = try Self.determineModulesToDiff(
packageGraph: packageGraph,
observabilityScope: swiftCommandState.observabilityScope
productNames: products,
targetNames: targets,
observabilityScope: swiftCommandState.observabilityScope,
diagnoseMissingNames: true,
)

if swiftCommandState.options.build.buildSystem == .swiftbuild {
try await runWithIntegratedAPIDigesterSupport(
swiftCommandState,
baselineRevision: baselineRevision,
baselineDir: baselineDir,
modulesToDiff: modulesToDiff
)
} else {
let buildSystem = try await swiftCommandState.createBuildSystem(
traitConfiguration: .init(traitOptions: self.traits),
cacheBuildManifest: false,
)
try await runWithSwiftPMCoordinatedDiffing(
swiftCommandState,
buildSystem: buildSystem,
baselineRevision: baselineRevision,
modulesToDiff: modulesToDiff
)
}
}

private func runWithSwiftPMCoordinatedDiffing(_ swiftCommandState: SwiftCommandState, buildSystem: any BuildSystem, baselineRevision: Revision, modulesToDiff: Set<String>) async throws {
let apiDigesterPath = try swiftCommandState.getTargetToolchain().getSwiftAPIDigester()
let apiDigesterTool = SwiftAPIDigester(fileSystem: swiftCommandState.fileSystem, tool: apiDigesterPath)

// Build the current package.
try await buildSystem.build()

Expand Down Expand Up @@ -173,39 +194,180 @@ struct APIDiff: AsyncSwiftCommand {
}
}

private func determineModulesToDiff(packageGraph: ModulesGraph, observabilityScope: ObservabilityScope) throws -> Set<String> {
private func runWithIntegratedAPIDigesterSupport(_ swiftCommandState: SwiftCommandState, baselineRevision: Revision, baselineDir: Basics.AbsolutePath, modulesToDiff: Set<String>) async throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider created automated tests that validate this function in isolation with happy path, and also fault injection, and more.

Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: consider type aliasing, or creating more specific type, for the modulesToDiff argument to aid in code readability. For example, instead of String, create a type alias which represents what the string is. e.g.: typealias ModuleName = String, and then modulesToDiff: Set<ModuleName>

// Build the baseline revision to generate baseline files.
let modulesWithBaselines = try await generateAPIBaselineUsingIntegratedAPIDigesterSupport(swiftCommandState, baselineRevision: baselineRevision, baselineDir: baselineDir, modulesNeedingBaselines: modulesToDiff)

// Build the package and run a comparison agains the baselines.
var productsBuildParameters = try swiftCommandState.productsBuildParameters
productsBuildParameters.apiDigesterMode = .compareToBaselines(
baselinesDirectory: baselineDir,
modulesToCompare: modulesWithBaselines,
breakageAllowListPath: breakageAllowlistPath
)
let delegate = DiagnosticsCapturingBuildSystemDelegate()
let buildSystem = try await swiftCommandState.createBuildSystem(
traitConfiguration: .init(traitOptions: self.traits),
cacheBuildManifest: false,
productsBuildParameters: productsBuildParameters,
delegate: delegate
)
try await buildSystem.build()

// Report the results of the comparison.
var comparisonResults: [SwiftAPIDigester.ComparisonResult] = []
for (targetName, diagnosticPaths) in delegate.serializedDiagnosticsPathsByTarget {
guard let targetName, !diagnosticPaths.isEmpty else {
continue
}
var apiBreakingChanges: [SerializedDiagnostics.Diagnostic] = []
var otherDiagnostics: [SerializedDiagnostics.Diagnostic] = []
for path in diagnosticPaths {
let contents = try swiftCommandState.fileSystem.readFileContents(path)
guard contents.count > 0 else {
continue
}
let serializedDiagnostics = try SerializedDiagnostics(bytes: contents)
let apiDigesterCategory = "api-digester-breaking-change"
apiBreakingChanges.append(contentsOf: serializedDiagnostics.diagnostics.filter { $0.category == apiDigesterCategory })
otherDiagnostics.append(contentsOf: serializedDiagnostics.diagnostics.filter { $0.category != apiDigesterCategory })
}
let result = SwiftAPIDigester.ComparisonResult(
moduleName: targetName,
apiBreakingChanges: apiBreakingChanges,
otherDiagnostics: otherDiagnostics
)
comparisonResults.append(result)
}

var detectedBreakingChange = false
for result in comparisonResults.sorted(by: { $0.moduleName < $1.moduleName }) {
if result.hasNoAPIBreakingChanges && !modulesToDiff.contains(result.moduleName) {
continue
}
try printComparisonResult(result, observabilityScope: swiftCommandState.observabilityScope)
detectedBreakingChange = detectedBreakingChange || !result.hasNoAPIBreakingChanges
}

for module in modulesToDiff.subtracting(modulesWithBaselines) {
print("\nSkipping \(module) because it does not exist in the baseline")
}

if detectedBreakingChange {
throw ExitCode(1)
}
}

private func generateAPIBaselineUsingIntegratedAPIDigesterSupport(_ swiftCommandState: SwiftCommandState, baselineRevision: Revision, baselineDir: Basics.AbsolutePath, modulesNeedingBaselines: Set<String>) async throws -> Set<String> {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider created automated tests that validate this function in isolation with happy path, and also fault injection, and more.

Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: consider type aliasing, or creating more specific type, for the return value to aid in code readability. For example, instead of String, create a type alias which represents what the string is. e.g.: typealias ModuleName = String

// Setup a temporary directory where we can checkout and build the baseline treeish.
let baselinePackageRoot = try swiftCommandState.productsBuildParameters.apiDiff.appending("\(baselineRevision.identifier)-checkout")
if swiftCommandState.fileSystem.exists(baselinePackageRoot) {
try swiftCommandState.fileSystem.removeFileTree(baselinePackageRoot)
}
if regenerateBaseline && swiftCommandState.fileSystem.exists(baselineDir) {
try swiftCommandState.fileSystem.removeFileTree(baselineDir)
}

// Clone the current package in a sandbox and checkout the baseline revision.
let repositoryProvider = GitRepositoryProvider()
let specifier = RepositorySpecifier(path: baselinePackageRoot)
let workingCopy = try await repositoryProvider.createWorkingCopy(
repository: specifier,
sourcePath: swiftCommandState.getPackageRoot(),
at: baselinePackageRoot,
editable: false
)

try workingCopy.checkout(revision: baselineRevision)

// Create the workspace for this package.
let workspace = try Workspace(
forRootPackage: baselinePackageRoot,
cancellator: swiftCommandState.cancellator
)

let graph = try await workspace.loadPackageGraph(
rootPath: baselinePackageRoot,
observabilityScope: swiftCommandState.observabilityScope
)

let baselineModules = try Self.determineModulesToDiff(
packageGraph: graph,
productNames: products,
targetNames: targets,
observabilityScope: swiftCommandState.observabilityScope,
diagnoseMissingNames: false
)

// Don't emit a baseline for a module that didn't exist yet in this revision.
var modulesNeedingBaselines = modulesNeedingBaselines
modulesNeedingBaselines.formIntersection(graph.apiDigesterModules)

// Abort if we weren't able to load the package graph.
if swiftCommandState.observabilityScope.errorsReported {
throw Diagnostics.fatalError
}

// Update the data path input build parameters so it's built in the sandbox.
var productsBuildParameters = try swiftCommandState.productsBuildParameters
productsBuildParameters.dataPath = workspace.location.scratchDirectory
productsBuildParameters.apiDigesterMode = .generateBaselines(baselinesDirectory: baselineDir, modulesRequestingBaselines: modulesNeedingBaselines)

// Build the baseline module.
// FIXME: We need to implement the build tool invocation closure here so that build tool plugins work with the APIDigester. rdar://86112934
let buildSystem = try await swiftCommandState.createBuildSystem(
traitConfiguration: .init(),
cacheBuildManifest: false,
productsBuildParameters: productsBuildParameters,
packageGraphLoader: { graph }
)
try await buildSystem.build()
return baselineModules
}

private static func determineModulesToDiff(packageGraph: ModulesGraph, productNames: [String], targetNames: [String], observabilityScope: ObservabilityScope, diagnoseMissingNames: Bool) throws -> Set<String> {
var modulesToDiff: Set<String> = []
if products.isEmpty && targets.isEmpty {
if productNames.isEmpty && targetNames.isEmpty {
modulesToDiff.formUnion(packageGraph.apiDigesterModules)
} else {
for productName in products {
for productName in productNames {
guard let product = packageGraph
.rootPackages
.flatMap(\.products)
.first(where: { $0.name == productName }) else {
observabilityScope.emit(error: "no such product '\(productName)'")
if diagnoseMissingNames {
observabilityScope.emit(error: "no such product '\(productName)'")
}
continue
}
guard product.type.isLibrary else {
observabilityScope.emit(error: "'\(productName)' is not a library product")
if diagnoseMissingNames {
observabilityScope.emit(error: "'\(productName)' is not a library product")
}
continue
}
modulesToDiff.formUnion(product.modules.filter { $0.underlying is SwiftModule }.map(\.c99name))
}
for targetName in targets {
for targetName in targetNames {
guard let target = packageGraph
.rootPackages
.flatMap(\.modules)
.first(where: { $0.name == targetName }) else {
observabilityScope.emit(error: "no such target '\(targetName)'")
if diagnoseMissingNames {
observabilityScope.emit(error: "no such target '\(targetName)'")
}
continue
}
guard target.type == .library else {
observabilityScope.emit(error: "'\(targetName)' is not a library target")
if diagnoseMissingNames {
observabilityScope.emit(error: "'\(targetName)' is not a library target")
}
continue
}
guard target.underlying is SwiftModule else {
observabilityScope.emit(error: "'\(targetName)' is not a Swift language target")
if diagnoseMissingNames {
observabilityScope.emit(error: "'\(targetName)' is not a Swift language target")
}
continue
}
modulesToDiff.insert(target.c99name)
Expand Down
8 changes: 4 additions & 4 deletions Sources/Commands/Utilities/APIDigester.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public struct SwiftAPIDigester {
let result = try runTool(args)

if !self.fileSystem.exists(outputPath) {
throw Error.failedToGenerateBaseline(module: module)
throw Error.failedToGenerateBaseline(module: module, output: (try? result.utf8Output()) ?? "", error: (try? result.utf8stderrOutput()) ?? "")
}

try self.fileSystem.readFileContents(outputPath).withData { data in
Expand Down Expand Up @@ -272,14 +272,14 @@ public struct SwiftAPIDigester {

extension SwiftAPIDigester {
public enum Error: Swift.Error, CustomStringConvertible {
case failedToGenerateBaseline(module: String)
case failedToGenerateBaseline(module: String, output: String, error: String)
case failedToValidateBaseline(module: String)
case noSymbolsInBaseline(module: String, toolOutput: String)

public var description: String {
switch self {
case .failedToGenerateBaseline(let module):
return "failed to generate baseline for \(module)"
case .failedToGenerateBaseline(let module, let output, let error):
return "failed to generate baseline for \(module) (output: \(output), error: \(error)"
case .failedToValidateBaseline(let module):
return "failed to validate baseline for \(module)"
case .noSymbolsInBaseline(let module, let toolOutput):
Expand Down
14 changes: 10 additions & 4 deletions Sources/CoreCommands/BuildSystemSupport.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ private struct NativeBuildSystemFactory: BuildSystemFactory {
packageGraphLoader: (() async throws -> ModulesGraph)?,
outputStream: OutputByteStream?,
logLevel: Diagnostic.Severity?,
observabilityScope: ObservabilityScope?
observabilityScope: ObservabilityScope?,
delegate: BuildSystemDelegate?
) async throws -> any BuildSystem {
_ = try await swiftCommandState.getRootPackageInformation(traitConfiguration: traitConfiguration)
let testEntryPointPath = productsBuildParameters?.testProductStyle.explicitlySpecifiedEntryPointPath
Expand Down Expand Up @@ -68,7 +69,8 @@ private struct NativeBuildSystemFactory: BuildSystemFactory {
outputStream: outputStream ?? self.swiftCommandState.outputStream,
logLevel: logLevel ?? self.swiftCommandState.logLevel,
fileSystem: self.swiftCommandState.fileSystem,
observabilityScope: observabilityScope ?? self.swiftCommandState.observabilityScope)
observabilityScope: observabilityScope ?? self.swiftCommandState.observabilityScope,
delegate: delegate)
}
}

Expand All @@ -84,7 +86,8 @@ private struct XcodeBuildSystemFactory: BuildSystemFactory {
packageGraphLoader: (() async throws -> ModulesGraph)?,
outputStream: OutputByteStream?,
logLevel: Diagnostic.Severity?,
observabilityScope: ObservabilityScope?
observabilityScope: ObservabilityScope?,
delegate: BuildSystemDelegate?
) throws -> any BuildSystem {
return try XcodeBuildSystem(
buildParameters: productsBuildParameters ?? self.swiftCommandState.productsBuildParameters,
Expand All @@ -97,7 +100,8 @@ private struct XcodeBuildSystemFactory: BuildSystemFactory {
outputStream: outputStream ?? self.swiftCommandState.outputStream,
logLevel: logLevel ?? self.swiftCommandState.logLevel,
fileSystem: self.swiftCommandState.fileSystem,
observabilityScope: observabilityScope ?? self.swiftCommandState.observabilityScope
observabilityScope: observabilityScope ?? self.swiftCommandState.observabilityScope,
delegate: delegate
)
}
}
Expand All @@ -115,6 +119,7 @@ private struct SwiftBuildSystemFactory: BuildSystemFactory {
outputStream: OutputByteStream?,
logLevel: Diagnostic.Severity?,
observabilityScope: ObservabilityScope?,
delegate: BuildSystemDelegate?
) throws -> any BuildSystem {
return try SwiftBuildSystem(
buildParameters: productsBuildParameters ?? self.swiftCommandState.productsBuildParameters,
Expand All @@ -135,6 +140,7 @@ private struct SwiftBuildSystemFactory: BuildSystemFactory {
workDirectory: try self.swiftCommandState.getActiveWorkspace().location.pluginWorkingDirectory,
disableSandbox: self.swiftCommandState.shouldDisableSandbox
),
delegate: delegate
)
}
}
Expand Down
Loading