Skip to content

[5.9] Improve handling of tool build flags #6709

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 3 commits into from
Jul 19, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,14 @@ public final class ClangTargetBuildDescription {
args += ["-include", resourceAccessorHeaderFile.pathString]
}

args += buildParameters.toolchain.extraFlags.cCompilerFlags
// User arguments (from -Xcc and -Xcxx below) should follow generated arguments to allow user overrides
args += buildParameters.flags.cCompilerFlags
args += self.buildParameters.toolchain.extraFlags.cCompilerFlags
// User arguments (from -Xcc) should follow generated arguments to allow user overrides
args += self.buildParameters.flags.cCompilerFlags

// Add extra C++ flags if this target contains C++ files.
if clangTarget.isCXX {
if isCXX {
args += self.buildParameters.toolchain.extraFlags.cxxCompilerFlags
// User arguments (from -Xcxx) should follow generated arguments to allow user overrides
args += self.buildParameters.flags.cxxCompilerFlags
}
return args
Expand Down
16 changes: 12 additions & 4 deletions Sources/Build/BuildDescription/ProductBuildDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
args += self.buildParameters.sanitizers.linkSwiftFlags()
args += self.additionalFlags

// pass `-v` during verbose builds.
if self.buildParameters.verboseOutput {
args += ["-v"]
}

// Pass `-g` during a *release* build so the Swift driver emits a dSYM file for the binary.
if self.buildParameters.configuration == .release {
if self.buildParameters.triple.isWindows() {
Expand Down Expand Up @@ -304,9 +309,12 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
args += self.swiftASTs.flatMap { ["-Xlinker", "-add_ast_path", "-Xlinker", $0.pathString] }

args += self.buildParameters.toolchain.extraFlags.swiftCompilerFlags
// User arguments (from -Xlinker and -Xswiftc) should follow generated arguments to allow user overrides
args += self.buildParameters.linkerFlags
args += self.stripInvalidArguments(self.buildParameters.swiftCompilerFlags)
// User arguments (from -Xswiftc) should follow generated arguments to allow user overrides
args += self.buildParameters.flags.swiftCompilerFlags

args += self.buildParameters.toolchain.extraFlags.linkerFlags.asSwiftcLinkerFlags()
// User arguments (from -Xlinker) should follow generated arguments to allow user overrides
args += self.buildParameters.flags.linkerFlags.asSwiftcLinkerFlags()

// Add toolchain's libdir at the very end (even after the user -Xlinker arguments).
//
Expand All @@ -323,7 +331,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
}
#endif

return args
return self.stripInvalidArguments(args)
}

/// Writes link filelist to the filesystem.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,11 @@ public final class SwiftTargetBuildDescription {
args += try self.buildParameters.targetTripleArgs(for: self.target)
args += ["-swift-version", self.swiftVersion.rawValue]

// pass `-v` during verbose builds.
if self.buildParameters.verboseOutput {
args += ["-v"]
}

// Enable batch mode in debug mode.
//
// Technically, it should be enabled whenever WMO is off but we
Expand Down Expand Up @@ -496,7 +501,17 @@ public final class SwiftTargetBuildDescription {

args += self.buildParameters.toolchain.extraFlags.swiftCompilerFlags
// User arguments (from -Xswiftc) should follow generated arguments to allow user overrides
args += self.buildParameters.swiftCompilerFlags
args += self.buildParameters.flags.swiftCompilerFlags

args += self.buildParameters.toolchain.extraFlags.cCompilerFlags.asSwiftcCCompilerFlags()
// User arguments (from -Xcc) should follow generated arguments to allow user overrides
args += self.buildParameters.flags.cCompilerFlags.asSwiftcCCompilerFlags()

// TODO: Pass -Xcxx flags to swiftc (#6491)
// Uncomment when downstream support arrives.
// args += self.buildParameters.toolchain.extraFlags.cxxCompilerFlags.asSwiftcCXXCompilerFlags()
// // User arguments (from -Xcxx) should follow generated arguments to allow user overrides
// args += self.buildParameters.flags.cxxCompilerFlags.asSwiftcCXXCompilerFlags()

// suppress warnings if the package is remote
if self.package.isRemote {
Expand Down Expand Up @@ -611,6 +626,11 @@ public final class SwiftTargetBuildDescription {
var result: [String] = []
result.append(self.buildParameters.toolchain.swiftCompilerPath.pathString)

// pass `-v` during verbose builds.
if self.buildParameters.verboseOutput {
result += ["-v"]
}

result.append("-module-name")
result.append(self.target.c99name)
result.append(contentsOf: packageNameArgumentIfSupported(with: self.package, packageAccess: self.target.packageAccess))
Expand Down Expand Up @@ -646,8 +666,21 @@ public final class SwiftTargetBuildDescription {
result += self.buildParameters.sanitizers.compileSwiftFlags()
result += ["-parseable-output"]
result += try self.buildSettingsFlags()

result += self.buildParameters.toolchain.extraFlags.swiftCompilerFlags
result += self.buildParameters.swiftCompilerFlags
// User arguments (from -Xswiftc) should follow generated arguments to allow user overrides
result += self.buildParameters.flags.swiftCompilerFlags

result += self.buildParameters.toolchain.extraFlags.cCompilerFlags.asSwiftcCCompilerFlags()
// User arguments (from -Xcc) should follow generated arguments to allow user overrides
result += self.buildParameters.flags.cCompilerFlags.asSwiftcCCompilerFlags()

// TODO: Pass -Xcxx flags to swiftc (#6491)
// Uncomment when downstream support arrives.
// result += self.buildParameters.toolchain.extraFlags.cxxCompilerFlags.asSwiftcCXXCompilerFlags()
// // User arguments (from -Xcxx) should follow generated arguments to allow user overrides
// result += self.buildParameters.flags.cxxCompilerFlags.asSwiftcCXXCompilerFlags()

result += try self.macroArguments()
return result
}
Expand Down
61 changes: 38 additions & 23 deletions Sources/Build/BuildPlan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,31 +42,30 @@ extension AbsolutePath {
}
}

extension BuildParameters {
/// Returns the directory to be used for module cache.
public var moduleCache: AbsolutePath {
get throws {
// FIXME: We use this hack to let swiftpm's functional test use shared
// cache so it doesn't become painfully slow.
if let path = ProcessEnv.vars["SWIFTPM_TESTS_MODULECACHE"] {
return try AbsolutePath(validating: path)
}
return buildPath.appending("ModuleCache")
}
extension Array where Element == String {
/// Converts a set of C compiler flags into an equivalent set to be
/// indirected through the Swift compiler instead.
func asSwiftcCCompilerFlags() -> Self {
self.flatMap { ["-Xcc", $0] }
}

/// Extra flags to pass to Swift compiler.
public var swiftCompilerFlags: [String] {
var flags = self.flags.cCompilerFlags.flatMap({ ["-Xcc", $0] })
flags += self.flags.swiftCompilerFlags
if self.verboseOutput {
flags.append("-v")
}
return flags
/// Converts a set of C++ compiler flags into an equivalent set to be
/// indirected through the Swift compiler instead.
func asSwiftcCXXCompilerFlags() -> Self {
_ = self.flatMap { ["-Xcxx", $0] }
// TODO: Pass -Xcxx flags to swiftc (#6491)
// Remove fatal error when downstream support arrives.
fatalError("swiftc does support -Xcxx flags yet.")
}

/// Extra flags to pass to linker.
public var linkerFlags: [String] {
/// Converts a set of linker flags into an equivalent set to be indirected
/// through the Swift compiler instead.
///
/// Some arguments can be passed directly to the Swift compiler. We omit
/// prefixing these arguments (in both the "-option value" and
/// "-option[=]value" forms) with "-Xlinker". All other arguments are
/// prefixed with "-Xlinker".
func asSwiftcLinkerFlags() -> Self {
// Arguments that can be passed directly to the Swift compiler and
// doesn't require -Xlinker prefix.
//
Expand All @@ -75,10 +74,11 @@ extension BuildParameters {
let directSwiftLinkerArgs = ["-L"]

var flags: [String] = []
var it = self.flags.linkerFlags.makeIterator()
var it = self.makeIterator()
while let flag = it.next() {
if directSwiftLinkerArgs.contains(flag) {
// `-L <value>` variant.
// `<option> <value>` variant.
flags.append(flag)
guard let nextFlag = it.next() else {
// We expected a flag but don't have one.
Expand All @@ -87,13 +87,28 @@ extension BuildParameters {
flags.append(nextFlag)
} else if directSwiftLinkerArgs.contains(where: { flag.hasPrefix($0) }) {
// `-L<value>` variant.
// `<option>[=]<value>` variant.
flags.append(flag)
} else {
flags += ["-Xlinker", flag]
}
}
return flags
}
}

extension BuildParameters {
/// Returns the directory to be used for module cache.
public var moduleCache: AbsolutePath {
get throws {
// FIXME: We use this hack to let swiftpm's functional test use shared
// cache so it doesn't become painfully slow.
if let path = ProcessEnv.vars["SWIFTPM_TESTS_MODULECACHE"] {
return try AbsolutePath(validating: path)
}
return buildPath.appending("ModuleCache")
}
}

/// Returns the compiler arguments for the index store, if enabled.
func indexStoreArguments(for target: ResolvedTarget) -> [String] {
Expand Down Expand Up @@ -147,7 +162,7 @@ extension BuildParameters {
else {
return nil
}
return args.flatMap { ["-Xlinker", $0] }
return args.asSwiftcLinkerFlags()
}

/// Returns the scoped view of build settings for a given target.
Expand Down
4 changes: 1 addition & 3 deletions Sources/CoreCommands/SwiftTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -680,15 +680,13 @@ public final class SwiftTool {
let dataPath = self.scratchDirectory.appending(
component: destinationTriple.platformBuildPathComponent(buildSystem: options.build.buildSystem)
)
var buildFlags = options.build.buildFlags
buildFlags.append(destinationToolchain.extraFlags)

return try BuildParameters(
dataPath: dataPath,
configuration: options.build.configuration,
toolchain: destinationToolchain,
destinationTriple: destinationTriple,
flags: buildFlags,
flags: options.build.buildFlags,
pkgConfigDirectories: options.locations.pkgConfigDirectories,
architectures: options.build.architectures,
workers: options.build.jobs ?? UInt32(ProcessInfo.processInfo.activeProcessorCount),
Expand Down
16 changes: 0 additions & 16 deletions Sources/PackageModel/BuildFlags.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,4 @@ public struct BuildFlags: Equatable, Encodable {
self.linkerFlags = linkerFlags
self.xcbuildFlags = xcbuildFlags
}

/// Appends corresponding properties of a different `BuildFlags` value into `self`.
/// - Parameter buildFlags: a `BuildFlags` value to merge flags from.
public mutating func append(_ buildFlags: BuildFlags) {
cCompilerFlags += buildFlags.cCompilerFlags
cxxCompilerFlags += buildFlags.cxxCompilerFlags
swiftCompilerFlags += buildFlags.swiftCompilerFlags
linkerFlags += buildFlags.linkerFlags

if var xcbuildFlags, let newXcbuildFlags = buildFlags.xcbuildFlags {
xcbuildFlags += newXcbuildFlags
self.xcbuildFlags = xcbuildFlags
} else if let xcbuildFlags = buildFlags.xcbuildFlags {
self.xcbuildFlags = xcbuildFlags
}
}
}
26 changes: 11 additions & 15 deletions Sources/PackageModel/UserToolchain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -481,13 +481,15 @@ public final class UserToolchain: Toolchain {
}

self.triple = triple
self.extraFlags = BuildFlags()

self.extraFlags.swiftCompilerFlags = try Self.deriveSwiftCFlags(
triple: triple,
destination: destination,
environment: environment
)
self.extraFlags = BuildFlags(
cCompilerFlags: destination.toolset.knownTools[.cCompiler]?.extraCLIOptions ?? [],
cxxCompilerFlags: destination.toolset.knownTools[.cxxCompiler]?.extraCLIOptions ?? [],
swiftCompilerFlags: try Self.deriveSwiftCFlags(
triple: triple,
destination: destination,
environment: environment),
linkerFlags: destination.toolset.knownTools[.linker]?.extraCLIOptions ?? [],
xcbuildFlags: destination.toolset.knownTools[.xcbuild]?.extraCLIOptions ?? [])

self.librarianPath = try UserToolchain.determineLibrarian(
triple: triple,
Expand All @@ -499,14 +501,8 @@ public final class UserToolchain: Toolchain {
)

if let sdkDir = destination.pathsConfiguration.sdkRootPath {
self.extraFlags.cCompilerFlags = [
triple.isDarwin() ? "-isysroot" : "--sysroot", sdkDir.pathString,
] + (destination.toolset.knownTools[.cCompiler]?.extraCLIOptions ?? [])

self.extraFlags.cxxCompilerFlags = destination.toolset.knownTools[.cxxCompiler]?.extraCLIOptions ?? []
} else {
self.extraFlags.cCompilerFlags = (destination.toolset.knownTools[.cCompiler]?.extraCLIOptions ?? [])
self.extraFlags.cxxCompilerFlags = (destination.toolset.knownTools[.cxxCompiler]?.extraCLIOptions ?? [])
let sysrootFlags = [triple.isDarwin() ? "-isysroot" : "--sysroot", sdkDir.pathString]
self.extraFlags.cCompilerFlags.insert(contentsOf: sysrootFlags, at: 0)
}

if triple.isWindows() {
Expand Down
7 changes: 4 additions & 3 deletions Sources/XCBuildSupport/XcodeBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -190,21 +190,22 @@ public final class XcodeBuildSystem: SPMBuildCore.BuildSystem {
settings["LIBRARY_SEARCH_PATHS"] = "$(inherited) \(try buildParameters.toolchain.toolchainLibDir.pathString)"
settings["OTHER_CFLAGS"] = (
["$(inherited)"]
+ buildParameters.toolchain.extraFlags.cCompilerFlags
+ buildParameters.toolchain.extraFlags.cCompilerFlags.map { $0.spm_shellEscaped() }
+ buildParameters.flags.cCompilerFlags.map { $0.spm_shellEscaped() }
).joined(separator: " ")
settings["OTHER_CPLUSPLUSFLAGS"] = (
["$(inherited)"]
+ buildParameters.toolchain.extraFlags.cxxCompilerFlags
+ buildParameters.toolchain.extraFlags.cxxCompilerFlags.map { $0.spm_shellEscaped() }
+ buildParameters.flags.cxxCompilerFlags.map { $0.spm_shellEscaped() }
).joined(separator: " ")
settings["OTHER_SWIFT_FLAGS"] = (
["$(inherited)"]
+ buildParameters.toolchain.extraFlags.swiftCompilerFlags
+ buildParameters.toolchain.extraFlags.swiftCompilerFlags.map { $0.spm_shellEscaped() }
+ buildParameters.flags.swiftCompilerFlags.map { $0.spm_shellEscaped() }
).joined(separator: " ")
settings["OTHER_LDFLAGS"] = (
["$(inherited)"]
+ buildParameters.toolchain.extraFlags.linkerFlags.map { $0.spm_shellEscaped() }
+ buildParameters.flags.linkerFlags.map { $0.spm_shellEscaped() }
).joined(separator: " ")

Expand Down
Loading