Skip to content

Commit c7ecfa5

Browse files
rauhulrauhul
committed
Fix swift-api-digester search paths
Fixes an architecture and implementation bug where the include paths for `swift-api-digester` were being determined inside build plan instead of by the target build description. This is effectively the same change as #7621 but for api-digester rather than symbolgraph-extract. This commit also leaves hooks in to enable api-digester on clang modules as a future improvement.
1 parent 9c0e48e commit c7ecfa5

File tree

11 files changed

+170
-97
lines changed

11 files changed

+170
-97
lines changed

‎Sources/Basics/Collections/IdentifiableSet.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,16 @@ public struct IdentifiableSet<Element: Identifiable>: Collection {
9797
public func contains(id: Element.ID) -> Bool {
9898
self.storage.keys.contains(id)
9999
}
100+
101+
public func filter(_ isIncluded: (Self.Element) throws -> Bool) rethrows -> Self {
102+
var copy = Self()
103+
for element in self {
104+
if try isIncluded(element) {
105+
copy.insert(element)
106+
}
107+
}
108+
return copy
109+
}
100110
}
101111

102112
extension OrderedDictionary where Value: Identifiable, Key == Value.ID {

‎Sources/Build/BuildDescription/ClangTargetBuildDescription.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,18 @@ public final class ClangTargetBuildDescription {
225225
return args
226226
}
227227

228+
/// Determines the arguments needed to run `swift-api-digester` for emitting
229+
/// an API baseline for this module.
230+
package func apiDigesterEmitBaselineArguments() throws -> [String] {
231+
throw InternalError("Unimplemented \(#function)")
232+
}
233+
234+
/// Determines the arguments needed to run `swift-api-digester` for
235+
/// comparing to an API baseline for this module.
236+
package func apiDigesterCompareBaselineArguments() throws -> [String] {
237+
throw InternalError("Unimplemented \(#function)")
238+
}
239+
228240
/// Builds up basic compilation arguments for a source file in this target; these arguments may be different for C++
229241
/// vs non-C++.
230242
/// NOTE: The parameter to specify whether to get C++ semantics is currently optional, but this is only for revlock

‎Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,44 @@ public final class SwiftTargetBuildDescription {
641641
return args
642642
}
643643

644+
/// Determines the arguments needed to run `swift-api-digester` for emitting
645+
/// an API baseline for this module.
646+
package func apiDigesterEmitBaselineArguments() throws -> [String] {
647+
var args = [String]()
648+
args += try self.apiDigesterCommonArguments()
649+
return args
650+
}
651+
652+
/// Determines the arguments needed to run `swift-api-digester` for
653+
/// comparing to an API baseline for this module.
654+
package func apiDigesterCompareBaselineArguments() throws -> [String] {
655+
var args = [String]()
656+
args += try self.apiDigesterCommonArguments()
657+
return args
658+
}
659+
660+
public func apiDigesterCommonArguments() throws -> [String] {
661+
var args = [String]()
662+
args += self.buildParameters.toolchain.extraFlags.swiftCompilerFlags
663+
664+
// FIXME: remove additionalFlags
665+
// Add search paths determined during planning
666+
args += self.additionalFlags
667+
args += ["-I", self.modulesPath.pathString]
668+
669+
// FIXME: Only include valid args
670+
// `swift-api-digester` args doesn't support -L args.
671+
for index in args.indices.dropLast().reversed() {
672+
if args[index] == "-L" {
673+
// Remove the flag
674+
args.remove(at: index)
675+
// Remove the argument
676+
args.remove(at: index)
677+
}
678+
}
679+
return args
680+
}
681+
644682
// FIXME: this function should operation on a strongly typed buildSetting
645683
// Move logic from PackageBuilder here.
646684
/// Determines the arguments needed for cxx interop for this module.

‎Sources/Build/BuildDescription/TargetBuildDescription.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,4 +124,22 @@ public enum TargetBuildDescription {
124124
case .clang(let target): try target.symbolGraphExtractArguments()
125125
}
126126
}
127+
128+
/// Determines the arguments needed to run `swift-api-digester` for emitting
129+
/// an API baseline for this module.
130+
package func apiDigesterEmitBaselineArguments() throws -> [String] {
131+
switch self {
132+
case .swift(let target): try target.apiDigesterEmitBaselineArguments()
133+
case .clang(let target): try target.apiDigesterEmitBaselineArguments()
134+
}
135+
}
136+
137+
/// Determines the arguments needed to run `swift-api-digester` for
138+
/// comparing to an API baseline for this module.
139+
package func apiDigesterCompareBaselineArguments() throws -> [String] {
140+
switch self {
141+
case .swift(let target): try target.apiDigesterCompareBaselineArguments()
142+
case .clang(let target): try target.apiDigesterCompareBaselineArguments()
143+
}
144+
}
127145
}

‎Sources/Build/BuildPlan/BuildPlan.swift

Lines changed: 18 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -523,53 +523,6 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
523523
// handle that situation.
524524
}
525525

526-
public func createAPIToolCommonArgs(includeLibrarySearchPaths: Bool) throws -> [String] {
527-
// API tool runs on products, hence using `self.productsBuildParameters`, not `self.toolsBuildParameters`
528-
let buildPath = self.destinationBuildParameters.buildPath.pathString
529-
var arguments = ["-I", buildPath]
530-
531-
// swift-symbolgraph-extract does not support parsing `-use-ld=lld` and
532-
// will silently error failing the operation. Filter out this flag
533-
// similar to how we filter out the library search path unless
534-
// explicitly requested.
535-
var extraSwiftCFlags = self.destinationBuildParameters.toolchain.extraFlags.swiftCompilerFlags
536-
.filter { !$0.starts(with: "-use-ld=") }
537-
if !includeLibrarySearchPaths {
538-
for index in extraSwiftCFlags.indices.dropLast().reversed() {
539-
if extraSwiftCFlags[index] == "-L" {
540-
// Remove the flag
541-
extraSwiftCFlags.remove(at: index)
542-
// Remove the argument
543-
extraSwiftCFlags.remove(at: index)
544-
}
545-
}
546-
}
547-
arguments += extraSwiftCFlags
548-
549-
// Add search paths to the directories containing module maps and Swift modules.
550-
for target in self.targets {
551-
switch target {
552-
case .swift(let targetDescription):
553-
arguments += ["-I", targetDescription.moduleOutputPath.parentDirectory.pathString]
554-
case .clang(let targetDescription):
555-
if let includeDir = targetDescription.moduleMap?.parentDirectory {
556-
arguments += ["-I", includeDir.pathString]
557-
}
558-
}
559-
}
560-
561-
// Add search paths from the system library targets.
562-
for target in self.graph.reachableTargets {
563-
if let systemLib = target.underlying as? SystemLibraryTarget {
564-
try arguments.append(contentsOf: self.pkgConfig(for: systemLib).cFlags)
565-
// Add the path to the module map.
566-
arguments += ["-I", systemLib.moduleMapPath.parentDirectory.pathString]
567-
}
568-
}
569-
570-
return arguments
571-
}
572-
573526
/// Creates arguments required to launch the Swift REPL that will allow
574527
/// importing the modules in the package graph.
575528
public func createREPLArguments() throws -> [String] {
@@ -656,6 +609,24 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
656609
}
657610
return try description.symbolGraphExtractArguments()
658611
}
612+
613+
/// Determines the arguments needed to run `swift-api-digester` for emitting
614+
/// an API baseline for a particular module.
615+
public func apiDigesterEmitBaselineArguments(for module: ResolvedModule) throws -> [String] {
616+
guard let description = self.targetMap[module.id] else {
617+
throw InternalError("Expected description for module \(module)")
618+
}
619+
return try description.apiDigesterEmitBaselineArguments()
620+
}
621+
622+
/// Determines the arguments needed to run `swift-api-digester` for
623+
/// comparing to an API baseline for a particular module.
624+
public func apiDigesterCompareBaselineArguments(for module: ResolvedModule) throws -> [String] {
625+
guard let description = self.targetMap[module.id] else {
626+
throw InternalError("Expected description for module \(module)")
627+
}
628+
return try description.apiDigesterCompareBaselineArguments()
629+
}
659630
}
660631

661632
extension Basics.Diagnostic {

‎Sources/Commands/PackageCommands/APIDiff.swift

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ import PackageModel
1919
import SourceControl
2020

2121
struct DeprecatedAPIDiff: ParsableCommand {
22-
static let configuration = CommandConfiguration(commandName: "experimental-api-diff",
23-
abstract: "Deprecated - use `swift package diagnose-api-breaking-changes` instead",
24-
shouldDisplay: false)
22+
static let configuration = CommandConfiguration(
23+
commandName: "experimental-api-diff",
24+
abstract: "Deprecated - use `swift package diagnose-api-breaking-changes` instead",
25+
shouldDisplay: false)
2526

2627
@Argument(parsing: .captureForPassthrough)
2728
var args: [String] = []
@@ -109,18 +110,18 @@ struct APIDiff: SwiftCommand {
109110
at: overrideBaselineDir,
110111
force: regenerateBaseline,
111112
logLevel: swiftCommandState.logLevel,
112-
swiftCommandState: swiftCommandState
113+
swiftCommandState: swiftCommandState
113114
)
114115

115116
let results = ThreadSafeArrayStore<SwiftAPIDigester.ComparisonResult>()
116117
let group = DispatchGroup()
117118
let semaphore = DispatchSemaphore(value: Int(try buildSystem.buildPlan.destinationBuildParameters.workers))
118-
var skippedModules: Set<String> = []
119+
var skippedModules = IdentifiableSet<ResolvedModule>()
119120

120121
for module in modulesToDiff {
121-
let moduleBaselinePath = baselineDir.appending("\(module).json")
122+
let moduleBaselinePath = baselineDir.appending("\(module.c99name).json")
122123
guard swiftCommandState.fileSystem.exists(moduleBaselinePath) else {
123-
print("\nSkipping \(module) because it does not exist in the baseline")
124+
print("\nSkipping \(module.c99name) because it does not exist in the baseline")
124125
skippedModules.insert(module)
125126
continue
126127
}
@@ -146,7 +147,7 @@ struct APIDiff: SwiftCommand {
146147

147148
let failedModules = modulesToDiff
148149
.subtracting(skippedModules)
149-
.subtracting(results.map(\.moduleName))
150+
.subtracting(results.map(\.module))
150151
for failedModule in failedModules {
151152
swiftCommandState.observabilityScope.emit(error: "failed to read API digester output for \(failedModule)")
152153
}
@@ -160,8 +161,8 @@ struct APIDiff: SwiftCommand {
160161
}
161162
}
162163

163-
private func determineModulesToDiff(packageGraph: ModulesGraph, observabilityScope: ObservabilityScope) throws -> Set<String> {
164-
var modulesToDiff: Set<String> = []
164+
private func determineModulesToDiff(packageGraph: ModulesGraph, observabilityScope: ObservabilityScope) throws -> IdentifiableSet<ResolvedModule> {
165+
var modulesToDiff = IdentifiableSet<ResolvedModule>()
165166
if products.isEmpty && targets.isEmpty {
166167
modulesToDiff.formUnion(packageGraph.apiDigesterModules)
167168
} else {
@@ -177,7 +178,10 @@ struct APIDiff: SwiftCommand {
177178
observabilityScope.emit(error: "'\(productName)' is not a library product")
178179
continue
179180
}
180-
modulesToDiff.formUnion(product.targets.filter { $0.underlying is SwiftTarget }.map(\.c99name))
181+
let swiftModules = product
182+
.targets
183+
.filter { $0.underlying is SwiftTarget }
184+
modulesToDiff.formUnion(swiftModules)
181185
}
182186
for targetName in targets {
183187
guard let target = packageGraph
@@ -195,7 +199,7 @@ struct APIDiff: SwiftCommand {
195199
observabilityScope.emit(error: "'\(targetName)' is not a Swift language target")
196200
continue
197201
}
198-
modulesToDiff.insert(target.c99name)
202+
modulesToDiff.insert(target)
199203
}
200204
guard !observabilityScope.errorsReported else {
201205
throw ExitCode.failure
@@ -232,7 +236,7 @@ struct APIDiff: SwiftCommand {
232236
}
233237
}
234238

235-
let moduleName = comparisonResult.moduleName
239+
let moduleName = comparisonResult.module.c99name
236240
if comparisonResult.apiBreakingChanges.isEmpty {
237241
print("\nNo breaking changes detected in \(moduleName)")
238242
} else {

0 commit comments

Comments
 (0)