Skip to content
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

Keep separate build parameters for host and target #7164

Merged
merged 13 commits into from
Dec 7, 2023
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Fix tests, restore SourceKit-LSP compatibility
MaxDesiatov committed Dec 6, 2023

Verified

This commit was signed with the committer’s verified signature. The key has expired.
MaxDesiatov Max Desiatov
commit d020e28bff7d12f5cd545745b7ffa7bf4a334247
22 changes: 1 addition & 21 deletions Sources/Build/BuildPlan/BuildPlan.swift
Original file line number Diff line number Diff line change
@@ -187,26 +187,6 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
/// Build parameters used for tools.
public let toolsBuildParameters: BuildParameters

/// Parameters used for building this target.
func buildParameters(for target: ResolvedTarget) -> BuildParameters {
switch target.buildTriple {
case .buildTools:
return self.toolsBuildParameters
case .buildProducts:
return self.productsBuildParameters
}
}

/// Parameters used for building this product.
func buildParameters(for product: ResolvedProduct) -> BuildParameters {
switch product.buildTriple {
case .buildTools:
return self.toolsBuildParameters
case .buildProducts:
return self.productsBuildParameters
}
}

/// Triple for which this target is compiled.
private func buildTriple(for target: ResolvedTarget) -> Basics.Triple {
self.buildParameters(for: target).triple
@@ -272,7 +252,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {

/// Create a build plan with a package graph and same build parameters for products and tools. Provided for
/// testing purposes only.
convenience init(
public convenience init(
buildParameters: BuildParameters,
graph: PackageGraph,
additionalFileRules: [FileRuleDescription] = [],
5 changes: 3 additions & 2 deletions Sources/Commands/PackageTools/APIDiff.swift
Original file line number Diff line number Diff line change
@@ -98,7 +98,8 @@ struct APIDiff: SwiftCommand {
let baselineDumper = try APIDigesterBaselineDumper(
baselineRevision: baselineRevision,
packageRoot: swiftTool.getPackageRoot(),
buildParameters: try buildSystem.buildPlan.buildParameters,
productsBuildParameters: try buildSystem.buildPlan.productsBuildParameters,
toolsBuildParameters: try buildSystem.buildPlan.toolsBuildParameters,
apiDigesterTool: apiDigesterTool,
observabilityScope: swiftTool.observabilityScope
)
@@ -113,7 +114,7 @@ struct APIDiff: SwiftCommand {

let results = ThreadSafeArrayStore<SwiftAPIDigester.ComparisonResult>()
let group = DispatchGroup()
let semaphore = DispatchSemaphore(value: Int(try buildSystem.buildPlan.buildParameters.workers))
let semaphore = DispatchSemaphore(value: Int(try buildSystem.buildPlan.productsBuildParameters.workers))
var skippedModules: Set<String> = []

for module in modulesToDiff {
2 changes: 1 addition & 1 deletion Sources/Commands/PackageTools/DumpCommands.swift
Original file line number Diff line number Diff line change
@@ -65,7 +65,7 @@ struct DumpSymbolGraph: SwiftCommand {

// Run the tool once for every library and executable target in the root package.
let buildPlan = try buildSystem.buildPlan
let symbolGraphDirectory = buildPlan.buildParameters.dataPath.appending("symbolgraph")
let symbolGraphDirectory = buildPlan.productsBuildParameters.dataPath.appending("symbolgraph")
let targets = try buildSystem.getPackageGraph().rootPackages.flatMap{ $0.targets }.filter{ $0.type == .library }
for target in targets {
print("-- Emitting symbol graph for", target.name)
3 changes: 1 addition & 2 deletions Sources/Commands/PackageTools/PluginCommand.swift
Original file line number Diff line number Diff line change
@@ -259,8 +259,7 @@ struct PluginCommand: SwiftCommand {
// Build or bring up-to-date any executable host-side tools on which this plugin depends. Add them and any binary dependencies to the tool-names-to-path map.
let buildSystem = try swiftTool.createBuildSystem(
explicitBuildSystem: .native,
cacheBuildManifest: false,
customBuildParameters: buildParameters
cacheBuildManifest: false
)
let accessibleTools = try plugin.processAccessibleTools(
packageGraph: packageGraph,
2 changes: 1 addition & 1 deletion Sources/Commands/Snippets/Cards/SnippetCard.swift
Original file line number Diff line number Diff line change
@@ -95,7 +95,7 @@ struct SnippetCard: Card {
print("Building '\(snippet.path)'\n")
let buildSystem = try swiftTool.createBuildSystem(explicitProduct: snippet.name)
try buildSystem.build(subset: .product(snippet.name))
let executablePath = try swiftTool.buildParameters().buildPath.appending(component: snippet.name)
let executablePath = try swiftTool.productsBuildParameters.buildPath.appending(component: snippet.name)
if let exampleTarget = try buildSystem.getPackageGraph().allTargets.first(where: { $0.name == snippet.name }) {
try ProcessEnv.chdir(exampleTarget.sources.paths[0].parentDirectory)
}
2 changes: 1 addition & 1 deletion Sources/Commands/SwiftBuildTool.swift
Original file line number Diff line number Diff line change
@@ -134,7 +134,7 @@ public struct SwiftBuildTool: SwiftCommand {
shouldLinkStaticSwiftStdlib: options.shouldLinkStaticSwiftStdlib,
// command result output goes on stdout
// ie "swift build" should output to stdout
customOutputStream: TSCBasic.stdoutStream
outputStream: TSCBasic.stdoutStream
)
do {
try buildSystem.build(subset: subset)
8 changes: 3 additions & 5 deletions Sources/Commands/SwiftRunTool.swift
Original file line number Diff line number Diff line change
@@ -125,15 +125,13 @@ public struct SwiftRunTool: SwiftCommand {
explicitProduct: self.options.executable
)
}
let buildParameters = try swiftTool.buildParameters()

// Construct the build operation.
// FIXME: We need to implement the build tool invocation closure here so that build tool plugins work with the REPL. rdar://86112934
let buildSystem = try swiftTool.createBuildSystem(
explicitBuildSystem: .native,
cacheBuildManifest: false,
customBuildParameters: buildParameters,
customPackageGraphLoader: graphLoader
packageGraphLoader: graphLoader
)

// Perform build.
@@ -159,7 +157,7 @@ public struct SwiftRunTool: SwiftCommand {
try buildSystem.build(subset: .product(productName))
}

let executablePath = try swiftTool.buildParameters().buildPath.appending(component: productName)
let executablePath = try swiftTool.productsBuildParameters.buildPath.appending(component: productName)

// Make sure we are running from the original working directory.
let cwd: AbsolutePath? = swiftTool.fileSystem.currentWorkingDirectory
@@ -201,7 +199,7 @@ public struct SwiftRunTool: SwiftCommand {
try buildSystem.build(subset: .product(productName))
}

let executablePath = try swiftTool.buildParameters().buildPath.appending(component: productName)
let executablePath = try swiftTool.productsBuildParameters.buildPath.appending(component: productName)
try self.run(
fileSystem: swiftTool.fileSystem,
executablePath: executablePath,
11 changes: 7 additions & 4 deletions Sources/Commands/SwiftTestTool.swift
Original file line number Diff line number Diff line change
@@ -217,7 +217,7 @@ public struct SwiftTestTool: SwiftCommand {
let buildParameters = try swiftTool.buildParametersForTest(options: self.options, library: .xctest)

// Remove test output from prior runs and validate priors.
if self.options.enableExperimentalTestOutput && buildParameters.targetTriple.supportsTestSummary {
if self.options.enableExperimentalTestOutput && buildParameters.triple.supportsTestSummary {
_ = try? localFileSystem.removeFileTree(buildParameters.testOutputPath)
}

@@ -573,7 +573,7 @@ extension SwiftTestTool {

func run(_ swiftTool: SwiftTool) throws {
try SwiftTestTool.handleTestOutput(
buildParameters: try swiftTool.buildParameters(),
buildParameters: try swiftTool.productsBuildParameters,
packagePath: localFileSystem.currentWorkingDirectory ?? .root // by definition runs in the current working directory
)
}
@@ -1201,7 +1201,10 @@ final class XUnitGenerator {
}

extension SwiftTool {
func buildParametersForTest(options: TestToolOptions, library: BuildParameters.Testing.Library) throws -> BuildParameters {
func buildParametersForTest(
options: TestToolOptions,
library: BuildParameters.Testing.Library
) throws -> BuildParameters {
var result = try self.buildParametersForTest(
enableCodeCoverage: options.enableCodeCoverage,
enableTestability: options.enableTestableImports,
@@ -1269,7 +1272,7 @@ private extension Basics.Diagnostic {
///
/// - Returns: The paths to the build test products.
private func buildTestsIfNeeded(swiftTool: SwiftTool, buildParameters: BuildParameters, testProduct: String?) throws -> [BuiltTestProduct] {
let buildSystem = try swiftTool.createBuildSystem(customBuildParameters: buildParameters)
let buildSystem = try swiftTool.createBuildSystem(productsBuildParameters: buildParameters)

let subset = testProduct.map(BuildSubset.product) ?? .allIncludingTests
try buildSystem.build(subset: subset)
26 changes: 16 additions & 10 deletions Sources/Commands/Utilities/APIDigester.swift
Original file line number Diff line number Diff line change
@@ -39,8 +39,11 @@ struct APIDigesterBaselineDumper {
/// The root package path.
let packageRoot: AbsolutePath

/// The input build parameters.
let inputBuildParameters: BuildParameters
/// Parameters used when building end products.
let productsBuildParameters: BuildParameters

/// Parameters used when building tools (plugins and macros).
let toolsBuildParameters: BuildParameters

/// The API digester tool.
let apiDigesterTool: SwiftAPIDigester
@@ -51,13 +54,15 @@ struct APIDigesterBaselineDumper {
init(
baselineRevision: Revision,
packageRoot: AbsolutePath,
buildParameters: BuildParameters,
productsBuildParameters: BuildParameters,
toolsBuildParameters: BuildParameters,
apiDigesterTool: SwiftAPIDigester,
observabilityScope: ObservabilityScope
) {
self.baselineRevision = baselineRevision
self.packageRoot = packageRoot
self.inputBuildParameters = buildParameters
self.productsBuildParameters = productsBuildParameters
self.toolsBuildParameters = toolsBuildParameters
self.apiDigesterTool = apiDigesterTool
self.observabilityScope = observabilityScope
}
@@ -71,7 +76,7 @@ struct APIDigesterBaselineDumper {
swiftTool: SwiftTool
) throws -> AbsolutePath {
var modulesToDiff = modulesToDiff
let apiDiffDir = inputBuildParameters.apiDiff
let apiDiffDir = productsBuildParameters.apiDiff
let baselineDir = (baselineDir ?? apiDiffDir).appending(component: baselineRevision.identifier)
let baselinePath: (String)->AbsolutePath = { module in
baselineDir.appending(component: module + ".json")
@@ -127,23 +132,24 @@ struct APIDigesterBaselineDumper {
}

// Update the data path input build parameters so it's built in the sandbox.
var buildParameters = inputBuildParameters
buildParameters.dataPath = workspace.location.scratchDirectory
var productsBuildParameters = productsBuildParameters
productsBuildParameters.dataPath = workspace.location.scratchDirectory

// 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 swiftTool.createBuildSystem(
explicitBuildSystem: .native,
cacheBuildManifest: false,
customBuildParameters: buildParameters,
customPackageGraphLoader: { graph }
productsBuildParameters: productsBuildParameters,
toolsBuildParameters: toolsBuildParameters,
packageGraphLoader: { graph }
)
try buildSystem.build()

// Dump the SDK JSON.
try swiftTool.fileSystem.createDirectory(baselineDir, recursive: true)
let group = DispatchGroup()
let semaphore = DispatchSemaphore(value: Int(buildParameters.workers))
let semaphore = DispatchSemaphore(value: Int(productsBuildParameters.workers))
let errors = ThreadSafeArrayStore<Swift.Error>()
for module in modulesToDiff {
semaphore.wait()
34 changes: 17 additions & 17 deletions Sources/Commands/Utilities/PluginDelegate.swift
Original file line number Diff line number Diff line change
@@ -71,7 +71,7 @@ final class PluginDelegate: PluginInvocationDelegate {
parameters: PluginInvocationBuildParameters
) throws -> PluginInvocationBuildResult {
// Configure the build parameters.
var buildParameters = try self.swiftTool.toolsBuildParameters
var buildParameters = try self.swiftTool.productsBuildParameters
switch parameters.configuration {
case .debug:
buildParameters.configuration = .debug
@@ -113,9 +113,9 @@ final class PluginDelegate: PluginInvocationDelegate {
explicitBuildSystem: .native,
explicitProduct: explicitProduct,
cacheBuildManifest: false,
customBuildParameters: buildParameters,
customOutputStream: outputStream,
customLogLevel: logLevel
productsBuildParameters: buildParameters,
outputStream: outputStream,
logLevel: logLevel
)

// Run the build. This doesn't return until the build is complete.
@@ -170,23 +170,23 @@ final class PluginDelegate: PluginInvocationDelegate {
) throws -> PluginInvocationTestResult {
// Build the tests. Ideally we should only build those that match the subset, but we don't have a way to know
// which ones they are until we've built them and can examine the binaries.
let toolchain = try swiftTool.getTargetToolchain()
var buildParameters = try swiftTool.toolsBuildParameters
buildParameters.testingParameters.enableTestability = true
buildParameters.testingParameters.enableCodeCoverage = parameters.enableCodeCoverage
let buildSystem = try swiftTool.createBuildSystem(customBuildParameters: buildParameters)
let toolchain = try swiftTool.getHostToolchain()
var toolsBuildParameters = try swiftTool.toolsBuildParameters
toolsBuildParameters.testingParameters.enableTestability = true
toolsBuildParameters.testingParameters.enableCodeCoverage = parameters.enableCodeCoverage
let buildSystem = try swiftTool.createBuildSystem(toolsBuildParameters: toolsBuildParameters)
try buildSystem.build(subset: .allIncludingTests)

// Clean out the code coverage directory that may contain stale `profraw` files from a previous run of
// the code coverage tool.
if parameters.enableCodeCoverage {
try swiftTool.fileSystem.removeFileTree(buildParameters.codeCovPath)
try swiftTool.fileSystem.removeFileTree(toolsBuildParameters.codeCovPath)
}

// Construct the environment we'll pass down to the tests.
let testEnvironment = try TestingSupport.constructTestEnvironment(
toolchain: toolchain,
buildParameters: buildParameters,
buildParameters: toolsBuildParameters,
sanitizers: swiftTool.options.build.sanitizers
)

@@ -271,12 +271,12 @@ final class PluginDelegate: PluginInvocationDelegate {
let codeCoverageDataFile: AbsolutePath?
if parameters.enableCodeCoverage {
// Use `llvm-prof` to merge all the `.profraw` files into a single `.profdata` file.
let mergedCovFile = buildParameters.codeCovDataFile
let codeCovFileNames = try swiftTool.fileSystem.getDirectoryContents(buildParameters.codeCovPath)
let mergedCovFile = toolsBuildParameters.codeCovDataFile
let codeCovFileNames = try swiftTool.fileSystem.getDirectoryContents(toolsBuildParameters.codeCovPath)
var llvmProfCommand = [try toolchain.getLLVMProf().pathString]
llvmProfCommand += ["merge", "-sparse"]
for fileName in codeCovFileNames where fileName.hasSuffix(".profraw") {
let filePath = buildParameters.codeCovPath.appending(component: fileName)
let filePath = toolsBuildParameters.codeCovPath.appending(component: fileName)
llvmProfCommand.append(filePath.pathString)
}
llvmProfCommand += ["-o", mergedCovFile.pathString]
@@ -291,8 +291,8 @@ final class PluginDelegate: PluginInvocationDelegate {
}
// We get the output on stdout, and have to write it to a JSON ourselves.
let jsonOutput = try TSCBasic.Process.checkNonZeroExit(arguments: llvmCovCommand)
let jsonCovFile = buildParameters.codeCovDataFile.parentDirectory.appending(
component: buildParameters.codeCovDataFile.basenameWithoutExt + ".json"
let jsonCovFile = toolsBuildParameters.codeCovDataFile.parentDirectory.appending(
component: toolsBuildParameters.codeCovDataFile.basenameWithoutExt + ".json"
)
try swiftTool.fileSystem.writeFileContents(jsonCovFile, string: jsonOutput)

@@ -369,7 +369,7 @@ final class PluginDelegate: PluginInvocationDelegate {
guard let package = packageGraph.package(for: target) else {
throw StringError("could not determine the package for target “\(target.name)")
}
let outputDir = try buildSystem.buildPlan.buildParameters.dataPath.appending(
let outputDir = try buildSystem.buildPlan.toolsBuildParameters.dataPath.appending(
components: "extracted-symbols",
package.identity.description,
target.name
6 changes: 4 additions & 2 deletions Sources/Commands/Utilities/SymbolGraphExtract.swift
Original file line number Diff line number Diff line change
@@ -50,15 +50,17 @@ public struct SymbolGraphExtract {
case json(pretty: Bool)
}

/// Creates a symbol graph for `target` in `outputDirectory` using the build information from `buildPlan`. The `outputDirection` determines how the output from the tool subprocess is handled, and `verbosity` specifies how much console output to ask the tool to emit.
/// Creates a symbol graph for `target` in `outputDirectory` using the build information from `buildPlan`.
/// The `outputDirection` determines how the output from the tool subprocess is handled, and `verbosity` specifies
/// how much console output to ask the tool to emit.
public func extractSymbolGraph(
target: ResolvedTarget,
buildPlan: BuildPlan,
outputRedirection: TSCBasic.Process.OutputRedirection = .none,
outputDirectory: AbsolutePath,
verboseOutput: Bool
) throws -> ProcessResult {
let buildParameters = buildPlan.buildParameters
let buildParameters = buildPlan.buildParameters(for: target)
try self.fileSystem.createDirectory(outputDirectory, recursive: true)

// Construct arguments for extracting symbols for a single target.
38 changes: 24 additions & 14 deletions Sources/CoreCommands/SwiftTool.swift
Original file line number Diff line number Diff line change
@@ -642,28 +642,30 @@ public final class SwiftTool {
explicitProduct: String? = .none,
cacheBuildManifest: Bool = true,
shouldLinkStaticSwiftStdlib: Bool = false,
customBuildParameters: BuildParameters? = .none,
customPackageGraphLoader: (() throws -> PackageGraph)? = .none,
customOutputStream: OutputByteStream? = .none,
customLogLevel: Basics.Diagnostic.Severity? = .none,
customObservabilityScope: ObservabilityScope? = .none
productsBuildParameters: BuildParameters? = .none,
toolsBuildParameters: BuildParameters? = .none,
packageGraphLoader: (() throws -> PackageGraph)? = .none,
outputStream: OutputByteStream? = .none,
logLevel: Basics.Diagnostic.Severity? = .none,
observabilityScope: ObservabilityScope? = .none
) throws -> BuildSystem {
guard let buildSystemProvider else {
fatalError("build system provider not initialized")
}

var buildParameters = try customBuildParameters ?? self.buildParameters()
buildParameters.linkingParameters.shouldLinkStaticSwiftStdlib = shouldLinkStaticSwiftStdlib
var productsParameters = try productsBuildParameters ?? self.productsBuildParameters
productsParameters.linkingParameters.shouldLinkStaticSwiftStdlib = shouldLinkStaticSwiftStdlib

let buildSystem = try buildSystemProvider.createBuildSystem(
kind: explicitBuildSystem ?? options.build.buildSystem,
explicitProduct: explicitProduct,
cacheBuildManifest: cacheBuildManifest,
productsBuildParameters: customBuildParameters,
packageGraphLoader: customPackageGraphLoader,
outputStream: customOutputStream,
logLevel: customLogLevel,
observabilityScope: customObservabilityScope
productsBuildParameters: productsParameters,
toolsBuildParameters: toolsBuildParameters,
packageGraphLoader: packageGraphLoader,
outputStream: outputStream,
logLevel: logLevel,
observabilityScope: observabilityScope
)

// register the build system with the cancellation handler
@@ -867,11 +869,19 @@ public final class SwiftTool {

var extraManifestFlags = self.options.build.manifestFlags
// Disable the implicit concurrency import if the compiler in use supports it to avoid warnings if we are building against an older SDK that does not contain a Concurrency module.
if DriverSupport.checkSupportedFrontendFlags(flags: ["disable-implicit-concurrency-module-import"], toolchain: try self.buildParameters().toolchain, fileSystem: self.fileSystem) {
if DriverSupport.checkSupportedFrontendFlags(
flags: ["disable-implicit-concurrency-module-import"],
toolchain: try self.toolsBuildParameters.toolchain,
fileSystem: self.fileSystem
) {
extraManifestFlags += ["-Xfrontend", "-disable-implicit-concurrency-module-import"]
}
// Disable the implicit string processing import if the compiler in use supports it to avoid warnings if we are building against an older SDK that does not contain a StringProcessing module.
if DriverSupport.checkSupportedFrontendFlags(flags: ["disable-implicit-string-processing-module-import"], toolchain: try self.buildParameters().toolchain, fileSystem: self.fileSystem) {
if DriverSupport.checkSupportedFrontendFlags(
flags: ["disable-implicit-string-processing-module-import"],
toolchain: try self.toolsBuildParameters.toolchain,
fileSystem: self.fileSystem
) {
extraManifestFlags += ["-Xfrontend", "-disable-implicit-string-processing-module-import"]
}

8 changes: 0 additions & 8 deletions Sources/SPMBuildCore/BuildParameters/BuildParameters.swift
Original file line number Diff line number Diff line change
@@ -36,14 +36,6 @@ public struct BuildParameters: Encodable {
public var toolchain: Toolchain { _toolchain.toolchain }
private let _toolchain: _Toolchain

/// Host triple.
@available(*, deprecated, message: "use separate `BuildParameters` values to distinguish between host and target")
public var hostTriple: Triple { self.triple }

/// Target triple.
@available(*, deprecated, message: "use separate `BuildParameters` values to distinguish between host and target")
public var targetTriple: Triple { self.triple }

/// The triple for which the code is built using these build parameters.
public var triple: Triple

31 changes: 29 additions & 2 deletions Sources/SPMBuildCore/BuildSystem/BuildSystem.swift
Original file line number Diff line number Diff line change
@@ -74,19 +74,46 @@ extension ProductBuildDescription {
/// The path to the product binary produced.
public var binaryPath: AbsolutePath {
get throws {
return try buildParameters.binaryPath(for: product)
return try self.buildParameters.binaryPath(for: product)
}
}
}

public protocol BuildPlan {
var buildParameters: BuildParameters { get }
/// Parameters used when building end products.
var productsBuildParameters: BuildParameters { get }

/// Parameters used when building tools (macros and plugins).
var toolsBuildParameters: BuildParameters { get }

var buildProducts: AnySequence<ProductBuildDescription> { get }

func createAPIToolCommonArgs(includeLibrarySearchPaths: Bool) throws -> [String]
func createREPLArguments() throws -> [String]
}

extension BuildPlan {
/// Parameters used for building this target.
public func buildParameters(for target: ResolvedTarget) -> BuildParameters {
switch target.buildTriple {
case .buildTools:
return self.toolsBuildParameters
case .buildProducts:
return self.productsBuildParameters
}
}

/// Parameters used for building this product.
public func buildParameters(for product: ResolvedProduct) -> BuildParameters {
switch product.buildTriple {
case .buildTools:
return self.toolsBuildParameters
case .buildProducts:
return self.productsBuildParameters
}
}
}

public protocol BuildSystemFactory {
func makeBuildSystem(
explicitProduct: String?,
521 changes: 415 additions & 106 deletions Tests/BuildTests/BuildPlanTests.swift

Large diffs are not rendered by default.