Skip to content

[Explicit Module Builds] Add API to specify richer external target module details #779

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 1 commit into from
Jul 30, 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
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ let package = Package(
name: "SwiftDriver",
dependencies: ["SwiftOptions", "SwiftToolsSupport-auto",
"CSwiftScan", "Yams"],
exclude: ["CMakeLists.txt"]),
exclude: ["CMakeLists.txt", SwiftDriver.docc]),
Copy link
Contributor

Choose a reason for hiding this comment

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

This broke the build because "SwiftDriver.docc" isn't a quoted string. Are we not building swift-driver as a package in swift-ci anywhere? /CC @shahmishal

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixing in #782


/// The execution library.
.target(
Expand Down
23 changes: 19 additions & 4 deletions Sources/SwiftDriver/Driver/Driver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public struct Driver {
case invalidArgumentValue(String, String)
case relativeFrontendPath(String)
case subcommandPassedToDriver
case externalTargetDetailsAPIError
case integratedReplRemoved
case cannotSpecify_OForMultipleOutputs
case conflictingOptions(Option, Option)
Expand Down Expand Up @@ -59,6 +60,8 @@ public struct Driver {
return "relative frontend path: \(path)"
case .subcommandPassedToDriver:
return "subcommand passed to driver"
case .externalTargetDetailsAPIError:
return "Cannot specify both: externalTargetModulePathMap and externalTargetModuleDetailsMap"
case .integratedReplRemoved:
return "Compiler-internal integrated REPL has been removed; use the LLDB-enhanced REPL instead."
case .cannotSpecify_OForMultipleOutputs:
Expand Down Expand Up @@ -319,7 +322,7 @@ public struct Driver {

/// A dictionary of external targets that are a part of the same build, mapping to filesystem paths
/// of their module files
@_spi(Testing) public var externalTargetModulePathMap: ExternalTargetModulePathMap? = nil
@_spi(Testing) public var externalTargetModuleDetailsMap: ExternalTargetModuleDetailsMap? = nil

/// A collection of all the flags the selected toolchain's `swift-frontend` supports
public let supportedFrontendFlags: Set<String>
Expand Down Expand Up @@ -400,8 +403,11 @@ public struct Driver {
/// an executable or as a library.
/// - Parameter compilerExecutableDir: Directory that contains the compiler executable to be used.
/// Used when in `integratedDriver` mode as a substitute for the driver knowing its executable path.
/// - Parameter externalTargetModulePathMap: A dictionary of external targets that are a part of
/// the same build, mapping to filesystem paths of their module files.
/// - Parameter externalTargetModulePathMap: DEPRECATED: A dictionary of external targets
/// that are a part of the same build, mapping to filesystem paths of their module files.
/// - Parameter externalTargetModuleDetailsMap: A dictionary of external targets that are a part of
/// the same build, mapping to a details value which includes a filesystem path of their
/// `.swiftmodule` and a flag indicating whether the external target is a framework.
/// - Parameter interModuleDependencyOracle: An oracle for querying inter-module dependencies,
/// shared across different module builds by a build system.
public init(
Expand All @@ -412,7 +418,9 @@ public struct Driver {
executor: DriverExecutor,
integratedDriver: Bool = true,
compilerExecutableDir: AbsolutePath? = nil,
// Deprecated in favour of the below `externalTargetModuleDetailsMap`
externalTargetModulePathMap: ExternalTargetModulePathMap? = nil,
externalTargetModuleDetailsMap: ExternalTargetModuleDetailsMap? = nil,
interModuleDependencyOracle: InterModuleDependencyOracle? = nil
) throws {
self.env = env
Expand All @@ -422,8 +430,15 @@ public struct Driver {
self.diagnosticEngine = diagnosticsEngine
self.executor = executor

if externalTargetModulePathMap != nil && externalTargetModuleDetailsMap != nil {
throw Error.externalTargetDetailsAPIError
}
if let externalTargetPaths = externalTargetModulePathMap {
self.externalTargetModulePathMap = externalTargetPaths
self.externalTargetModuleDetailsMap = externalTargetPaths.mapValues {
ExternalTargetModuleDetails(path: $0, isFramework: false)
}
} else if let externalTargetDetails = externalTargetModuleDetailsMap {
self.externalTargetModuleDetailsMap = externalTargetDetails
}

if case .subcommand = try Self.invocationRunMode(forArgs: args).mode {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,22 @@ import TSCUtility
import Foundation

/// A map from a module identifier to a path to its .swiftmodule file.
/// Deprecated in favour of the below `ExternalTargetModuleDetails`
public typealias ExternalTargetModulePathMap = [ModuleDependencyId: AbsolutePath]

/// Details about an external target, including the path to its .swiftmodule file
/// and whether it is a framework.
public struct ExternalTargetModuleDetails {
@_spi(Testing) public init(path: AbsolutePath, isFramework: Bool) {
self.path = path
self.isFramework = isFramework
}
let path: AbsolutePath
let isFramework: Bool
}

public typealias ExternalTargetModuleDetailsMap = [ModuleDependencyId: ExternalTargetModuleDetails]

/// In Explicit Module Build mode, this planner is responsible for generating and providing
/// build jobs for all module dependencies and providing compile command options
/// that specify said explicit module dependencies.
Expand Down Expand Up @@ -317,16 +331,17 @@ public typealias ExternalTargetModulePathMap = [ModuleDependencyId: AbsolutePath
modulePath: TextualVirtualPath(path: clangModulePath),
moduleMapPath: dependencyClangModuleDetails.moduleMapPath))
case .swiftPrebuiltExternal:
let compiledModulePath = try dependencyGraph
.swiftPrebuiltDetails(of: dependencyId)
.compiledModulePath
let prebuiltModuleDetails = try dependencyGraph.swiftPrebuiltDetails(of: dependencyId)
let compiledModulePath = prebuiltModuleDetails.compiledModulePath
let isFramework = prebuiltModuleDetails.isFramework
let swiftModulePath: TypedVirtualPath =
.init(file: compiledModulePath.path, type: .swiftModule)
// Accumulate the requried information about this dependency
// TODO: add .swiftdoc and .swiftsourceinfo for this module.
swiftDependencyArtifacts.append(
SwiftModuleArtifactInfo(name: dependencyId.moduleName,
modulePath: TextualVirtualPath(path: swiftModulePath.fileHandle)))
modulePath: TextualVirtualPath(path: swiftModulePath.fileHandle),
isFramework: isFramework))
case .swiftPlaceholder:
fatalError("Unresolved placeholder dependencies at planning stage: \(dependencyId) of \(moduleId)")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import TSCBasic
/// For targets that are built alongside the driver's current module, the scanning action will report them as
/// textual targets to be built from source. Because we can rely on these targets to have been built prior
/// to the driver's current target, we resolve such external targets as prebuilt binary modules, in the graph.
mutating func resolveExternalDependencies(for externalTargetModulePathMap: ExternalTargetModulePathMap)
mutating func resolveExternalDependencies(for externalTargetModuleDetailsMap: ExternalTargetModuleDetailsMap)
throws {
for (externalModuleId, externalModulePath) in externalTargetModulePathMap {
for (externalModuleId, externalModuleDetails) in externalTargetModuleDetailsMap {
let externalModulePath = externalModuleDetails.path
// Replace the occurence of a Swift module to-be-built from source-file
// to an info that describes a pre-built binary module.
let swiftModuleId: ModuleDependencyId = .swift(externalModuleId.moduleName)
Expand All @@ -32,12 +33,12 @@ import TSCBasic
let newModuleId: ModuleDependencyId = .swiftPrebuiltExternal(externalModuleId.moduleName)
let newExternalModuleDetails =
try SwiftPrebuiltExternalModuleDetails(compiledModulePath:
TextualVirtualPath(path: VirtualPath.absolute(externalModulePath).intern()))
TextualVirtualPath(path: VirtualPath.absolute(externalModulePath).intern()),
isFramework: externalModuleDetails.isFramework)
let newInfo = ModuleInfo(modulePath: TextualVirtualPath(path: VirtualPath.absolute(externalModulePath).intern()),
sourceFiles: [],
directDependencies: currentInfo.directDependencies,
details: .swiftPrebuiltExternal(newExternalModuleDetails))

Self.replaceModule(originalId: swiftModuleId, replacementId: newModuleId,
replacementInfo: newInfo, in: &modules)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,17 @@ public struct SwiftPrebuiltExternalModuleDetails: Codable {
/// The path to the .swiftSourceInfo file.
public var moduleSourceInfoPath: TextualVirtualPath?

/// A flag to indicate whether or not this module is a framework.
public var isFramework: Bool

public init(compiledModulePath: TextualVirtualPath,
moduleDocPath: TextualVirtualPath? = nil,
moduleSourceInfoPath: TextualVirtualPath? = nil) throws {
moduleSourceInfoPath: TextualVirtualPath? = nil,
isFramework: Bool = false) throws {
self.compiledModulePath = compiledModulePath
self.moduleDocPath = moduleDocPath
self.moduleSourceInfoPath = moduleSourceInfoPath
self.isFramework = isFramework
}
}

Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftDriver/Jobs/Planning.swift
Original file line number Diff line number Diff line change
Expand Up @@ -532,9 +532,9 @@ extension Driver {
throws -> InterModuleDependencyGraph {
var dependencyGraph = try performDependencyScan()

if let externalTargetPaths = externalTargetModulePathMap {
if let externalTargetDetails = externalTargetModuleDetailsMap {
// Resolve external dependencies in the dependency graph, if any.
try dependencyGraph.resolveExternalDependencies(for: externalTargetPaths)
try dependencyGraph.resolveExternalDependencies(for: externalTargetDetails)
}

// Re-scan Clang modules at all the targets they will be built against.
Expand Down
46 changes: 46 additions & 0 deletions Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,52 @@ final class ExplicitModuleBuildTests: XCTestCase {
}
}

func testModuleDependencyBuildCommandGenerationWithExternalFramework() throws {
do {
let externalDetails: ExternalTargetModuleDetailsMap =
[.swiftPrebuiltExternal("A"): ExternalTargetModuleDetails(path: AbsolutePath("/tmp/A.swiftmodule"),
isFramework: true)]
var driver = try Driver(args: ["swiftc", "-experimental-explicit-module-build",
"-module-name", "testModuleDependencyBuildCommandGenerationWithExternalFramework",
"test.swift"])
var moduleDependencyGraph =
try JSONDecoder().decode(
InterModuleDependencyGraph.self,
from: ModuleDependenciesInputs.simpleDependencyGraphInput.data(using: .utf8)!)
// Key part of this test, using the external info to generate dependency pre-build jobs
try moduleDependencyGraph.resolveExternalDependencies(for: externalDetails)
driver.explicitDependencyBuildPlanner =
try ExplicitDependencyBuildPlanner(dependencyGraph: moduleDependencyGraph,
toolchain: driver.toolchain)
let modulePrebuildJobs =
try driver.explicitDependencyBuildPlanner!.generateExplicitModuleDependenciesBuildJobs()

XCTAssertEqual(modulePrebuildJobs.count, 1)
let job = modulePrebuildJobs.first!
// Load the dependency JSON and verify this dependency was encoded correctly
let explicitDepsFlag =
SwiftDriver.Job.ArgTemplate.flag(String("-explicit-swift-module-map-file"))
XCTAssert(job.commandLine.contains(explicitDepsFlag))
let jsonDepsPathIndex = job.commandLine.firstIndex(of: explicitDepsFlag)
let jsonDepsPathArg = job.commandLine[jsonDepsPathIndex! + 1]
guard case .path(let jsonDepsPath) = jsonDepsPathArg else {
XCTFail("No JSON dependency file path found.")
return
}
guard case let .temporaryWithKnownContents(_, contents) = jsonDepsPath else {
XCTFail("Unexpected path type")
return
}
let dependencyInfoList = try JSONDecoder().decode(Array<SwiftModuleArtifactInfo>.self,
from: contents)
XCTAssertEqual(dependencyInfoList.count, 1)
let dependencyArtifacts =
dependencyInfoList.first(where:{ $0.moduleName == "A" })!
// Ensure this is a framework, as specified by the externalDetails above.
XCTAssertEqual(dependencyArtifacts.isFramework, true)
}
}

private func pathMatchesSwiftModule(path: VirtualPath, _ name: String) -> Bool {
return path.basenameWithoutExt.starts(with: "\(name)-") &&
path.extension! == FileType.swiftModule.rawValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,82 @@ enum ModuleDependenciesInputs {
"""
}

static var simpleDependencyGraphInput: String {
"""
{
"mainModuleName": "main",
"modules": [
{
"swift": "main"
},
{
"modulePath": "main.swiftmodule",
"sourceFiles": [
"/main/main.swift"
],
"directDependencies": [
{
"swift": "B"
},
],
"details": {
"swift": {
"isFramework": false,
"extraPcmArgs": [
"-Xcc",
"-fapinotes-swift-version=5"
]
}
}
},
{
"swift" : "B"
},
{
"modulePath" : "B.swiftmodule",
"sourceFiles": [
],
"directDependencies" : [
{
"swift": "A"
},
],
"details" : {
"swift" : {
"moduleInterfacePath": "B.swiftmodule/B.swiftinterface",
"isFramework": false,
"extraPcmArgs": [
"-Xcc",
"-fapinotes-swift-version=5"
],
}
}
},
{
"swift": "A"
},
{
"modulePath": "/tmp/A.swiftmodule",
"sourceFiles": [
"/A/A.swift"
],
"directDependencies" : [
],
"details": {
"swift": {
"isFramework": false,
"extraPcmArgs": [
"-Xcc",
"-fapinotes-swift-version=5"
]
}
}
}
]
}
"""
}

static var mergeGraphInput2: String {
"""
{
Expand Down