diff --git a/Sources/Basics/Collections/IdentifiableSet.swift b/Sources/Basics/Collections/IdentifiableSet.swift index 46383085b2b..e449f4713c9 100644 --- a/Sources/Basics/Collections/IdentifiableSet.swift +++ b/Sources/Basics/Collections/IdentifiableSet.swift @@ -97,6 +97,16 @@ public struct IdentifiableSet: Collection { public func contains(id: Element.ID) -> Bool { self.storage.keys.contains(id) } + + public func filter(_ isIncluded: (Self.Element) throws -> Bool) rethrows -> Self { + var copy = Self() + for element in self { + if try isIncluded(element) { + copy.insert(element) + } + } + return copy + } } extension OrderedDictionary where Value: Identifiable, Key == Value.ID { @@ -118,3 +128,9 @@ extension IdentifiableSet: Hashable { } } } + +extension IdentifiableSet: ExpressibleByArrayLiteral { + public init(arrayLiteral elements: Element...) { + self.init(elements) + } +} diff --git a/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift b/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift index 0b3b0dc39d0..885ff0d095c 100644 --- a/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift @@ -225,6 +225,18 @@ public final class ClangTargetBuildDescription { return args } + /// Determines the arguments needed to run `swift-api-digester` for emitting + /// an API baseline for this module. + package func apiDigesterEmitBaselineArguments() throws -> [String] { + throw InternalError("Unimplemented \(#function)") + } + + /// Determines the arguments needed to run `swift-api-digester` for + /// comparing to an API baseline for this module. + package func apiDigesterCompareBaselineArguments() throws -> [String] { + throw InternalError("Unimplemented \(#function)") + } + /// Builds up basic compilation arguments for a source file in this target; these arguments may be different for C++ /// vs non-C++. /// NOTE: The parameter to specify whether to get C++ semantics is currently optional, but this is only for revlock diff --git a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift index 514e05f9957..d9c4559b4b8 100644 --- a/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift @@ -641,6 +641,44 @@ public final class SwiftTargetBuildDescription { return args } + /// Determines the arguments needed to run `swift-api-digester` for emitting + /// an API baseline for this module. + package func apiDigesterEmitBaselineArguments() throws -> [String] { + var args = [String]() + args += try self.apiDigesterCommonArguments() + return args + } + + /// Determines the arguments needed to run `swift-api-digester` for + /// comparing to an API baseline for this module. + package func apiDigesterCompareBaselineArguments() throws -> [String] { + var args = [String]() + args += try self.apiDigesterCommonArguments() + return args + } + + public func apiDigesterCommonArguments() throws -> [String] { + var args = [String]() + args += self.buildParameters.toolchain.extraFlags.swiftCompilerFlags + + // FIXME: remove additionalFlags + // Add search paths determined during planning + args += self.additionalFlags + args += ["-I", self.modulesPath.pathString] + + // FIXME: Only include valid args + // `swift-api-digester` args doesn't support -L args. + for index in args.indices.dropLast().reversed() { + if args[index] == "-L" { + // Remove the flag + args.remove(at: index) + // Remove the argument + args.remove(at: index) + } + } + return args + } + // FIXME: this function should operation on a strongly typed buildSetting // Move logic from PackageBuilder here. /// Determines the arguments needed for cxx interop for this module. diff --git a/Sources/Build/BuildDescription/TargetBuildDescription.swift b/Sources/Build/BuildDescription/TargetBuildDescription.swift index 3e97fede5b7..f61d745708c 100644 --- a/Sources/Build/BuildDescription/TargetBuildDescription.swift +++ b/Sources/Build/BuildDescription/TargetBuildDescription.swift @@ -124,4 +124,22 @@ public enum TargetBuildDescription { case .clang(let target): try target.symbolGraphExtractArguments() } } + + /// Determines the arguments needed to run `swift-api-digester` for emitting + /// an API baseline for this module. + package func apiDigesterEmitBaselineArguments() throws -> [String] { + switch self { + case .swift(let target): try target.apiDigesterEmitBaselineArguments() + case .clang(let target): try target.apiDigesterEmitBaselineArguments() + } + } + + /// Determines the arguments needed to run `swift-api-digester` for + /// comparing to an API baseline for this module. + package func apiDigesterCompareBaselineArguments() throws -> [String] { + switch self { + case .swift(let target): try target.apiDigesterCompareBaselineArguments() + case .clang(let target): try target.apiDigesterCompareBaselineArguments() + } + } } diff --git a/Sources/Build/BuildPlan/BuildPlan.swift b/Sources/Build/BuildPlan/BuildPlan.swift index bbbe94b243c..ddb83cbebc1 100644 --- a/Sources/Build/BuildPlan/BuildPlan.swift +++ b/Sources/Build/BuildPlan/BuildPlan.swift @@ -523,53 +523,6 @@ public class BuildPlan: SPMBuildCore.BuildPlan { // handle that situation. } - public func createAPIToolCommonArgs(includeLibrarySearchPaths: Bool) throws -> [String] { - // API tool runs on products, hence using `self.productsBuildParameters`, not `self.toolsBuildParameters` - let buildPath = self.destinationBuildParameters.buildPath.pathString - var arguments = ["-I", buildPath] - - // swift-symbolgraph-extract does not support parsing `-use-ld=lld` and - // will silently error failing the operation. Filter out this flag - // similar to how we filter out the library search path unless - // explicitly requested. - var extraSwiftCFlags = self.destinationBuildParameters.toolchain.extraFlags.swiftCompilerFlags - .filter { !$0.starts(with: "-use-ld=") } - if !includeLibrarySearchPaths { - for index in extraSwiftCFlags.indices.dropLast().reversed() { - if extraSwiftCFlags[index] == "-L" { - // Remove the flag - extraSwiftCFlags.remove(at: index) - // Remove the argument - extraSwiftCFlags.remove(at: index) - } - } - } - arguments += extraSwiftCFlags - - // Add search paths to the directories containing module maps and Swift modules. - for target in self.targets { - switch target { - case .swift(let targetDescription): - arguments += ["-I", targetDescription.moduleOutputPath.parentDirectory.pathString] - case .clang(let targetDescription): - if let includeDir = targetDescription.moduleMap?.parentDirectory { - arguments += ["-I", includeDir.pathString] - } - } - } - - // Add search paths from the system library targets. - for target in self.graph.reachableTargets { - if let systemLib = target.underlying as? SystemLibraryTarget { - try arguments.append(contentsOf: self.pkgConfig(for: systemLib).cFlags) - // Add the path to the module map. - arguments += ["-I", systemLib.moduleMapPath.parentDirectory.pathString] - } - } - - return arguments - } - /// Creates arguments required to launch the Swift REPL that will allow /// importing the modules in the package graph. public func createREPLArguments() throws -> [String] { @@ -656,6 +609,24 @@ public class BuildPlan: SPMBuildCore.BuildPlan { } return try description.symbolGraphExtractArguments() } + + /// Determines the arguments needed to run `swift-api-digester` for emitting + /// an API baseline for a particular module. + public func apiDigesterEmitBaselineArguments(for module: ResolvedModule) throws -> [String] { + guard let description = self.targetMap[module.id] else { + throw InternalError("Expected description for module \(module)") + } + return try description.apiDigesterEmitBaselineArguments() + } + + /// Determines the arguments needed to run `swift-api-digester` for + /// comparing to an API baseline for a particular module. + public func apiDigesterCompareBaselineArguments(for module: ResolvedModule) throws -> [String] { + guard let description = self.targetMap[module.id] else { + throw InternalError("Expected description for module \(module)") + } + return try description.apiDigesterCompareBaselineArguments() + } } extension Basics.Diagnostic { diff --git a/Sources/Commands/PackageCommands/APIDiff.swift b/Sources/Commands/PackageCommands/APIDiff.swift index acb38e87ed0..b596e3d889c 100644 --- a/Sources/Commands/PackageCommands/APIDiff.swift +++ b/Sources/Commands/PackageCommands/APIDiff.swift @@ -19,9 +19,10 @@ import PackageModel import SourceControl struct DeprecatedAPIDiff: ParsableCommand { - static let configuration = CommandConfiguration(commandName: "experimental-api-diff", - abstract: "Deprecated - use `swift package diagnose-api-breaking-changes` instead", - shouldDisplay: false) + static let configuration = CommandConfiguration( + commandName: "experimental-api-diff", + abstract: "Deprecated - use `swift package diagnose-api-breaking-changes` instead", + shouldDisplay: false) @Argument(parsing: .captureForPassthrough) var args: [String] = [] @@ -109,18 +110,18 @@ struct APIDiff: SwiftCommand { at: overrideBaselineDir, force: regenerateBaseline, logLevel: swiftCommandState.logLevel, - swiftCommandState: swiftCommandState + swiftCommandState: swiftCommandState ) let results = ThreadSafeArrayStore() let group = DispatchGroup() let semaphore = DispatchSemaphore(value: Int(try buildSystem.buildPlan.destinationBuildParameters.workers)) - var skippedModules: Set = [] + var skippedModules = IdentifiableSet() for module in modulesToDiff { - let moduleBaselinePath = baselineDir.appending("\(module).json") + let moduleBaselinePath = baselineDir.appending("\(module.c99name).json") guard swiftCommandState.fileSystem.exists(moduleBaselinePath) else { - print("\nSkipping \(module) because it does not exist in the baseline") + print("\nSkipping \(module.c99name) because it does not exist in the baseline") skippedModules.insert(module) continue } @@ -146,7 +147,7 @@ struct APIDiff: SwiftCommand { let failedModules = modulesToDiff .subtracting(skippedModules) - .subtracting(results.map(\.moduleName)) + .subtracting(results.map(\.module)) for failedModule in failedModules { swiftCommandState.observabilityScope.emit(error: "failed to read API digester output for \(failedModule)") } @@ -160,8 +161,8 @@ struct APIDiff: SwiftCommand { } } - private func determineModulesToDiff(packageGraph: ModulesGraph, observabilityScope: ObservabilityScope) throws -> Set { - var modulesToDiff: Set = [] + private func determineModulesToDiff(packageGraph: ModulesGraph, observabilityScope: ObservabilityScope) throws -> IdentifiableSet { + var modulesToDiff = IdentifiableSet() if products.isEmpty && targets.isEmpty { modulesToDiff.formUnion(packageGraph.apiDigesterModules) } else { @@ -177,7 +178,10 @@ struct APIDiff: SwiftCommand { observabilityScope.emit(error: "'\(productName)' is not a library product") continue } - modulesToDiff.formUnion(product.targets.filter { $0.underlying is SwiftTarget }.map(\.c99name)) + let swiftModules = product + .targets + .filter { $0.underlying is SwiftTarget } + modulesToDiff.formUnion(swiftModules) } for targetName in targets { guard let target = packageGraph @@ -195,7 +199,7 @@ struct APIDiff: SwiftCommand { observabilityScope.emit(error: "'\(targetName)' is not a Swift language target") continue } - modulesToDiff.insert(target.c99name) + modulesToDiff.insert(target) } guard !observabilityScope.errorsReported else { throw ExitCode.failure @@ -232,7 +236,7 @@ struct APIDiff: SwiftCommand { } } - let moduleName = comparisonResult.moduleName + let moduleName = comparisonResult.module.c99name if comparisonResult.apiBreakingChanges.isEmpty { print("\nNo breaking changes detected in \(moduleName)") } else { diff --git a/Sources/Commands/Utilities/APIDigester.swift b/Sources/Commands/Utilities/APIDigester.swift index 7a8b5f4dd6b..ac4850aa8b1 100644 --- a/Sources/Commands/Utilities/APIDigester.swift +++ b/Sources/Commands/Utilities/APIDigester.swift @@ -69,27 +69,27 @@ struct APIDigesterBaselineDumper { /// Emit the API baseline files and return the path to their directory. func emitAPIBaseline( - for modulesToDiff: Set, + for modulesToDiffInCurrentRevision: IdentifiableSet, at baselineDir: AbsolutePath?, force: Bool, logLevel: Basics.Diagnostic.Severity, swiftCommandState: SwiftCommandState ) throws -> AbsolutePath { - var modulesToDiff = modulesToDiff + var modulesToDiffInCurrentRevision = modulesToDiffInCurrentRevision let apiDiffDir = productsBuildParameters.apiDiff let baselineDir = (baselineDir ?? apiDiffDir).appending(component: baselineRevision.identifier) - let baselinePath: (String)->AbsolutePath = { module in - baselineDir.appending(component: module + ".json") + let baselinePath: (ResolvedModule) -> AbsolutePath = { module in + baselineDir.appending(component: module.c99name + ".json") } if !force { // Baselines which already exist don't need to be regenerated. - modulesToDiff = modulesToDiff.filter { + modulesToDiffInCurrentRevision = modulesToDiffInCurrentRevision.filter { !swiftCommandState.fileSystem.exists(baselinePath($0)) } } - guard !modulesToDiff.isEmpty else { + guard !modulesToDiffInCurrentRevision.isEmpty else { // If none of the baselines need to be regenerated, return. return baselineDir } @@ -123,8 +123,14 @@ struct APIDigesterBaselineDumper { observabilityScope: self.observabilityScope ) - // Don't emit a baseline for a module that didn't exist yet in this revision. - modulesToDiff.formIntersection(graph.apiDigesterModules) + // FIXME: Indicate modules added or removed instead of ignoring + // Only emit a baseline for modules that exists both in the previous + // revision and the current revision of the package. + let modulesToDiffInCurrentRevisionC99Names = Set(modulesToDiffInCurrentRevision.map(\.c99name)) + let modulesInPreviousRevision = graph.apiDigesterModules + let modulesToDiff = modulesInPreviousRevision.filter { + modulesToDiffInCurrentRevisionC99Names.contains($0.c99name) + } // Abort if we weren't able to load the package graph. if observabilityScope.errorsReported { @@ -195,12 +201,15 @@ public struct SwiftAPIDigester { /// Emit an API baseline file for the specified module at the specified location. public func emitAPIBaseline( to outputPath: AbsolutePath, - for module: String, - buildPlan: SPMBuildCore.BuildPlan + for module: ResolvedModule, + buildPlan: BuildPlan ) throws { - var args = ["-dump-sdk", "-compiler-style-diags"] - args += try buildPlan.createAPIToolCommonArgs(includeLibrarySearchPaths: false) - args += ["-module", module, "-o", outputPath.pathString] + var args = [String]() + args += try buildPlan.apiDigesterEmitBaselineArguments(for: module) + + // FIXME: everything here should be in apiDigesterEmitBaselineArguments + args += ["-dump-sdk", "-compiler-style-diags"] + args += ["-module", module.c99name, "-o", outputPath.pathString] let result = try runTool(args) @@ -225,16 +234,19 @@ public struct SwiftAPIDigester { /// Compare the current package API to a provided baseline file. public func compareAPIToBaseline( at baselinePath: AbsolutePath, - for module: String, + for module: ResolvedModule, buildPlan: SPMBuildCore.BuildPlan, except breakageAllowlistPath: AbsolutePath? ) throws -> ComparisonResult? { - var args = [ + var args = [String]() + args += try buildPlan.apiDigesterCompareBaselineArguments(for: module) + + // FIXME: everything here should be in apiDigesterCompareBaselineArguments + args += [ "-diagnose-sdk", "-baseline-path", baselinePath.pathString, - "-module", module + "-module", module.c99name ] - args.append(contentsOf: try buildPlan.createAPIToolCommonArgs(includeLibrarySearchPaths: false)) if let breakageAllowlistPath { args.append(contentsOf: ["-breakage-allowlist-path", breakageAllowlistPath.pathString]) } @@ -250,9 +262,10 @@ public struct SwiftAPIDigester { let apiDigesterCategory = "api-digester-breaking-change" let apiBreakingChanges = serializedDiagnostics.diagnostics.filter { $0.category == apiDigesterCategory } let otherDiagnostics = serializedDiagnostics.diagnostics.filter { $0.category != apiDigesterCategory } - return ComparisonResult(moduleName: module, - apiBreakingChanges: apiBreakingChanges, - otherDiagnostics: otherDiagnostics) + return ComparisonResult( + module: module, + apiBreakingChanges: apiBreakingChanges, + otherDiagnostics: otherDiagnostics) } } @@ -269,18 +282,18 @@ public struct SwiftAPIDigester { extension SwiftAPIDigester { public enum Error: Swift.Error, CustomStringConvertible { - case failedToGenerateBaseline(module: String) - case failedToValidateBaseline(module: String) - case noSymbolsInBaseline(module: String, toolOutput: String) + case failedToGenerateBaseline(module: ResolvedModule) + case failedToValidateBaseline(module: ResolvedModule) + case noSymbolsInBaseline(module: ResolvedModule, toolOutput: String) public var description: String { switch self { case .failedToGenerateBaseline(let module): - return "failed to generate baseline for \(module)" + return "failed to generate baseline for \(module.c99name)" case .failedToValidateBaseline(let module): - return "failed to validate baseline for \(module)" + return "failed to validate baseline for \(module.c99name)" case .noSymbolsInBaseline(let module, let toolOutput): - return "baseline for \(module) contains no symbols, swift-api-digester output: \(toolOutput)" + return "baseline for \(module.c99name) contains no symbols, swift-api-digester output: \(toolOutput)" } } } @@ -289,8 +302,8 @@ extension SwiftAPIDigester { extension SwiftAPIDigester { /// The result of comparing a module's API to a provided baseline. public struct ComparisonResult { - /// The name of the module being diffed. - var moduleName: String + /// The module being diffed. + var module: ResolvedModule /// Breaking changes made to the API since the baseline was generated. var apiBreakingChanges: [SerializedDiagnostics.Diagnostic] /// Other diagnostics emitted while comparing the current API to the baseline. @@ -312,13 +325,14 @@ extension BuildParameters { extension ModulesGraph { /// The list of modules that should be used as an input to the API digester. - var apiDigesterModules: [String] { + var apiDigesterModules: IdentifiableSet { self.rootPackages .flatMap(\.products) .filter { $0.type.isLibrary } .flatMap(\.targets) + // FIXME: this should be runnable on ClangTargets .filter { $0.underlying is SwiftTarget } - .map { $0.c99name } + .reduce(into: .init()) { $0.insert($1) } } } diff --git a/Sources/PackageGraph/ModulesGraph+Loading.swift b/Sources/PackageGraph/ModulesGraph+Loading.swift index 085e118be2e..4803994edba 100644 --- a/Sources/PackageGraph/ModulesGraph+Loading.swift +++ b/Sources/PackageGraph/ModulesGraph+Loading.swift @@ -94,7 +94,7 @@ extension ModulesGraph { )) return try ModulesGraph( - rootPackages: [], + rootPackages: .init(), rootDependencies: [], packages: IdentifiableSet(), dependencies: requiredDependencies, @@ -188,7 +188,7 @@ extension ModulesGraph { private func checkAllDependenciesAreUsed( packages: IdentifiableSet, - _ rootPackages: [ResolvedPackage], + _ rootPackages: IdentifiableSet, observabilityScope: ObservabilityScope ) { for package in rootPackages { diff --git a/Sources/PackageGraph/ModulesGraph.swift b/Sources/PackageGraph/ModulesGraph.swift index 9558e494709..52fb60fa789 100644 --- a/Sources/PackageGraph/ModulesGraph.swift +++ b/Sources/PackageGraph/ModulesGraph.swift @@ -232,13 +232,12 @@ public struct ModulesGraph { /// Construct a package graph directly. public init( - rootPackages: [ResolvedPackage], + rootPackages: IdentifiableSet, rootDependencies: [ResolvedPackage] = [], packages: IdentifiableSet, dependencies requiredDependencies: [PackageReference], binaryArtifacts: [PackageIdentity: [String: BinaryArtifact]] ) throws { - let rootPackages = IdentifiableSet(rootPackages) self.requiredDependencies = requiredDependencies self.inputPackages = rootPackages + rootDependencies self.binaryArtifacts = binaryArtifacts diff --git a/Sources/SPMBuildCore/BuildSystem/BuildSystem.swift b/Sources/SPMBuildCore/BuildSystem/BuildSystem.swift index 37d770a8bd3..b3523f0b87d 100644 --- a/Sources/SPMBuildCore/BuildSystem/BuildSystem.swift +++ b/Sources/SPMBuildCore/BuildSystem/BuildSystem.swift @@ -90,12 +90,19 @@ public protocol BuildPlan { var buildProducts: AnySequence { get } - func createAPIToolCommonArgs(includeLibrarySearchPaths: Bool) throws -> [String] func createREPLArguments() throws -> [String] /// Determines the arguments needed to run `swift-symbolgraph-extract` for /// a particular module. func symbolGraphExtractArguments(for module: ResolvedModule) throws -> [String] + + /// Determines the arguments needed to run `swift-api-digester` for emitting + /// an API baseline for a particular module. + func apiDigesterEmitBaselineArguments(for module: ResolvedModule) throws -> [String] + + /// Determines the arguments needed to run `swift-api-digester` for + /// comparing to an API baseline for a particular module. + func apiDigesterCompareBaselineArguments(for module: ResolvedModule) throws -> [String] } public protocol BuildSystemFactory {