Merge pull request #588 from abertelrud/error-improvements

abertelrud · web-flow · commit 64a4183e83f4 · 2016-08-29T11:32:46.000-07:00
Improved error messages for when swiftc, clang, or sysroot can't be found
diff --git a/Sources/Build/Command.compile(ClangModule).swift b/Sources/Build/Command.compile(ClangModule).swift
@@ -114,7 +114,7 @@ struct ClangModuleBuildMetadata {
 }
 
 extension Command {
-    static func compile(clangModule module: ClangModule, externalModules: Set<Module>, configuration conf: Configuration, prefix: AbsolutePath, CC: String, otherArgs: [String]) throws -> [Command] {
+    static func compile(clangModule module: ClangModule, externalModules: Set<Module>, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], compilerExec: AbsolutePath) throws -> [Command] {
 
         let buildMeta = ClangModuleBuildMetadata(module: module, prefix: prefix, otherArgs: otherArgs)
         
@@ -138,7 +138,7 @@ extension Command {
             let clang = ClangTool(desc: "Compile \(module.name) \(path.filename.asString)",
                                   inputs: buildMeta.inputs + [path.source.asString],
                                   outputs: [path.object.asString],
-                                  args: [CC] + args,
+                                  args: [compilerExec.asString] + args,
                                   deps: path.deps.asString)
 
             let command = Command(node: path.object.asString, tool: clang)
diff --git a/Sources/Build/Command.compile(SwiftModule).swift b/Sources/Build/Command.compile(SwiftModule).swift
@@ -14,7 +14,7 @@ import PackageLoading
 import Utility
 
 extension Command {
-    static func compile(swiftModule module: SwiftModule, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], SWIFT_EXEC: String) throws -> Command {
+    static func compile(swiftModule module: SwiftModule, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], compilerExec: AbsolutePath) throws -> Command {
         let otherArgs = otherArgs + module.XccFlags(prefix) + (try module.pkgConfigSwiftcArgs()) + module.moduleCacheArgs(prefix: prefix)
         var args = ["-j\(SwiftcTool.numThreads)", "-D", "SWIFT_PACKAGE"]
 
@@ -29,7 +29,7 @@ extension Command {
         args += ["-F", try platformFrameworksPath().asString]
       #endif
 
-        let tool = SwiftcTool(module: module, prefix: prefix, otherArgs: args + otherArgs, executable: SWIFT_EXEC, conf: conf)
+        let tool = SwiftcTool(module: module, prefix: prefix, otherArgs: args + otherArgs, executable: compilerExec.asString, conf: conf)
         return Command(node: module.targetName, tool: tool)
     }
 }
diff --git a/Sources/Build/Command.link(ClangModule).swift b/Sources/Build/Command.link(ClangModule).swift
@@ -14,7 +14,7 @@ import PackageLoading
 import Utility
 
 extension Command {
-    static func linkClangModule(_ product: Product, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], CC: String) throws -> Command {
+    static func linkClangModule(_ product: Product, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], linkerExec: AbsolutePath) throws -> Command {
         precondition(product.containsOnlyClangModules)
 
         let clangModules = product.modules.flatMap { $0 as? ClangModule }
@@ -54,7 +54,7 @@ extension Command {
         let shell = ShellTool(description: "Linking \(product.name)",
                               inputs: objects.map{ $0.asString } + inputs,
                               outputs: [productPath.asString, product.targetName],
-                              args: [CC] + args)
+                              args: [linkerExec.asString] + args)
         
         return Command(node: product.targetName, tool: shell)
     }
diff --git a/Sources/Build/Command.link(SwiftModule).swift b/Sources/Build/Command.link(SwiftModule).swift
@@ -16,21 +16,21 @@ import Utility
 //FIXME messy :/
 
 extension Command {
-    static func linkSwiftModule(_ product: Product, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], SWIFT_EXEC: String) throws -> Command {
+    static func linkSwiftModule(_ product: Product, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], linkerExec: AbsolutePath) throws -> Command {
 
         // Get the unique set of all input modules.
         //
         // FIXME: This needs to handle C language targets.
         let buildables = OrderedSet(product.modules.flatMap{ [$0] + $0.recursiveDependencies }.flatMap{ $0 as? SwiftModule }).contents
         
-        var objects = buildables.flatMap { SwiftcTool(module: $0, prefix: prefix, otherArgs: [], executable: SWIFT_EXEC, conf: conf).objects }
+        var objects = buildables.flatMap { SwiftcTool(module: $0, prefix: prefix, otherArgs: [], executable: linkerExec.asString, conf: conf).objects }
 
         let outpath = prefix.appending(product.outname)
 
         var args: [String]
         switch product.type {
         case .Library(.Dynamic), .Executable, .Test:
-            args = [SWIFT_EXEC] + otherArgs
+            args = [linkerExec.asString] + otherArgs
 
             if conf == .debug {
                 args += ["-g"]
diff --git a/Sources/Build/describe().swift b/Sources/Build/describe().swift
@@ -32,16 +32,13 @@ public func describe(_ prefix: AbsolutePath, _ conf: Configuration, _ graph: Pac
     try makeDirectories(prefix)
     let swiftcArgs = flags.cCompilerFlags.flatMap{ ["-Xcc", $0] } + flags.swiftCompilerFlags + verbosity.ccArgs
 
-    let SWIFT_EXEC = toolchain.SWIFT_EXEC
-    let CC = getenv("CC") ?? "clang"
-
     var commands = [Command]()
     var targets = Targets()
 
     for module in graph.modules {
         switch module {
         case let module as SwiftModule:
-            let compile = try Command.compile(swiftModule: module, configuration: conf, prefix: prefix, otherArgs: swiftcArgs + toolchain.platformArgsSwiftc, SWIFT_EXEC: SWIFT_EXEC)
+            let compile = try Command.compile(swiftModule: module, configuration: conf, prefix: prefix, otherArgs: swiftcArgs + toolchain.swiftPlatformArgs, compilerExec: toolchain.swiftCompiler)
             commands.append(compile)
             targets.append([compile], for: module)
 
@@ -51,7 +48,7 @@ public func describe(_ prefix: AbsolutePath, _ conf: Configuration, _ graph: Pac
             if module.isTest { continue }
           #endif
             // FIXME: Find a way to eliminate `externalModules` from here.
-            let compile = try Command.compile(clangModule: module, externalModules: graph.externalModules, configuration: conf, prefix: prefix, CC: CC, otherArgs: flags.cCompilerFlags + toolchain.platformArgsClang)
+            let compile = try Command.compile(clangModule: module, externalModules: graph.externalModules, configuration: conf, prefix: prefix, otherArgs: flags.cCompilerFlags + toolchain.clangPlatformArgs, compilerExec: toolchain.clangCompiler)
             commands += compile
             targets.append(compile, for: module)
 
@@ -74,9 +71,9 @@ public func describe(_ prefix: AbsolutePath, _ conf: Configuration, _ graph: Pac
 #endif
         let command: Command
         if product.containsOnlyClangModules {
-            command = try Command.linkClangModule(product, configuration: conf, prefix: prefix, otherArgs: Xld, CC: CC)
+            command = try Command.linkClangModule(product, configuration: conf, prefix: prefix, otherArgs: Xld, linkerExec: toolchain.clangCompiler)
         } else {
-            command = try Command.linkSwiftModule(product, configuration: conf, prefix: prefix, otherArgs: Xld + swiftcArgs + toolchain.platformArgsSwiftc + rpathArgs, SWIFT_EXEC: SWIFT_EXEC)
+            command = try Command.linkSwiftModule(product, configuration: conf, prefix: prefix, otherArgs: Xld + swiftcArgs + toolchain.swiftPlatformArgs + rpathArgs, linkerExec: toolchain.swiftCompiler)
         }
 
         commands.append(command)
diff --git a/Sources/Build/misc.swift b/Sources/Build/misc.swift
@@ -16,11 +16,20 @@ import func POSIX.getenv
 import func POSIX.popen
 
 public protocol Toolchain {
-    var platformArgsClang: [String] { get }
-    var platformArgsSwiftc: [String] { get }
-    var sysroot: String?  { get }
-    var SWIFT_EXEC: String { get }
-    var clang: String { get }
+    /// Path of the `swiftc` compiler.
+    var swiftCompiler: AbsolutePath { get }
+    
+    /// Platform-specific arguments for Swift compiler.
+    var swiftPlatformArgs: [String] { get }
+    
+    /// Path of the `clang` compiler.
+    var clangCompiler: AbsolutePath { get }
+    
+    /// Platform-specific arguments for Clang compiler.
+    var clangPlatformArgs: [String] { get }
+    
+    /// Path of the default SDK (a.k.a. "sysroot"), if any.
+    var defaultSDK: AbsolutePath? { get }
 }
 
 extension AbsolutePath {
diff --git a/Sources/Commands/Error.swift b/Sources/Commands/Error.swift
@@ -20,7 +20,7 @@ import enum PackageLoading.ManifestParseError
 
 public enum Error: Swift.Error {
     case noManifestFound
-    case invalidToolchain
+    case invalidToolchain(problem: String)
     case buildYAMLNotFound(String)
     case repositoryHasChanges(String)
 }
@@ -30,8 +30,8 @@ extension Error: FixableError {
         switch self {
         case .noManifestFound:
             return "no \(Manifest.filename) file found"
-        case .invalidToolchain:
-            return "invalid inferred toolchain"
+        case .invalidToolchain(let problem):
+            return "invalid inferred toolchain: \(problem)"
         case .buildYAMLNotFound(let value):
             return "no build YAML found: \(value)"
         case .repositoryHasChanges(let value):
diff --git a/Sources/Commands/UserToolchain.swift b/Sources/Commands/UserToolchain.swift
@@ -15,47 +15,94 @@ import protocol Build.Toolchain
 
 #if os(macOS)
     private let whichClangArgs = ["xcrun", "--find", "clang"]
+    private let whichDefaultSDKArgs = ["xcrun", "--sdk", "macosx", "--show-sdk-path"]
 #else
     private let whichClangArgs = ["which", "clang"]
 #endif
 
 struct UserToolchain: Toolchain {
-    let SWIFT_EXEC: String
-    let clang: String
-    let sysroot: String?
+    /// Path of the `swiftc` compiler.
+    let swiftCompiler: AbsolutePath
+    
+    /// Path of the `clang` compiler.
+    let clangCompiler: AbsolutePath
+    
+    /// Path of the default SDK (a.k.a. "sysroot"), if any.
+    let defaultSDK: AbsolutePath?
 
 #if os(macOS)
-    var platformArgsClang: [String] {
-        return ["-arch", "x86_64", "-mmacosx-version-min=10.10", "-isysroot", sysroot!]
+    var clangPlatformArgs: [String] {
+        return ["-arch", "x86_64", "-mmacosx-version-min=10.10", "-isysroot", defaultSDK!.asString]
     }
-
-    var platformArgsSwiftc: [String] {
-        return ["-target", "x86_64-apple-macosx10.10", "-sdk", sysroot!]
+    var swiftPlatformArgs: [String] {
+        return ["-target", "x86_64-apple-macosx10.10", "-sdk", defaultSDK!.asString]
     }
 #else
-    let platformArgsClang: [String] = []
-    let platformArgsSwiftc: [String] = []
+    let clangPlatformArgs: [String] = []
+    let swiftPlatformArgs: [String] = []
 #endif
 
     init() throws {
-        do {
-            SWIFT_EXEC = getenv("SWIFT_EXEC")
-                // use the swiftc installed alongside ourselves
-                ?? AbsolutePath(CommandLine.arguments[0], relativeTo: currentWorkingDirectory).parentDirectory.appending(component: "swiftc").asString
-
-            clang = try getenv("CC") ?? POSIX.popen(whichClangArgs).chomp()
-
-            #if os(macOS)
-                sysroot = try getenv("SYSROOT") ?? POSIX.popen(["xcrun", "--sdk", "macosx", "--show-sdk-path"]).chomp()
-            #else
-                sysroot = nil
-            #endif
-
-            guard !SWIFT_EXEC.isEmpty && !clang.isEmpty && (sysroot == nil || !sysroot!.isEmpty) else {
-                throw Error.invalidToolchain
+        // Find the Swift compiler, looking first in the environment.
+        if let value = getenv("SWIFT_EXEC"), !value.isEmpty {
+            // We have a value, but it could be an absolute or a relative path.
+            swiftCompiler = AbsolutePath(value, relativeTo: currentWorkingDirectory)
+        }
+        else {
+            // No value in env, so look for `swiftc` alongside our own binary.
+            swiftCompiler = AbsolutePath(CommandLine.arguments[0], relativeTo: currentWorkingDirectory).parentDirectory.appending(component: "swiftc")
+        }
+        
+        // Check that it's valid in the file system.
+        // FIXME: We should also check that it resolves to an executable file
+        //        (it could be a symlink to such as file).
+        guard localFileSystem.exists(swiftCompiler) else {
+            throw Error.invalidToolchain(problem: "could not find `swiftc` at expected path \(swiftCompiler.asString)")
+        }
+        
+        // Find the Clang compiler, looking first in the environment.
+        if let value = getenv("CC"), !value.isEmpty {
+            // We have a value, but it could be an absolute or a relative path.
+            clangCompiler = AbsolutePath(value, relativeTo: currentWorkingDirectory)
+        }
+        else {
+            // No value in env, so search for `clang`.
+            guard let foundPath = try? POSIX.popen(whichClangArgs).chomp(), !foundPath.isEmpty else {
+                throw Error.invalidToolchain(problem: "could not find `clang`")
+            }
+            clangCompiler = AbsolutePath(foundPath, relativeTo: currentWorkingDirectory)
+        }
+        
+        // Check that it's valid in the file system.
+        // FIXME: We should also check that it resolves to an executable file
+        //        (it could be a symlink to such as file).
+        guard localFileSystem.exists(clangCompiler) else {
+            throw Error.invalidToolchain(problem: "could not find `clang` at expected path \(clangCompiler.asString)")
+        }
+        
+        // Find the default SDK (on macOS only).
+      #if os(macOS)
+        if let value = getenv("SYSROOT"), !value.isEmpty {
+            // We have a value, but it could be an absolute or a relative path.
+            defaultSDK = AbsolutePath(value, relativeTo: currentWorkingDirectory)
+        }
+        else {
+            // No value in env, so search for it.
+            guard let foundPath = try? POSIX.popen(whichDefaultSDKArgs).chomp(), !foundPath.isEmpty else {
+                throw Error.invalidToolchain(problem: "could not find default SDK")
+            }
+            defaultSDK = AbsolutePath(foundPath, relativeTo: currentWorkingDirectory)
+        }
+        
+        // If we have an SDK, we check that it's valid in the file system.
+        if let sdk = defaultSDK {
+            // FIXME: We should probably also check that it is a directory, etc.
+            guard localFileSystem.exists(sdk) else {
+                throw Error.invalidToolchain(problem: "could not find default SDK at expected path \(sdk.asString)")
             }
-        } catch POSIX.Error.exitStatus {
-            throw Error.invalidToolchain
         }
+      #else
+        defaultSDK = nil
+      #endif
     }
 }
diff --git a/Tests/BuildTests/DescribeTests.swift b/Tests/BuildTests/DescribeTests.swift
@@ -21,11 +21,11 @@ final class DescribeTests: XCTestCase {
     let dummyPackage = Package(manifest: Manifest(path: AbsolutePath("/"), url: "/", package: PackageDescription.Package(name: "Foo"), products: [], version: nil), path: AbsolutePath("/"), modules: [], testModules: [], products: [])
     
     struct InvalidToolchain: Toolchain {
-        var platformArgsClang: [String] { fatalError() }
-        var platformArgsSwiftc: [String] { fatalError() }
-        var sysroot: String?  { fatalError() }
-        var SWIFT_EXEC: String { fatalError() }
-        var clang: String { fatalError() }
+        var swiftCompiler: AbsolutePath { fatalError() }
+        var clangCompiler: AbsolutePath { fatalError() }
+        var defaultSDK: AbsolutePath?  { fatalError() }
+        var swiftPlatformArgs: [String] { fatalError() }
+        var clangPlatformArgs: [String] { fatalError() }
     }
 
     func testDescribingNoModulesThrows() {

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ import PackageLoading`
`14`	`14`	`import Utility`
`15`	`15`
`16`	`16`	`extension Command {`
`17`		`- static func compile(swiftModule module: SwiftModule, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], SWIFT_EXEC: String) throws -> Command {`
	`17`	`+ static func compile(swiftModule module: SwiftModule, configuration conf: Configuration, prefix: AbsolutePath, otherArgs: [String], compilerExec: AbsolutePath) throws -> Command {`
`18`	`18`	`let otherArgs = otherArgs + module.XccFlags(prefix) + (try module.pkgConfigSwiftcArgs()) + module.moduleCacheArgs(prefix: prefix)`
`19`	`19`	`var args = ["-j\(SwiftcTool.numThreads)", "-D", "SWIFT_PACKAGE"]`
`20`	`20`
`@@ -29,7 +29,7 @@ extension Command {`
`29`	`29`	`args += ["-F", try platformFrameworksPath().asString]`
`30`	`30`	`#endif`
`31`	`31`
`32`		`- let tool = SwiftcTool(module: module, prefix: prefix, otherArgs: args + otherArgs, executable: SWIFT_EXEC, conf: conf)`
	`32`	`+ let tool = SwiftcTool(module: module, prefix: prefix, otherArgs: args + otherArgs, executable: compilerExec.asString, conf: conf)`
`33`	`33`	`return Command(node: module.targetName, tool: tool)`
`34`	`34`	`}`
`35`	`35`	`}`