refactor ManifestResources into simpler ToolchainConfiguration

motivation: improve code readability, prep work for further api refactoring for configuration changes: * remove ManifestResources abstraction and impl into plain struct named ToolchainConfiguration * udpate test module to define defaults for ToolchainConfiguration and UserToolchain to simplify and clean code * update call-sites, tests
swiftlang · Aug 10, 2021 · 1607349 · 1607349
1 parent 2226d7e
commit 1607349
Show file tree

Hide file tree

Showing 30 changed files with 211 additions and 240 deletions.
diff --git a/Sources/Commands/SwiftTool.swift b/Sources/Commands/SwiftTool.swift
@@ -644,7 +644,7 @@ public class SwiftTool {
             // The `cache` directory is in the plugins directory and is where the plugin script runner caches
             // compiled plugin binaries and any other derived information.
             let cacheDir = pluginsDir.appending(component: "cache")
-            let pluginScriptRunner = try DefaultPluginScriptRunner(cacheDir: cacheDir, manifestResources: self._hostToolchain.get().manifestResources)
+            let pluginScriptRunner = try DefaultPluginScriptRunner(cacheDir: cacheDir, toolchain: self._hostToolchain.get().configuration)
 
             // The `outputs` directory contains subdirectories for each combination of package, target, and plugin.
             // Each usage of a plugin has an output directory that is writable by the plugin, where it can write
@@ -873,7 +873,7 @@ public class SwiftTool {
 
             return try ManifestLoader(
                 // Always use the host toolchain's resources for parsing manifest.
-                manifestResources: self._hostToolchain.get().manifestResources,
+                toolchain: self._hostToolchain.get().configuration,
                 isManifestSandboxEnabled: !self.options.shouldDisableSandbox,
                 cacheDir: cachePath,
                 extraManifestFlags: extraManifestFlags

diff --git a/Sources/PackageLoading/CMakeLists.txt b/Sources/PackageLoading/CMakeLists.txt
@@ -17,8 +17,7 @@ add_library(PackageLoading
   PlatformRegistry.swift
   Target+PkgConfig.swift
   TargetSourcesBuilder.swift
-  ToolsVersionLoader.swift
-  UserManifestResources.swift)
+  ToolsVersionLoader.swift)
 target_link_libraries(PackageLoading PUBLIC
   TSCBasic
   Basics

diff --git a/Sources/PackageLoading/ManifestLoader.swift b/Sources/PackageLoading/ManifestLoader.swift
@@ -23,52 +23,6 @@ public enum ManifestParseError: Swift.Error, Equatable {
     case runtimeManifestErrors([String])
 }
 
-/// Resources required for manifest loading.
-///
-/// These requirements are abstracted out to make it easier to add support for
-/// using the package manager with alternate toolchains in the future.
-public protocol ManifestResourceProvider {
-    /// The path of the swift compiler.
-    var swiftCompiler: AbsolutePath { get }
-
-    /// The path of the library resources.
-    var libDir: AbsolutePath { get }
-
-    /// The path to SDK root.
-    ///
-    /// If provided, it will be passed to the swift interpreter.
-    var sdkRoot: AbsolutePath? { get }
-
-    /// The bin directory.
-    var binDir: AbsolutePath? { get }
-
-    /// Extra flags to pass the Swift compiler.
-    var swiftCompilerFlags: [String] { get }
-
-    /// XCTest Location
-    var xctestLocation: AbsolutePath? { get }
-}
-
-/// Default implemention for the resource provider.
-public extension ManifestResourceProvider {
-
-    var sdkRoot: AbsolutePath? {
-        return nil
-    }
-
-    var binDir: AbsolutePath? {
-        return nil
-    }
-
-    var swiftCompilerFlags: [String] {
-        return []
-    }
-
-    var xctestLocation: AbsolutePath? {
-        return nil
-    }
-}
-
 /// Protocol for the manifest loader interface.
 public protocol ManifestLoaderProtocol {
     /// Load the manifest for the package at `path`.
@@ -124,7 +78,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
     private static var _hostTriple = ThreadSafeBox<Triple>()
     private static var _packageDescriptionMinimumDeploymentTarget = ThreadSafeBox<String>()
 
-    private let resources: ManifestResourceProvider
+    private let toolchain: ToolchainConfiguration
     private let serializedDiagnostics: Bool
     private let isManifestSandboxEnabled: Bool
     private let delegate: ManifestLoaderDelegate?
@@ -137,14 +91,14 @@ public final class ManifestLoader: ManifestLoaderProtocol {
     private let operationQueue: OperationQueue
 
     public init(
-        manifestResources: ManifestResourceProvider,
+        toolchain: ToolchainConfiguration,
         serializedDiagnostics: Bool = false,
         isManifestSandboxEnabled: Bool = true,
         cacheDir: AbsolutePath? = nil,
         delegate: ManifestLoaderDelegate? = nil,
         extraManifestFlags: [String] = []
     ) {
-        self.resources = manifestResources
+        self.toolchain = toolchain
         self.serializedDiagnostics = serializedDiagnostics
         self.isManifestSandboxEnabled = isManifestSandboxEnabled
         self.delegate = delegate
@@ -177,8 +131,8 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         completion: @escaping (Result<Manifest, Error>) -> Void
     ) {
         do {
-            let resources = try UserManifestResources(swiftCompiler: swiftCompiler, swiftCompilerFlags: swiftCompilerFlags)
-            let loader = ManifestLoader(manifestResources: resources)
+            let toolchain = try ToolchainConfiguration(swiftCompiler: swiftCompiler, swiftCompilerFlags: swiftCompilerFlags)
+            let loader = ManifestLoader(toolchain: toolchain)
             let toolsVersion = try ToolsVersionLoader().load(at: path, fileSystem: fileSystem)
             let packageLocation = fileSystem.isFile(path) ? path.parentDirectory : path
             let packageIdentity = identityResolver.resolveIdentity(for: packageLocation)
@@ -713,14 +667,14 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         let moduleCachePath = (ProcessEnv.vars["SWIFTPM_MODULECACHE_OVERRIDE"] ?? ProcessEnv.vars["SWIFTPM_TESTS_MODULECACHE"]).flatMap{ AbsolutePath.init($0) }
 
         var cmd: [String] = []
-        cmd += [resources.swiftCompiler.pathString]
+        cmd += [self.toolchain.swiftCompiler.pathString]
         cmd += verbosity.ccArgs
 
         let macOSPackageDescriptionPath: AbsolutePath
         // If we got the binDir that means we could be developing SwiftPM in Xcode
         // which produces a framework for dynamic package products.
         let packageFrameworkPath = runtimePath.appending(component: "PackageFrameworks")
-        if resources.binDir != nil, localFileSystem.exists(packageFrameworkPath)  {
+        if self.toolchain.binDir != nil, localFileSystem.exists(packageFrameworkPath)  {
             cmd += [
                 "-F", packageFrameworkPath.pathString,
                 "-framework", "PackageDescription",
@@ -746,7 +700,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         // Use the same minimum deployment target as the PackageDescription library (with a fallback of 10.15).
 #if os(macOS)
         let triple = Self._hostTriple.memoize {
-            Triple.getHostTriple(usingSwiftCompiler: resources.swiftCompiler)
+            Triple.getHostTriple(usingSwiftCompiler: self.toolchain.swiftCompiler)
         }
 
         let version = try Self._packageDescriptionMinimumDeploymentTarget.memoize {
@@ -756,7 +710,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
 #endif
 
         // Add any extra flags required as indicated by the ManifestLoader.
-        cmd += resources.swiftCompilerFlags
+        cmd += self.toolchain.swiftCompilerFlags
 
         cmd += self.interpreterFlags(for: toolsVersion)
         if let moduleCachePath = moduleCachePath {
@@ -879,7 +833,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         cmd += ["-swift-version", toolsVersion.swiftLanguageVersion.rawValue]
         cmd += ["-I", runtimePath.pathString]
       #if os(macOS)
-        if let sdkRoot = resources.sdkRoot ?? self.sdkRoot() {
+        if let sdkRoot = self.toolchain.sdkRoot ?? self.sdkRoot() {
             cmd += ["-sdk", sdkRoot.pathString]
         }
       #endif
@@ -890,18 +844,18 @@ public final class ManifestLoader: ManifestLoaderProtocol {
     /// Returns the runtime path given the manifest version and path to libDir.
     private func runtimePath(for version: ToolsVersion) -> AbsolutePath {
         // Bin dir will be set when developing swiftpm without building all of the runtimes.
-        if let binDir = resources.binDir {
+        if let binDir = self.toolchain.binDir {
             return binDir
         }
 
         // Otherwise we use the standard location of the manifest API in the toolchain, if it exists.
-        let manifestAPIDir = resources.libDir.appending(component: "ManifestAPI")
+        let manifestAPIDir = self.toolchain.libDir.appending(component: "ManifestAPI")
         if localFileSystem.exists(manifestAPIDir) {
             return manifestAPIDir
         }
 
         // Otherwise, fall back on the old location (this would indicate that we're using an old toolchain).
-        return resources.libDir.appending(version.runtimeSubpath)
+        return self.toolchain.libDir.appending(version.runtimeSubpath)
     }
 
     /// Returns path to the manifest database inside the given cache directory.

diff --git a/Sources/PackageModel/CMakeLists.txt b/Sources/PackageModel/CMakeLists.txt
@@ -31,6 +31,7 @@ add_library(PackageModel
   SupportedLanguageExtension.swift
   SwiftLanguageVersion.swift
   Target.swift
+  ToolchainConfiguration.swift
   ToolsVersion.swift
   ToolsVersionSpecificationGeneration.swift)
 target_link_libraries(PackageModel PUBLIC

diff --git a/...ackageLoading/UserManifestResources.swift → ...PackageModel/ToolchainConfiguration.swift b/...ackageLoading/UserManifestResources.swift → ...PackageModel/ToolchainConfiguration.swift
@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
+ Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See http://swift.org/LICENSE.txt for license information
@@ -10,33 +10,45 @@
 
 import TSCBasic
 
-/// Concrete object for manifest resource provider.
-public struct UserManifestResources: ManifestResourceProvider {
+/// Toolchain configuration required for evaluation os swift code such as the manifests or plugins
+///
+/// These requirements are abstracted out to make it easier to add support for
+/// using the package manager with alternate toolchains in the future.
+public struct ToolchainConfiguration {
+    /// The path of the swift compiler.
     public let swiftCompiler: AbsolutePath
+
+    /// Extra flags to pass the Swift compiler.
     public let swiftCompilerFlags: [String]
+
+    /// The path of the library resources.
     public let libDir: AbsolutePath
+
+    /// The bin directory.
+    public let binDir: AbsolutePath?
+
+    /// The path to SDK root.
+    ///
+    /// If provided, it will be passed to the swift interpreter.
     public let sdkRoot: AbsolutePath?
+
+    /// XCTest Location
     public let xctestLocation: AbsolutePath?
-    public let binDir: AbsolutePath?
 
     public init(
         swiftCompiler: AbsolutePath,
         swiftCompilerFlags: [String],
         libDir: AbsolutePath,
+        binDir: AbsolutePath? = nil,
         sdkRoot: AbsolutePath? = nil,
-        xctestLocation: AbsolutePath? = nil,
-        binDir: AbsolutePath? = nil
+        xctestLocation: AbsolutePath? = nil
     ) {
         self.swiftCompiler = swiftCompiler
         self.swiftCompilerFlags = swiftCompilerFlags
         self.libDir = libDir
+        self.binDir = binDir
         self.sdkRoot = sdkRoot
         self.xctestLocation = xctestLocation
-        self.binDir = binDir
-    }
-
-    public static func libDir(forBinDir binDir: AbsolutePath) -> AbsolutePath {
-        return binDir.parentDirectory.appending(components: "lib", "swift", "pm")
     }
 
     /// Creates the set of manifest resources associated with a `swiftc` executable.
@@ -48,7 +60,11 @@ public struct UserManifestResources: ManifestResourceProvider {
         self.init(
             swiftCompiler: swiftCompiler,
             swiftCompilerFlags: swiftCompilerFlags,
-            libDir: UserManifestResources.libDir(forBinDir: binDir)
+            libDir: Self.libDir(forBinDir: binDir)
         )
     }
+
+    public static func libDir(forBinDir binDir: AbsolutePath) -> AbsolutePath {
+        return binDir.parentDirectory.appending(components: "lib", "swift", "pm")
+    }
 }
diff --git a/Sources/SPMTestSupport/Resources.swift b/Sources/SPMTestSupport/Resources.swift
diff --git a/Sources/SPMTestSupport/Toolchain.swift b/Sources/SPMTestSupport/Toolchain.swift
@@ -0,0 +1,61 @@
+/*
+ This source file is part of the Swift.org open source project
+
+ Copyright (c) 2021 Apple Inc. and the Swift project authors
+ Licensed under Apache License v2.0 with Runtime Library Exception
+
+ See http://swift.org/LICENSE.txt for license information
+ See http://swift.org/CONTRIBUTORS.txt for Swift project authors
+ */
+
+import Foundation
+import PackageModel
+import Workspace
+import TSCBasic
+
+#if os(macOS)
+private func bundleRoot() -> AbsolutePath {
+    for bundle in Bundle.allBundles where bundle.bundlePath.hasSuffix(".xctest") {
+        return AbsolutePath(bundle.bundlePath).parentDirectory
+    }
+    fatalError()
+}
+#endif
+
+private func binDir() -> AbsolutePath {
+#if os(macOS)
+    return bundleRoot()
+#else
+    return AbsolutePath(CommandLine.arguments[0], relativeTo: localFileSystem.currentWorkingDirectory!).parentDirectory
+#endif
+}
+
+extension ToolchainConfiguration {
+    public static var `default`: Self {
+        get {
+            let toolchain = UserToolchain.default
+            return .init(
+                swiftCompiler: toolchain.configuration.swiftCompiler,
+                swiftCompilerFlags: [],
+                libDir: toolchain.configuration.libDir,
+                binDir: toolchain.configuration.binDir
+            )
+        }
+    }
+
+#if os(macOS)
+    public var sdkPlatformFrameworksPath: AbsolutePath {
+        return Destination.sdkPlatformFrameworkPaths()!.fwk
+    }
+#endif
+
+}
+
+extension UserToolchain {
+    public static var `default`: Self {
+        get {
+            let binDir = binDir()
+            return try! .init(destination: Destination.hostDestination(binDir))
+        }
+    }
+}