Optionally use a per-package filesystem for PackageBuilder (swiftlang#3827)

neonichu · web-flow · commit bb2158d0f23a · 2021-12-10T21:18:08.000-08:00
Currently, we have the ability to provide a custom file system to the whole workspace and we partially use git-backed file system during package resolution. This change aims to allow using a custom file system when running `PackageBuilder` as well which e.g. allows to load a graph without having full checkouts of every package.

To utilize this feature, clients of `Workspace` can pass a custom `PackageContainerProvider` and use the new `CustomPackageContainer` protocol.
diff --git a/Sources/PackageGraph/GraphLoadingNode.swift b/Sources/PackageGraph/GraphLoadingNode.swift
@@ -28,16 +28,30 @@ public struct GraphLoadingNode: Equatable, Hashable {
     /// The product filter applied to the package.
     public let productFilter: ProductFilter
 
-    public init(identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter) {
+    /// The file system to use for loading the given package.
+    public let fileSystem: FileSystem
+
+    public init(identity: PackageIdentity, manifest: Manifest, productFilter: ProductFilter, fileSystem: FileSystem) {
         self.identity = identity
         self.manifest = manifest
         self.productFilter = productFilter
+        self.fileSystem = fileSystem
     }
 
     /// Returns the dependencies required by this node.
     internal func requiredDependencies() -> [PackageDependency] {
         return manifest.dependenciesRequired(for: productFilter)
     }
+
+    public func hash(into hasher: inout Hasher) {
+        hasher.combine(identity)
+        hasher.combine(manifest)
+        hasher.combine(productFilter)
+    }
+
+    public static func == (lhs: GraphLoadingNode, rhs: GraphLoadingNode) -> Bool {
+        return lhs.identity == rhs.identity && lhs.manifest == rhs.manifest && lhs.productFilter == rhs.productFilter
+    }
 }
 
 extension GraphLoadingNode: CustomStringConvertible {
diff --git a/Sources/PackageGraph/PackageContainer.swift b/Sources/PackageGraph/PackageContainer.swift
@@ -11,6 +11,8 @@
 import Basics
 import Dispatch
 import PackageModel
+import struct TSCBasic.AbsolutePath
+import protocol TSCBasic.FileSystem
 import struct TSCUtility.Version
 
 /// A container of packages.
@@ -103,6 +105,24 @@ extension PackageContainer {
     }
 }
 
+public protocol CustomPackageContainer: PackageContainer {
+    /// Retrieve the package using this package container.
+    func retrieve(
+       at version: Version,
+       progressHandler: ((_ bytesReceived: Int64, _ totalBytes: Int64?) -> Void)?,
+       observabilityScope: ObservabilityScope
+    ) throws -> AbsolutePath
+
+    /// Get the custom file system for this package container.
+    func getFileSystem() throws -> FileSystem?
+}
+
+public extension CustomPackageContainer {
+    func retrieve(at version: Version, observabilityScope: ObservabilityScope) throws -> AbsolutePath {
+        return try self.retrieve(at: version, progressHandler: .none, observabilityScope: observabilityScope)
+    }
+}
+
 // MARK: - PackageContainerConstraint
 
 /// An individual constraint onto a container.
diff --git a/Sources/PackageGraph/PackageGraph+Loading.swift b/Sources/PackageGraph/PackageGraph+Loading.swift
@@ -20,7 +20,7 @@ extension PackageGraph {
         root: PackageGraphRoot,
         identityResolver: IdentityResolver,
         additionalFileRules: [FileRuleDescription] = [],
-        externalManifests: OrderedDictionary<PackageIdentity, Manifest>,
+        externalManifests: OrderedDictionary<PackageIdentity, (manifest: Manifest, fs: FileSystem)>,
         requiredDependencies: Set<PackageReference> = [],
         unsafeAllowedPackages: Set<PackageReference> = [],
         binaryArtifacts: [BinaryArtifact] = [],
@@ -37,28 +37,28 @@ extension PackageGraph {
         var manifestMap = externalManifests
         // prefer roots
         root.manifests.forEach {
-            manifestMap[$0.key] = $0.value
+            manifestMap[$0.key] = ($0.value, fileSystem)
         }
 
         let successors: (GraphLoadingNode) -> [GraphLoadingNode] = { node in
             node.requiredDependencies().compactMap{ dependency in
-                return manifestMap[dependency.identity].map { manifest in
-                    GraphLoadingNode(identity: dependency.identity, manifest: manifest, productFilter: dependency.productFilter)
+                return manifestMap[dependency.identity].map { (manifest, fileSystem) in
+                    GraphLoadingNode(identity: dependency.identity, manifest: manifest, productFilter: dependency.productFilter, fileSystem: fileSystem)
                 }
             }
         }
 
         // Construct the root manifest and root dependencies set.
         let rootManifestSet = Set(root.manifests.values)
         let rootDependencies = Set(root.dependencies.compactMap{
-            manifestMap[$0.identity]
+            manifestMap[$0.identity]?.manifest
         })
         let rootManifestNodes = root.packages.map { identity, package in
-            GraphLoadingNode(identity: identity, manifest: package.manifest, productFilter: .everything)
+            GraphLoadingNode(identity: identity, manifest: package.manifest, productFilter: .everything, fileSystem: fileSystem)
         }
         let rootDependencyNodes = root.dependencies.lazy.compactMap { (dependency: PackageDependency) -> GraphLoadingNode? in
             manifestMap[dependency.identity].map {
-                GraphLoadingNode(identity: dependency.identity, manifest: $0, productFilter: dependency.productFilter)
+                GraphLoadingNode(identity: dependency.identity, manifest: $0.manifest, productFilter: dependency.productFilter, fileSystem: $0.fs)
             }
         }
         let inputManifests = rootManifestNodes + rootDependencyNodes
@@ -82,7 +82,8 @@ extension PackageGraph {
                 let merged = GraphLoadingNode(
                     identity: node.identity,
                     manifest: node.manifest,
-                    productFilter: existing.productFilter.union(node.productFilter)
+                    productFilter: existing.productFilter.union(node.productFilter),
+                    fileSystem: node.fileSystem
                 )
                 flattenedManifests[node.identity] = merged
             } else {
@@ -117,7 +118,7 @@ extension PackageGraph {
                     xcTestMinimumDeploymentTargets: xcTestMinimumDeploymentTargets,
                     shouldCreateMultipleTestProducts: shouldCreateMultipleTestProducts,
                     createREPLProduct: manifest.packageKind.isRoot ? createREPLProduct : false,
-                    fileSystem: fileSystem,
+                    fileSystem: node.fileSystem,
                     observabilityScope: nodeObservabilityScope
                 )
                 let package = try builder.construct()
diff --git a/Sources/SPMTestSupport/MockPackageContainer.swift b/Sources/SPMTestSupport/MockPackageContainer.swift
@@ -18,7 +18,7 @@ import XCTest
 
 import struct TSCUtility.Version
 
-public class MockPackageContainer: PackageContainer {
+public class MockPackageContainer: CustomPackageContainer {
     public typealias Constraint = PackageContainerConstraint
 
     public typealias Dependency = (container: PackageReference, requirement: PackageRequirement)
@@ -28,6 +28,8 @@ public class MockPackageContainer: PackageContainer {
     let dependencies: [String: [Dependency]]
     let filteredMode: Bool
     let filteredDependencies: [ProductFilter: [Dependency]]
+    let fileSystem: FileSystem?
+    let customRetrievalPath: AbsolutePath?
 
     public var unversionedDeps: [MockPackageContainer.Constraint] = []
 
@@ -81,6 +83,18 @@ public class MockPackageContainer: PackageContainer {
         return true
     }
 
+    public func retrieve(at version: Version, progressHandler: ((Int64, Int64?) -> Void)?, observabilityScope: ObservabilityScope) throws -> AbsolutePath {
+        if let path = customRetrievalPath {
+            return path
+        } else {
+            throw StringError("no path configured for mock package container")
+        }
+    }
+
+    public func getFileSystem() throws -> FileSystem? {
+        return fileSystem
+    }
+
     public convenience init(
         name: String,
         dependenciesByVersion: [Version: [(container: String, versionRequirement: VersionSetSpecifier)]]
@@ -100,13 +114,18 @@ public class MockPackageContainer: PackageContainer {
 
     public init(
         package: PackageReference,
-        dependencies: [String: [Dependency]] = [:]
+        dependencies: [String: [Dependency]] = [:],
+        fileSystem: FileSystem? = nil,
+        customRetrievalPath: AbsolutePath? = nil
     ) {
         self.package = package
         self._versions = dependencies.keys.compactMap(Version.init(_:)).sorted()
         self.dependencies = dependencies
         self.filteredMode = false
         self.filteredDependencies = [:]
+
+        self.fileSystem = fileSystem
+        self.customRetrievalPath = customRetrievalPath
     }
 
     public init(
@@ -128,6 +147,9 @@ public class MockPackageContainer: PackageContainer {
         self.dependencies = [:]
         self.filteredMode = true
         self.filteredDependencies = dependencies
+
+        self.fileSystem = nil
+        self.customRetrievalPath = nil
     }
 }
 
diff --git a/Sources/SPMTestSupport/MockWorkspace.swift b/Sources/SPMTestSupport/MockWorkspace.swift
@@ -28,6 +28,7 @@ public final class MockWorkspace {
     public let archiver: MockArchiver
     public let checksumAlgorithm: MockHashAlgorithm
     public let fingerprintStorage: MockPackageFingerprintStorage
+    public let customPackageContainerProvider: MockPackageContainerProvider?
     let roots: [MockPackage]
     let packages: [MockPackage]
     public let mirrors: DependencyMirrors
@@ -51,6 +52,7 @@ public final class MockWorkspace {
         customBinaryArchiver: MockArchiver? = .none,
         customChecksumAlgorithm: MockHashAlgorithm? = .none,
         customFingerprintStorage: MockPackageFingerprintStorage? = .none,
+        customPackageContainerProvider: MockPackageContainerProvider? = .none,
         resolverUpdateEnabled: Bool = true
     ) throws {
         let archiver = customBinaryArchiver ?? MockArchiver()
@@ -64,6 +66,7 @@ public final class MockWorkspace {
         self.fingerprintStorage = customFingerprintStorage ?? MockPackageFingerprintStorage()
         self.mirrors = mirrors ?? DependencyMirrors()
         self.identityResolver = DefaultIdentityResolver(locationMapper: self.mirrors.effectiveURL(for:))
+        self.customPackageContainerProvider = customPackageContainerProvider
         self.roots = roots
         self.packages = packages
 
@@ -115,7 +118,17 @@ public final class MockWorkspace {
             case .fileSystem(let path):
                 packagePath = basePath.appending(path)
             case .sourceControl(let url):
-                packagePath = basePath.appending(components: "sourceControl", url.absoluteString.spm_mangledToC99ExtendedIdentifier())
+                if let containerProvider = customPackageContainerProvider {
+                    let observability = ObservabilitySystem.makeForTesting()
+                    let packageRef = PackageReference(identity: PackageIdentity(url: url), kind: .remoteSourceControl(url))
+                    let container = try temp_await { containerProvider.getContainer(for: packageRef, skipUpdate: true, observabilityScope: observability.topScope, on: .sharedConcurrent, completion: $0) }
+                    guard let customContainer = container as? CustomPackageContainer else {
+                        throw StringError("invalid custom container: \(container)")
+                    }
+                    packagePath = try customContainer.retrieve(at: try Version(versionString: package.versions.first!!), observabilityScope: observability.topScope)
+                } else {
+                    packagePath = basePath.appending(components: "sourceControl", url.absoluteString.spm_mangledToC99ExtendedIdentifier())
+                }
             case .registry(let identity):
                 packagePath = basePath.appending(components: "registry", identity.description.spm_mangledToC99ExtendedIdentifier())
             }
@@ -233,6 +246,7 @@ public final class MockWorkspace {
             mirrors: self.mirrors,
             customToolsVersion: self.toolsVersion,
             customManifestLoader: self.manifestLoader,
+            customPackageContainerProvider: self.customPackageContainerProvider,
             customRepositoryProvider: self.repositoryProvider,
             customRegistryClient: self.registryClient,
             customIdentityResolver: self.identityResolver,
@@ -510,6 +524,7 @@ public final class MockWorkspace {
         case registryDownload(TSCUtility.Version)
         case edited(AbsolutePath?)
         case local
+        case custom(TSCUtility.Version, AbsolutePath)
     }
 
     public struct ManagedDependencyResult {
@@ -568,6 +583,11 @@ public final class MockWorkspace {
                     XCTFail("Expected local dependency", file: file, line: line)
                     return
                 }
+            case .custom(let currentVersion, let currentPath):
+                guard case .custom(let version, let path) = dependency.state else {
+                    return XCTFail("invalid dependency state \(dependency.state)", file: file, line: line)
+                }
+                XCTAssertTrue(currentVersion == version && currentPath == path, file: file, line: line)
             }
         }
     }
@@ -694,7 +714,7 @@ public final class MockWorkspace {
                     return XCTFail("invalid pin state \(pin.state)", file: file, line: line)
                 }
                 XCTAssertEqual(pinVersion, downloadVersion, file: file, line: line)
-            case .edited, .local:
+            case .edited, .local, .custom:
                 XCTFail("Unimplemented", file: file, line: line)
             }
         }
diff --git a/Sources/SPMTestSupport/misc.swift b/Sources/SPMTestSupport/misc.swift
@@ -231,8 +231,8 @@ public func loadPackageGraph(
     observabilityScope: ObservabilityScope
 ) throws -> PackageGraph {
     let rootManifests = manifests.filter { $0.packageKind.isRoot }.spm_createDictionary{ ($0.path, $0) }
-    let externalManifests = try manifests.filter { !$0.packageKind.isRoot }.reduce(into: OrderedDictionary<PackageIdentity, Manifest>()) { partial, item in
-        partial[try identityResolver.resolveIdentity(for: item.packageKind)] = item
+    let externalManifests = try manifests.filter { !$0.packageKind.isRoot }.reduce(into: OrderedDictionary<PackageIdentity, (manifest: Manifest, fs: FileSystem)>()) { partial, item in
+        partial[try identityResolver.resolveIdentity(for: item.packageKind)] = (item, fs)
     }
 
     let packages = Array(rootManifests.keys)
diff --git a/Sources/Workspace/Diagnostics.swift b/Sources/Workspace/Diagnostics.swift
@@ -127,6 +127,10 @@ extension Basics.Diagnostic {
         .warning("dependency '\(packageName)' is missing; downloading again")
     }
 
+    static func customDependencyMissing(packageName: String) -> Self {
+        .warning("dependency '\(packageName)' is missing; retrieving again")
+    }
+
     static func artifactChecksumChanged(targetName: String) -> Self {
         .error("artifact of binary target '\(targetName)' has changed checksum; this is a potential security risk so the new artifact won't be downloaded")
     }
diff --git a/Sources/Workspace/FileSystemPackageContainer.swift b/Sources/Workspace/FileSystemPackageContainer.swift
@@ -21,7 +21,7 @@ import TSCBasic
 /// There is no need to perform any git operations on such packages and they
 /// should be used as-is. In fact, they might not even have a git repository.
 /// Examples: Root packages, local dependencies, edited packages.
-internal struct FileSystemPackageContainer: PackageContainer {
+public struct FileSystemPackageContainer: PackageContainer {
     public let package: PackageReference
     private let identityResolver: IdentityResolver
     private let manifestLoader: ManifestLoaderProtocol
diff --git a/Sources/Workspace/ManagedDependency.swift b/Sources/Workspace/ManagedDependency.swift
@@ -39,6 +39,8 @@ extension Workspace {
             /// for top of the tree style development.
             case edited(basedOn: ManagedDependency?, unmanagedPath: AbsolutePath?)
 
+            case custom(version: Version, path: AbsolutePath)
+
             public var description: String {
                 switch self {
                 case .fileSystem(let path):
@@ -49,6 +51,8 @@ extension Workspace {
                     return "registryDownload (\(version))"
                 case .edited:
                     return "edited"
+                case .custom:
+                    return "custom"
                 }
             }
         }
diff --git a/Sources/Workspace/ResolverPrecomputationProvider.swift b/Sources/Workspace/ResolverPrecomputationProvider.swift
@@ -61,7 +61,7 @@ struct ResolverPrecomputationProvider: PackageContainerProvider {
     ) {
         queue.async {
             // Start by searching manifests from the Workspace's resolved dependencies.
-            if let manifest = self.dependencyManifests.dependencies.first(where: { _, managed, _ in managed.packageRef == package }) {
+            if let manifest = self.dependencyManifests.dependencies.first(where: { _, managed, _, _ in managed.packageRef == package }) {
                 let container = LocalPackageContainer(
                     package: package,
                     manifest: manifest.manifest,
diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift
diff --git a/Sources/Workspace/WorkspaceState.swift b/Sources/Workspace/WorkspaceState.swift
diff --git a/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift b/Tests/PackageGraphPerformanceTests/PackageGraphPerfTests.swift
diff --git a/Tests/WorkspaceTests/WorkspaceTests.swift b/Tests/WorkspaceTests/WorkspaceTests.swift

Original file line number	Diff line number	Diff line change
`@@ -127,6 +127,10 @@ extension Basics.Diagnostic {`
`127`	`127`	`.warning("dependency '\(packageName)' is missing; downloading again")`
`128`	`128`	`}`
`129`	`129`
	`130`	`+ static func customDependencyMissing(packageName: String) -> Self {`
	`131`	`+ .warning("dependency '\(packageName)' is missing; retrieving again")`
	`132`	`+ }`
	`133`	`+`
`130`	`134`	`static func artifactChecksumChanged(targetName: String) -> Self {`
`131`	`135`	`.error("artifact of binary target '\(targetName)' has changed checksum; this is a potential security risk so the new artifact won't be downloaded")`
`132`	`136`	`}`
Original file line number	Diff line number	Diff line change
`@@ -39,6 +39,8 @@ extension Workspace {`
`39`	`39`	`/// for top of the tree style development.`
`40`	`40`	`case edited(basedOn: ManagedDependency?, unmanagedPath: AbsolutePath?)`
`41`	`41`
	`42`	`+ case custom(version: Version, path: AbsolutePath)`
	`43`	`+`
`42`	`44`	`public var description: String {`
`43`	`45`	`switch self {`
`44`	`46`	`case .fileSystem(let path):`
`@@ -49,6 +51,8 @@ extension Workspace {`
`49`	`51`	`return "registryDownload (\(version))"`
`50`	`52`	`case .edited:`
`51`	`53`	`return "edited"`
	`54`	`+ case .custom:`
	`55`	`+ return "custom"`
`52`	`56`	`}`
`53`	`57`	`}`
`54`	`58`	`}`