[Basic] Convert precondition in determineTmpDir into a runtime error

This assumption is not always true and we shouldn't be crashing in such
cases.

<rdar://problem/45198013> Turn precondition in determineTempDirectory into a runtime throw

Author: aciidgh

Sources/Basic/TemporaryFile.swift

@@ -16,9 +16,13 @@ import class Foundation.FileManager
 public enum TempFileError: Swift.Error {
     /// Could not create a unique temporary filename.
     case couldNotCreateUniqueName
+
     /// Some error thrown defined by posix's open().
     // FIXME: This should be factored out into a open error enum.
     case other(Int32)
+
+    /// Couldn't find a temporary directory.
+    case couldNotFindTmpDir
 }
 
 private extension TempFileError {
@@ -39,11 +43,12 @@ private extension TempFileError {
 ///     - dir: If present this will be the temporary directory.
 ///
 /// - Returns: Path to directory in which temporary file should be created.
-public func determineTempDirectory(_ dir: AbsolutePath? = nil) -> AbsolutePath {
+public func determineTempDirectory(_ dir: AbsolutePath? = nil) throws -> AbsolutePath {
     // FIXME: Add other platform specific locations.
     let tmpDir = dir ?? cachedTempDirectory
-    // FIXME: This is a runtime condition, so it should throw and not crash.
-    precondition(localFileSystem.isDirectory(tmpDir))
+    guard localFileSystem.isDirectory(tmpDir) else {
+        throw TempFileError.couldNotFindTmpDir
+    }
     return tmpDir
 }
 
@@ -95,7 +100,7 @@ public final class TemporaryFile {
         self.prefix = prefix
         self.deleteOnClose = deleteOnClose
         // Determine in which directory to create the temporary file.
-        self.dir = determineTempDirectory(dir)
+        self.dir = try determineTempDirectory(dir)
         // Construct path to the temporary file.
         let path = self.dir.appending(RelativePath(prefix + ".XXXXXX" + suffix))
 
@@ -193,7 +198,7 @@ public final class TemporaryDirectory {
         self.shouldRemoveTreeOnDeinit = removeTreeOnDeinit
         self.prefix = prefix
         // Construct path to the temporary directory.
-        let path = determineTempDirectory(dir).appending(RelativePath(prefix + ".XXXXXX"))
+        let path = try determineTempDirectory(dir).appending(RelativePath(prefix + ".XXXXXX"))
 
         // Convert path to a C style string terminating with null char to be an valid input
         // to mkdtemp method. The XXXXXX in this string will be replaced by a random string

Sources/Build/BuildPlan.swift

@@ -18,7 +18,8 @@ import func POSIX.getenv
 public struct BuildParameters {
 
     /// Path to the module cache directory to use for SwiftPM's own tests.
-    public static let swiftpmTestCache = resolveSymlinks(determineTempDirectory()).appending(component: "org.swift.swiftpm.tests-3")
+    // FIXME: Error handling.
+    public static let swiftpmTestCache = resolveSymlinks(try! determineTempDirectory()).appending(component: "org.swift.swiftpm.tests-3")
 
     /// Returns the directory to be used for module cache.
     fileprivate var moduleCache: AbsolutePath {

Sources/Commands/SwiftTool.swift

@@ -620,7 +620,7 @@ public class SwiftTool<Options: ToolOptions> {
     func runLLBuildAsExecutable(manifest: AbsolutePath, llbuildTarget: String) throws {
         // Create a temporary directory for the build process.
         let tempDirName = "org.swift.swiftpm.\(NSUserName())"
-        let tempDir = Basic.determineTempDirectory().appending(component: tempDirName)
+        let tempDir = try determineTempDirectory().appending(component: tempDirName)
         try localFileSystem.createDirectory(tempDir, recursive: true)
 
         // Run the swift-build-tool with the generated manifest.
@@ -717,10 +717,9 @@ public class SwiftTool<Options: ToolOptions> {
         return Result(anyError: {
             try ManifestLoader(
                 // Always use the host toolchain's resources for parsing manifest.
-                resources: self._hostToolchain.dematerialize().manifestResources,
+                manifestResources: self._hostToolchain.dematerialize().manifestResources,
                 isManifestSandboxEnabled: !self.options.shouldDisableSandbox,
-                isManifestCachingEnabled: !self.options.shouldDisableManifestCaching,
-                cacheDir: self.buildPath
+                cacheDir: self.options.shouldDisableManifestCaching ? nil : self.buildPath
             )
         })
     }()

Sources/PackageLoading/ManifestLoader.swift

@@ -131,20 +131,20 @@ public final class ManifestLoader: ManifestLoaderProtocol {
 
     let resources: ManifestResourceProvider
     let isManifestSandboxEnabled: Bool
-    let isManifestCachingEnabled: Bool
-    let cacheDir: AbsolutePath
+    var isManifestCachingEnabled: Bool {
+        return cacheDir != nil
+    }
+    let cacheDir: AbsolutePath!
     let delegate: ManifestLoaderDelegate?
 
     public init(
-        resources: ManifestResourceProvider,
+        manifestResources: ManifestResourceProvider,
         isManifestSandboxEnabled: Bool = true,
-        isManifestCachingEnabled: Bool = true,
-        cacheDir: AbsolutePath = determineTempDirectory(),
+        cacheDir: AbsolutePath? = nil,
         delegate: ManifestLoaderDelegate? = nil
     ) {
-        self.resources = resources
+        self.resources = manifestResources
         self.isManifestSandboxEnabled = isManifestSandboxEnabled
-        self.isManifestCachingEnabled = isManifestCachingEnabled
         self.delegate = delegate
         self.cacheDir = cacheDir
     }
@@ -155,10 +155,8 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         isManifestSandboxEnabled: Bool = true
     ) {
         self.init(
-            resources: resources,
-            isManifestSandboxEnabled: isManifestSandboxEnabled,
-            isManifestCachingEnabled: false,
-            cacheDir: determineTempDirectory()
+            manifestResources: resources,
+            isManifestSandboxEnabled: isManifestSandboxEnabled
        )
     }
 

Sources/Utility/Platform.swift

@@ -44,7 +44,7 @@ public enum Platform {
         var directories = [AbsolutePath]()
         // Compute the directories.
         directories.append(AbsolutePath("/private/var/tmp"))
-        directories.append(Basic.determineTempDirectory())
+        (try? Basic.determineTempDirectory()).map{ directories.append($0) }
       #if os(macOS)
         getConfstr(_CS_DARWIN_USER_TEMP_DIR).map({ directories.append($0) })
         getConfstr(_CS_DARWIN_USER_CACHE_DIR).map({ directories.append($0) })

Tests/PackageGraphPerformanceTests/DependencyResolverPerfTests.swift

@@ -97,7 +97,7 @@ class DependencyResolverPerfTests: XCTestCasePerf {
 
             let containerProvider = RepositoryPackageContainerProvider(
                 repositoryManager: repositoryManager,
-                manifestLoader: ManifestLoader(resources: Resources.default, isManifestCachingEnabled: false, cacheDir: path))
+                manifestLoader: ManifestLoader(manifestResources: Resources.default))
 
             let resolver = DependencyResolver(containerProvider, GitRepositoryResolutionHelper.DummyResolverDelegate())
             let container = PackageReference(identity: "dep", path: dep.asString)

Tests/PackageLoadingPerformanceTests/ManifestLoadingTests.swift

@@ -15,7 +15,7 @@ import TestSupport
 import PackageLoading
 
 class ManifestLoadingPerfTests: XCTestCasePerf {
-    let manifestLoader = ManifestLoader(resources: Resources.default, isManifestCachingEnabled: false)
+    let manifestLoader = ManifestLoader(manifestResources: Resources.default)
 
     func write(_ bytes: ByteString, body: (AbsolutePath) -> ()) {
         mktmpdir { path in

Tests/PackageLoadingTests/PD4LoadingTests.swift

@@ -19,7 +19,7 @@ import TestSupport
 import PackageLoading
 
 class PackageDescription4LoadingTests: XCTestCase {
-    let manifestLoader = ManifestLoader(resources: Resources.default, isManifestCachingEnabled: false)
+    let manifestLoader = ManifestLoader(manifestResources: Resources.default)
 
     private func loadManifestThrowing(
         _ contents: ByteString,

Tests/PackageLoadingTests/PD4_2LoadingTests.swift

@@ -18,7 +18,7 @@ import PackageLoading
 
 // FIXME: We should share the infra with other loading tests.
 class PackageDescription4_2LoadingTests: XCTestCase {
-    let manifestLoader = ManifestLoader(resources: Resources.default, isManifestCachingEnabled: false)
+    let manifestLoader = ManifestLoader(manifestResources: Resources.default)
 
     private func loadManifestThrowing(
         _ contents: ByteString,
@@ -378,8 +378,9 @@ class PackageDescription4_2LoadingTests: XCTestCase {
             }
 
             let delegate = ManifestTestDelegate()
+
             let manifestLoader = ManifestLoader(
-                resources: Resources.default, cacheDir: path, delegate: delegate)
+                manifestResources: Resources.default, cacheDir: path, delegate: delegate)
 
             func check(loader: ManifestLoader, expectCached: Bool) {
                 delegate.clear()
@@ -422,7 +423,7 @@ class PackageDescription4_2LoadingTests: XCTestCase {
             }
 
             let noCacheLoader = ManifestLoader(
-                resources: Resources.default, isManifestCachingEnabled: false, delegate: delegate)
+                manifestResources: Resources.default, delegate: delegate)
             for _ in 0..<2 {
                 check(loader: noCacheLoader, expectCached: false)
             }

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -119,8 +119,7 @@ final class WorkspaceTests: XCTestCase {
                 """
             }
 
-            let manifestLoader = ManifestLoader(
-                resources: Resources.default, isManifestCachingEnabled: false)
+            let manifestLoader = ManifestLoader(manifestResources: Resources.default)
 
             let sandbox = path.appending(component: "ws")
             let ws = Workspace(