refactor ManifestResources into simpler ToolchainConfiguration (#3656)

* 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

Author: tomerd

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

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

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
@@ -157,6 +111,26 @@ public final class ManifestLoader: ManifestLoaderProtocol {
         self.operationQueue.maxConcurrentOperationCount = Concurrency.maxOperations
     }
 
+    // deprecated 8/2021
+    @available(*, deprecated, message: "use non-deprecated constructor instead")
+    public convenience init(
+        manifestResources: ToolchainConfiguration,
+        serializedDiagnostics: Bool = false,
+        isManifestSandboxEnabled: Bool = true,
+        cacheDir: AbsolutePath? = nil,
+        delegate: ManifestLoaderDelegate? = nil,
+        extraManifestFlags: [String] = []
+    ) {
+        self.init(
+            toolchain: manifestResources,
+            serializedDiagnostics: serializedDiagnostics,
+            isManifestSandboxEnabled: isManifestSandboxEnabled,
+            cacheDir: cacheDir,
+            delegate: delegate,
+            extraManifestFlags: extraManifestFlags
+        )
+    }
+
     /// Loads a root manifest from a path using the resources associated with a particular `swiftc` executable.
     ///
     /// - Parameters:
@@ -177,8 +151,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 +687,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 +720,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 +730,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 +853,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 +864,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.

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

Sources/PackageLoading/UserManifestResources.swift renamed to Sources/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")
+    }
 }