Skip to content

[SR-14896] swift build --build-system xcode doesn't respect SWIFT_EXEC #3596

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
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
24 changes: 15 additions & 9 deletions Sources/XCBuildSupport/XcodeBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,7 @@ public final class XcodeBuildSystem: SPMBuildCore.BuildSystem {
}

func createBuildParametersFile() throws -> AbsolutePath? {
// We only generate the build parameters file if it's required.
guard !buildParameters.archs.isEmpty else { return nil }

// Generate the run destination parameters.
let runDestination = XCBBuildParameters.RunDestination(
platform: "macosx",
sdk: "macosx",
Expand All @@ -133,20 +131,28 @@ public final class XcodeBuildSystem: SPMBuildCore.BuildSystem {
supportedArchitectures: [],
Copy link
Contributor

Choose a reason for hiding this comment

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

While you're in this area, can targetArchitecture also be updated to the host architecture (x86_64 or arm64) instead of hardcoded, if it's readily available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely — there's a whole range of fixes here, but I wanted to parcel them out across a couple of different PRs. This particular one would unblock some CI problems, as alluded to, but I plan to put up another PR that cleans up some of the other hardcoded things here. Also -toolchain and -sdk are not respected, for example.

Copy link
Contributor Author

@abertelrud abertelrud Jul 8, 2021

Choose a reason for hiding this comment

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

Put up as a separate PR here: #3597

disableOnlyActiveArch: true
)

// Generate a table of any overriding build settings.
var settings: [String: String] = [:]
// Always specify the path of the effective Swift compiler, which was determined in the same way as for the native build system.
settings["SWIFT_EXEC"] = buildParameters.toolchain.swiftCompiler.pathString
// Optionally also set the list of architectures to build for.
if !buildParameters.archs.isEmpty {
settings["ARCHS"] = buildParameters.archs.joined(separator: " ")
}

// Generate the build parameters.
let params = XCBBuildParameters(
configurationName: buildParameters.configuration.xcbuildName,
overrides: .init(commandLine: .init(table: [
"ARCHS": "\(buildParameters.archs.joined(separator: " "))",
])),
overrides: .init(commandLine: .init(table: settings)),
activeRunDestination: runDestination
)

let encoder = JSONEncoder.makeWithDefaults()

// Write out the parameters as a JSON file, and return the path.
let encoder = JSONEncoder.makeWithDefaults()
let data = try encoder.encode(params)
let file = try withTemporaryFile(deleteOnClose: false) { $0.path }
try localFileSystem.writeFileContents(file, bytes: ByteString(data))

return file
}

Expand Down
35 changes: 34 additions & 1 deletion Tests/CommandsTests/BuildToolTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ final class BuildToolTests: XCTestCase {
@discardableResult
private func execute(
_ args: [String],
environment: [String : String]? = nil,
packagePath: AbsolutePath? = nil
) throws -> (stdout: String, stderr: String) {
return try SwiftPMProduct.SwiftBuild.execute(args, packagePath: packagePath)
return try SwiftPMProduct.SwiftBuild.execute(args, packagePath: packagePath, env: environment)
}

func build(_ args: [String], packagePath: AbsolutePath? = nil) throws -> BuildResult {
Expand Down Expand Up @@ -280,4 +281,36 @@ final class BuildToolTests: XCTestCase {
}
}
}

func testXcodeBuildSystemOverrides() throws {
#if !os(macOS)
try XCTSkipIf(true, "test requires `xcbuild` and is therefore only supported on macOS")
#endif
fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { path in
// Try building using XCBuild without specifying overrides. This should succeed, and should use the default compiler path.
let defaultOutput = try execute(["-c", "debug", "-v"], packagePath: path).stdout
XCTAssert(defaultOutput.contains(Resources.default.swiftCompiler.pathString), defaultOutput)

// Now try building using XCBuild while specifying a faulty compiler override. This should fail. Note that we need to set the executable to use for the manifest itself to the default one, since it defaults to SWIFT_EXEC if not provided.
var overriddenOutput = ""
do {
overriddenOutput = try execute(["-c", "debug", "-v"], environment: ["SWIFT_EXEC": "/usr/bin/false", "SWIFT_EXEC_MANIFEST": Resources.default.swiftCompiler.pathString], packagePath: path).stdout
XCTFail("unexpected success (was SWIFT_EXEC not overridden properly?)")
}
catch SwiftPMProductError.executionFailure(let error, let stdout, _) {
switch error {
case ProcessResult.Error.nonZeroExit(let result) where result.exitStatus != .terminated(code: 0):
overriddenOutput = stdout
break
default:
XCTFail("`swift build' failed in an unexpected manner")
}
}
catch {
XCTFail("`swift build' failed in an unexpected manner")
}
XCTAssert(overriddenOutput.contains("/usr/bin/false"), overriddenOutput)
}
}

}