Skip to content

Add an integration with swift-api-digester #721

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.

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 4 commits into from
Jul 3, 2021
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
4 changes: 2 additions & 2 deletions Package.resolved

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Sources/SwiftDriver/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ add_library(SwiftDriver
"IncrementalCompilation/SourceFileDependencyGraph.swift"
"IncrementalCompilation/TwoDMap.swift"

Jobs/APIDigesterJobs.swift
Jobs/AutolinkExtractJob.swift
Jobs/BackendJob.swift
Jobs/CommandLineArguments.swift
Expand Down
94 changes: 93 additions & 1 deletion Sources/SwiftDriver/Driver/Driver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public struct Driver {
case missingProfilingData(String)
case conditionalCompilationFlagHasRedundantPrefix(String)
case conditionalCompilationFlagIsNotValidIdentifier(String)
case baselineGenerationRequiresTopLevelModule(String)
case optionRequiresAnother(String, String)
// Explicit Module Build Failures
case malformedModuleDependency(String, String)
case missingPCMArguments(String)
Expand Down Expand Up @@ -100,6 +102,10 @@ public struct Driver {
return "unable to load output file map '\(path)': no such file or directory"
case .missingExternalDependency(let moduleName):
return "Missing External dependency info for module: \(moduleName)"
case .baselineGenerationRequiresTopLevelModule(let arg):
return "generating a baseline with '\(arg)' is only supported with '-emit-module' or '-emit-module-path'"
case .optionRequiresAnother(let first, let second):
return "'\(first)' cannot be specified if '\(second)' is not present"
}
}
}
Expand Down Expand Up @@ -288,6 +294,12 @@ public struct Driver {
/// Path to the Swift module source information file.
let moduleSourceInfoPath: VirtualPath.Handle?

/// Path to the module's digester baseline file.
let digesterBaselinePath: VirtualPath.Handle?

/// The mode the API digester should run in.
let digesterMode: DigesterMode

/// Force the driver to emit the module first and then run compile jobs. This could be used to unblock
/// dependencies in parallel builds.
var forceEmitModuleBeforeCompile: Bool = false
Expand Down Expand Up @@ -535,6 +547,16 @@ public struct Driver {
self.numThreads = Self.determineNumThreads(&parsedOptions, compilerMode: compilerMode, diagnosticsEngine: diagnosticEngine)
self.numParallelJobs = Self.determineNumParallelJobs(&parsedOptions, diagnosticsEngine: diagnosticEngine, env: env)

var mode = DigesterMode.api
if let modeArg = parsedOptions.getLastArgument(.digesterMode)?.asSingle {
if let digesterMode = DigesterMode(rawValue: modeArg) {
mode = digesterMode
} else {
diagnosticsEngine.emit(Error.invalidArgumentValue(Option.digesterMode.spelling, modeArg))
}
}
self.digesterMode = mode

Self.validateWarningControlArgs(&parsedOptions, diagnosticEngine: diagnosticEngine)
Self.validateProfilingArgs(&parsedOptions,
fileSystem: fileSystem,
Expand Down Expand Up @@ -587,7 +609,7 @@ public struct Driver {
diagnosticEngine: diagnosticEngine,
toolchain: toolchain,
targetInfo: frontendTargetInfo)

Self.validateSanitizerAddressUseOdrIndicatorFlag(&parsedOptions, diagnosticEngine: diagnosticsEngine, addressSanitizerEnabled: enabledSanitizers.contains(.address))

Self.validateSanitizerRecoverArgValues(&parsedOptions, diagnosticEngine: diagnosticsEngine, enabledSanitizers: enabledSanitizers)
Expand Down Expand Up @@ -663,6 +685,15 @@ public struct Driver {
outputFileMap: self.outputFileMap,
moduleName: moduleOutputInfo.name,
projectDirectory: projectDirectory)
self.digesterBaselinePath = try Self.computeDigesterBaselineOutputPath(
&parsedOptions,
moduleOutputPath: self.moduleOutputInfo.output?.outputPath,
mode: self.digesterMode,
compilerOutputType: compilerOutputType,
compilerMode: compilerMode,
outputFileMap: self.outputFileMap,
moduleName: moduleOutputInfo.name,
projectDirectory: projectDirectory)
self.swiftInterfacePath = try Self.computeSupplementaryOutputPath(
&parsedOptions, type: .swiftInterface, isOutputOptions: [.emitModuleInterface],
outputPath: .emitModuleInterfacePath,
Expand Down Expand Up @@ -700,6 +731,12 @@ public struct Driver {
outputFileMap: self.outputFileMap,
moduleName: moduleOutputInfo.name)

Self.validateDigesterArgs(&parsedOptions,
moduleOutputInfo: moduleOutputInfo,
digesterMode: self.digesterMode,
swiftInterfacePath: self.swiftInterfacePath,
diagnosticEngine: diagnosticsEngine)

try verifyOutputOptions()
}

Expand Down Expand Up @@ -2196,6 +2233,36 @@ extension Driver {
}
}

static func validateDigesterArgs(_ parsedOptions: inout ParsedOptions,
moduleOutputInfo: ModuleOutputInfo,
digesterMode: DigesterMode,
swiftInterfacePath: VirtualPath.Handle?,
diagnosticEngine: DiagnosticsEngine) {
if moduleOutputInfo.output?.isTopLevel != true {
for arg in parsedOptions.arguments(for: .emitDigesterBaseline, .emitDigesterBaselinePath, .compareToBaselinePath) {
diagnosticEngine.emit(Error.baselineGenerationRequiresTopLevelModule(arg.option.spelling))
}
}

if parsedOptions.hasArgument(.serializeBreakingChangesPath) && !parsedOptions.hasArgument(.compareToBaselinePath) {
diagnosticEngine.emit(Error.optionRequiresAnother(Option.serializeBreakingChangesPath.spelling,
Option.compareToBaselinePath.spelling))
}
if parsedOptions.hasArgument(.digesterBreakageAllowlistPath) && !parsedOptions.hasArgument(.compareToBaselinePath) {
diagnosticEngine.emit(Error.optionRequiresAnother(Option.digesterBreakageAllowlistPath.spelling,
Option.compareToBaselinePath.spelling))
}
if digesterMode == .abi && !parsedOptions.hasArgument(.enableLibraryEvolution) {
diagnosticEngine.emit(Error.optionRequiresAnother("\(Option.digesterMode.spelling) abi",
Option.enableLibraryEvolution.spelling))
}
if digesterMode == .abi && swiftInterfacePath == nil {
diagnosticEngine.emit(Error.optionRequiresAnother("\(Option.digesterMode.spelling) abi",
Option.emitModuleInterface.spelling))
}
}


static func validateProfilingArgs(_ parsedOptions: inout ParsedOptions,
fileSystem: FileSystem,
workingDirectory: AbsolutePath?,
Expand Down Expand Up @@ -2650,6 +2717,31 @@ extension Driver {
projectDirectory: projectDirectory)
}

static func computeDigesterBaselineOutputPath(
_ parsedOptions: inout ParsedOptions,
moduleOutputPath: VirtualPath.Handle?,
mode: DigesterMode,
compilerOutputType: FileType?,
compilerMode: CompilerMode,
outputFileMap: OutputFileMap?,
moduleName: String,
projectDirectory: VirtualPath.Handle?
) throws -> VirtualPath.Handle? {
// Only emit a baseline if at least of the arguments was provided.
guard parsedOptions.hasArgument(.emitDigesterBaseline, .emitDigesterBaselinePath) else { return nil }
return try computeModuleAuxiliaryOutputPath(&parsedOptions,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing the previous round of comments! Here will give us a location next to .swiftmodule file. Can we change it so that we emit next to .swiftsourceinfo file instead? .swiftsourceinfo is in the Project directory if you use XCBuild, which is a sub-directory reserved for local artifacts.

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 think this is already using the same logic as .swiftsourceinfo, they both call computeModuleAuxiliaryOutputPath and should use the Project folder if it's present. The only difference is that source info is opt-out and baseline generation is opt-in. I have a couple basic tests for this in testBaselineOutputPath which roughly correspond to the ones in testSourceInfoFileEmitOption. It's possible I made a mistake somewhere though.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, you are right. I meant the build system has already been passing down the path of .swiftsourceinfo via output file map. We could just use that to decide where we should put the baseline files. I presume we could add a special case in here: https://github.com/apple/swift-driver/blob/main/Sources/SwiftDriver/Driver/OutputFileMap.swift#L49

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see, that makes sense! I added the new rule and a couple tests involving output file maps

moduleOutputPath: moduleOutputPath,
type: mode.baselineFileType,
isOutput: .emitDigesterBaseline,
outputPath: .emitDigesterBaselinePath,
compilerOutputType: compilerOutputType,
compilerMode: compilerMode,
outputFileMap: outputFileMap,
moduleName: moduleName,
projectDirectory: projectDirectory)
}



/// Determine the output path for a module auxiliary output.
static func computeModuleAuxiliaryOutputPath(
Expand Down
7 changes: 7 additions & 0 deletions Sources/SwiftDriver/Driver/OutputFileMap.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ public struct OutputFileMap: Hashable, Codable {
}
return VirtualPath.lookup(path).replacingExtension(with: outputType).intern()

case .jsonAPIBaseline, .jsonABIBaseline:
// Infer paths for these entities using .swiftsourceinfo path.
guard let path = entries[inputFile]?[.swiftSourceInfoFile] else {
return nil
}
return VirtualPath.lookup(path).replacingExtension(with: outputType).intern()

case .object:
// We may generate .o files from bitcode .bc files, but the output file map
// uses .swift file as the key for .o file paths. So we need to dig further.
Expand Down
152 changes: 152 additions & 0 deletions Sources/SwiftDriver/Jobs/APIDigesterJobs.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
//===- APIDigesterJobs.swift - Baseline Generation and API/ABI Comparison -===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2021 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import TSCBasic

enum DigesterMode: String {
case api, abi

var baselineFileType: FileType {
switch self {
case .api:
return .jsonAPIBaseline
case .abi:
return .jsonABIBaseline
}
}

var baselineGenerationJobKind: Job.Kind {
switch self {
case .api:
return .generateAPIBaseline
case .abi:
return .generateABIBaseline
}
}

var baselineComparisonJobKind: Job.Kind {
switch self {
case .api:
return .compareAPIBaseline
case .abi:
return .compareABIBaseline
}
}
}

extension Driver {
mutating func digesterBaselineGenerationJob(modulePath: VirtualPath.Handle, outputPath: VirtualPath.Handle, mode: DigesterMode) throws -> Job {
var commandLine = [Job.ArgTemplate]()
commandLine.appendFlag("-dump-sdk")

try addCommonDigesterOptions(&commandLine, modulePath: modulePath, mode: mode)

commandLine.appendFlag(.o)
commandLine.appendPath(VirtualPath.lookup(outputPath))

return Job(
moduleName: moduleOutputInfo.name,
kind: mode.baselineGenerationJobKind,
tool: .absolute(try toolchain.getToolPath(.swiftAPIDigester)),
commandLine: commandLine,
inputs: [.init(file: modulePath, type: .swiftModule)],
Copy link
Contributor

Choose a reason for hiding this comment

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

For -abi, we should also require the interface file as the input. Please also add digester flag -use-interface-for-module CurrentMainModule when generating ABI baselines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 makes sense, done!

primaryInputs: [],
outputs: [.init(file: outputPath, type: mode.baselineFileType)],
supportsResponseFiles: true
)
}

mutating func digesterCompareToBaselineJob(modulePath: VirtualPath.Handle, baselinePath: VirtualPath.Handle, mode: DigesterMode) throws -> Job {
var commandLine = [Job.ArgTemplate]()
commandLine.appendFlag("-diagnose-sdk")
commandLine.appendFlag("-disable-fail-on-error")
commandLine.appendFlag("-baseline-path")
commandLine.appendPath(VirtualPath.lookup(baselinePath))

try addCommonDigesterOptions(&commandLine, modulePath: modulePath, mode: mode)

var serializedDiagnosticsPath: VirtualPath.Handle?
if let arg = parsedOptions.getLastArgument(.serializeBreakingChangesPath)?.asSingle {
let path = try VirtualPath.intern(path: arg)
commandLine.appendFlag("-serialize-diagnostics-path")
commandLine.appendPath(VirtualPath.lookup(path))
serializedDiagnosticsPath = path
}
if let arg = parsedOptions.getLastArgument(.digesterBreakageAllowlistPath)?.asSingle {
let path = try VirtualPath(path: arg)
commandLine.appendFlag("-breakage-allowlist-path")
commandLine.appendPath(path)
}

var inputs: [TypedVirtualPath] = [.init(file: modulePath, type: .swiftModule),
.init(file: baselinePath, type: mode.baselineFileType)]
// If a module interface was emitted, treat it as an input in ABI mode.
if let interfacePath = self.swiftInterfacePath, mode == .abi {
inputs.append(.init(file: interfacePath, type: .swiftInterface))
}

return Job(
moduleName: moduleOutputInfo.name,
kind: mode.baselineComparisonJobKind,
tool: .absolute(try toolchain.getToolPath(.swiftAPIDigester)),
commandLine: commandLine,
inputs: inputs,
primaryInputs: [],
outputs: [.init(file: serializedDiagnosticsPath ?? VirtualPath.Handle.standardOutput, type: .diagnostics)],
supportsResponseFiles: true
)
}

private mutating func addCommonDigesterOptions(_ commandLine: inout [Job.ArgTemplate],
modulePath: VirtualPath.Handle,
mode: DigesterMode) throws {
commandLine.appendFlag("-module")
commandLine.appendFlag(moduleOutputInfo.name)
if mode == .abi {
commandLine.appendFlag("-abi")
commandLine.appendFlag("-use-interface-for-module")
commandLine.appendFlag(moduleOutputInfo.name)
}

// Add a search path for the emitted module, and its module interface if there is one.
let searchPath = VirtualPath.lookup(modulePath).parentDirectory
commandLine.appendFlag(.I)
commandLine.appendPath(searchPath)
if let interfacePath = self.swiftInterfacePath {
let interfaceSearchPath = VirtualPath.lookup(interfacePath).parentDirectory
if interfaceSearchPath != searchPath {
commandLine.appendFlag(.I)
commandLine.appendPath(interfaceSearchPath)
}
}

commandLine.appendFlag(.target)
commandLine.appendFlag(targetTriple.triple)

if let sdkPath = frontendTargetInfo.sdkPath?.path {
commandLine.appendFlag(.sdk)
commandLine.append(.path(VirtualPath.lookup(sdkPath)))
}

commandLine.appendFlag(.resourceDir)
commandLine.appendPath(VirtualPath.lookup(frontendTargetInfo.runtimeResourcePath.path))

try commandLine.appendAll(.I, from: &parsedOptions)
try commandLine.appendAll(.F, from: &parsedOptions)
for systemFramework in parsedOptions.arguments(for: .Fsystem) {
commandLine.appendFlag(.iframework)
commandLine.appendFlag(systemFramework.argument.asSingle)
}

try commandLine.appendLast(.swiftVersion, from: &parsedOptions)
}
}
4 changes: 2 additions & 2 deletions Sources/SwiftDriver/Jobs/CompileJob.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ extension Driver {
.privateSwiftInterface, .swiftSourceInfoFile, .diagnostics, .objcHeader, .swiftDeps,
.remap, .tbd, .moduleTrace, .yamlOptimizationRecord, .bitstreamOptimizationRecord, .pcm,
.pch, .clangModuleMap, .jsonCompilerFeatures, .jsonTargetInfo, .jsonSwiftArtifacts,
.indexUnitOutputPath, .modDepCache, nil:
.indexUnitOutputPath, .modDepCache, .jsonAPIBaseline, .jsonABIBaseline, nil:
return false
}
}
Expand Down Expand Up @@ -444,7 +444,7 @@ extension FileType {
.diagnostics, .objcHeader, .image, .swiftDeps, .moduleTrace, .tbd,
.yamlOptimizationRecord, .bitstreamOptimizationRecord, .swiftInterface,
.privateSwiftInterface, .swiftSourceInfoFile, .clangModuleMap, .jsonSwiftArtifacts,
.indexUnitOutputPath, .modDepCache:
.indexUnitOutputPath, .modDepCache, .jsonAPIBaseline, .jsonABIBaseline:
fatalError("Output type can never be a primary output")
}
}
Expand Down
Loading