Skip to content

Commit

Permalink
[SR-8135] Changes for bug fix to do a clang check only when needed fo…
Browse files Browse the repository at this point in the history
…r certain … (#1695)

* Changes for bug fix to do a clang check only when needed for certain commands

* Code review changes. Changed lookup and envSearchPaths to be a method and property of UserToolchain class
  • Loading branch information
prachipai authored and aciidgh committed Jul 24, 2018
1 parent 567b0c5 commit 353522a
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 32 deletions.
2 changes: 1 addition & 1 deletion Sources/Build/Toolchain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public protocol Toolchain {
var swiftCompiler: AbsolutePath { get }

/// Path of the `clang` compiler.
var clangCompiler: AbsolutePath { get }
func getClangCompiler() throws -> AbsolutePath

/// Additional flags to be passed to the C compiler.
var extraCCFlags: [String] { get }
Expand Down
8 changes: 4 additions & 4 deletions Sources/Build/llbuild.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public struct LLBuildManifestGenerator {
buildByDefault: plan.graph.reachableTargets.contains(target),
isTest: description.isTestTarget)
case .clang(let description):
targets.append(createClangCompileTarget(description),
targets.append(try createClangCompileTarget(description),
buildByDefault: plan.graph.reachableTargets.contains(target),
isTest: description.isTestTarget)
}
Expand Down Expand Up @@ -272,14 +272,14 @@ public struct LLBuildManifestGenerator {
}

/// Create a llbuild target for a Clang target description.
private func createClangCompileTarget(_ target: ClangTargetBuildDescription) -> Target {
private func createClangCompileTarget(_ target: ClangTargetBuildDescription) throws -> Target {

let standards = [
(target.clangTarget.cxxLanguageStandard, SupportedLanguageExtension.cppExtensions),
(target.clangTarget.cLanguageStandard, SupportedLanguageExtension.cExtensions),
]

let commands: [Command] = target.compilePaths().map({ path in
let commands: [Command] = try target.compilePaths().map({ path in
var args = target.basicArguments()
args += ["-MD", "-MT", "dependencies", "-MF", path.deps.asString]

Expand All @@ -298,7 +298,7 @@ public struct LLBuildManifestGenerator {
//FIXME: Should we add build time dependency on dependent targets?
inputs: [path.source.asString],
outputs: [path.object.asString],
args: [plan.buildParameters.toolchain.clangCompiler.asString] + args,
args: [try plan.buildParameters.toolchain.getClangCompiler().asString] + args,
deps: path.deps.asString)
return Command(name: path.object.asString, tool: clang)
})
Expand Down
59 changes: 33 additions & 26 deletions Sources/Commands/UserToolchain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ public struct UserToolchain: Toolchain {
/// Path of the `swiftc` compiler.
public let swiftCompiler: AbsolutePath

/// Path of the `clang` compiler.
public let clangCompiler: AbsolutePath

public let extraCCFlags: [String]

public let extraSwiftCFlags: [String]
Expand Down Expand Up @@ -69,6 +66,9 @@ public struct UserToolchain: Toolchain {
/// The compilation destination object.
let destination: Destination

/// Search paths from the PATH environment variable.
let envSearchPaths: [AbsolutePath]

/// Returns the runtime library for the given sanitizer.
func runtimeLibrary(for sanitizer: Sanitizer) throws -> AbsolutePath {
// FIXME: This is only for SwiftPM development time support. It is OK
Expand Down Expand Up @@ -122,33 +122,16 @@ public struct UserToolchain: Toolchain {
return (SWIFT_EXEC ?? resolvedBinDirCompiler, SWIFT_EXEC_MANIFEST ?? resolvedBinDirCompiler)
}

public init(destination: Destination) throws {
self.destination = destination

// Get the search paths from PATH.
let envSearchPaths = getEnvSearchPaths(
pathString: getenv("PATH"), currentWorkingDirectory: localFileSystem.currentWorkingDirectory)

func lookup(fromEnv: String) -> AbsolutePath? {
return lookupExecutablePath(
filename: getenv(fromEnv),
searchPaths: envSearchPaths)
}

// Get the binDir from destination.
let binDir = destination.binDir
private static func lookup(variable: String, searchPaths: [AbsolutePath]) -> AbsolutePath? {
return lookupExecutablePath(filename: getenv(variable), searchPaths: searchPaths)
}

let swiftCompilers = try UserToolchain.determineSwiftCompilers(binDir: binDir, lookup: lookup(fromEnv:))
self.swiftCompiler = swiftCompilers.compile
public func getClangCompiler() throws -> AbsolutePath {

// Look for llbuild in bin dir.
llbuild = binDir.appending(component: "swift-build-tool")
guard localFileSystem.exists(llbuild) else {
throw Error.invalidToolchain(problem: "could not find `llbuild` at expected path \(llbuild.asString)")
}
let clangCompiler: AbsolutePath

// Find the Clang compiler, looking first in the environment.
if let value = lookup(fromEnv: "CC") {
if let value = UserToolchain.lookup(variable: "CC", searchPaths: envSearchPaths) {
clangCompiler = value
} else {
// No value in env, so search for `clang`.
Expand All @@ -163,6 +146,30 @@ public struct UserToolchain: Toolchain {
guard localFileSystem.isExecutableFile(clangCompiler) else {
throw Error.invalidToolchain(problem: "could not find `clang` at expected path \(clangCompiler.asString)")
}
return clangCompiler
}

public init(destination: Destination) throws {
self.destination = destination

// Get the search paths from PATH.
let searchPaths = getEnvSearchPaths(
pathString: getenv("PATH"), currentWorkingDirectory: localFileSystem.currentWorkingDirectory)

self.envSearchPaths = searchPaths

// Get the binDir from destination.
let binDir = destination.binDir

let swiftCompilers = try UserToolchain.determineSwiftCompilers(binDir: binDir, lookup: { UserToolchain.lookup(variable: $0, searchPaths: searchPaths) })
self.swiftCompiler = swiftCompilers.compile

// Look for llbuild in bin dir.
llbuild = binDir.appending(component: "swift-build-tool")
guard localFileSystem.exists(llbuild) else {
throw Error.invalidToolchain(problem: "could not find `llbuild` at expected path \(llbuild.asString)")
}


// We require xctest to exist on macOS.
#if os(macOS)
Expand Down
4 changes: 3 additions & 1 deletion Tests/BuildTests/BuildPlanTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import PackageModel

private struct MockToolchain: Toolchain {
let swiftCompiler = AbsolutePath("/fake/path/to/swiftc")
let clangCompiler = AbsolutePath("/fake/path/to/clang")
let extraCCFlags: [String] = []
let extraSwiftCFlags: [String] = []
#if os(macOS)
Expand All @@ -32,6 +31,9 @@ private struct MockToolchain: Toolchain {
#else
let dynamicLibraryExtension = "so"
#endif
func getClangCompiler() throws -> AbsolutePath {
return AbsolutePath("/fake/path/to/clang")
}
}

final class BuildPlanTests: XCTestCase {
Expand Down

0 comments on commit 353522a

Please sign in to comment.