Skip to content

Commit 0eab459

Browse files
matttmattt
committed
Implement PackageContainerProvider in Workspace directly
1 parent ab90ad4 commit 0eab459

File tree

7 files changed

+121
-133
lines changed

7 files changed

+121
-133
lines changed

‎Sources/PackageGraph/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ add_library(PackageGraph
1616
GraphLoadingNode.swift
1717
LocalPackageContainer.swift
1818
PackageContainer.swift
19+
PackageContainerProvider.swift
1920
PackageGraph.swift
2021
PackageGraph+Loading.swift
2122
PackageGraphRoot.swift

‎Sources/PackageGraph/PackageContainer.swift

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,3 @@ extension PackageContainerConstraint: CustomStringConvertible {
138138
return "Constraint(\(self.package), \(requirement), \(products)"
139139
}
140140
}
141-
142-
// MARK: -
143-
144-
/// An interface for resolving package containers.
145-
public protocol PackageContainerProvider {
146-
/// Get the container for a particular identifier asynchronously.
147-
func getContainer(
148-
for package: PackageReference,
149-
skipUpdate: Bool,
150-
on queue: DispatchQueue,
151-
completion: @escaping (Result<PackageContainer, Swift.Error>) -> Void
152-
)
153-
}

‎Sources/PackageGraph/PackageContainerProvider.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
This source file is part of the Swift.org open source project
3+
4+
Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors
5+
Licensed under Apache License v2.0 with Runtime Library Exception
6+
7+
See http://swift.org/LICENSE.txt for license information
8+
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
9+
*/
10+
11+
import Dispatch
12+
import Basics
13+
import TSCBasic
14+
import PackageLoading
15+
import PackageModel
16+
import SourceControl
17+
import TSCUtility
18+
19+
/// An interface for resolving package containers.
20+
public protocol PackageContainerProvider {
21+
/// Get the container for a particular identifier asynchronously.
22+
func getContainer(
23+
for package: PackageReference,
24+
skipUpdate: Bool,
25+
on queue: DispatchQueue,
26+
completion: @escaping (Result<PackageContainer, Swift.Error>) -> Void
27+
)
28+
}

‎Sources/PackageGraph/RegistryPackageContainer.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public class RegistryPackageContainer: PackageContainer {
3131
private var validToolsVersionsCache = ThreadSafeKeyValueStore<Version, Bool>()
3232
private var manifestsCache = ThreadSafeKeyValueStore<Version, Manifest>()
3333

34-
init(
34+
public init(
3535
package: PackageReference,
3636
identityResolver: IdentityResolver,
3737
manager: RegistryManager,

‎Sources/PackageGraph/RepositoryPackageContainer.swift

Lines changed: 3 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public class RepositoryPackageContainer: PackageContainer, CustomStringConvertib
7474
/// valid or not.
7575
internal var validToolsVersionsCache = ThreadSafeKeyValueStore<Version, Bool>()
7676

77-
init(
77+
public init(
7878
package: PackageReference,
7979
identityResolver: IdentityResolver,
8080
repository: Repository,
@@ -325,88 +325,9 @@ public class RepositoryPackageContainer: PackageContainer, CustomStringConvertib
325325
}
326326
}
327327

328-
/// Adaptor for exposing repositories as PackageContainerProvider instances.
329-
///
330-
/// This is the root class for bridging the manifest & SCM systems into the
331-
/// interfaces used by the `DependencyResolver` algorithm.
332-
public class RepositoryPackageContainerProvider: PackageContainerProvider {
333-
let repositoryManager: RepositoryManager
334-
let manifestLoader: ManifestLoaderProtocol
335-
let identityResolver: IdentityResolver
336-
337-
/// The tools version currently in use. Only the container versions less than and equal to this will be provided by
338-
/// the container.
339-
let currentToolsVersion: ToolsVersion
340-
341-
/// The tools version loader.
342-
let toolsVersionLoader: ToolsVersionLoaderProtocol
343-
344-
/// Create a repository-based package provider.
345-
///
346-
/// - Parameters:
347-
/// - repositoryManager: The repository manager responsible for providing repositories.
348-
/// - manifestLoader: The manifest loader instance.
349-
/// - currentToolsVersion: The current tools version in use.
350-
/// - toolsVersionLoader: The tools version loader.
351-
public init(
352-
repositoryManager: RepositoryManager,
353-
identityResolver: IdentityResolver,
354-
manifestLoader: ManifestLoaderProtocol,
355-
currentToolsVersion: ToolsVersion = ToolsVersion.currentToolsVersion,
356-
toolsVersionLoader: ToolsVersionLoaderProtocol = ToolsVersionLoader()
357-
) {
358-
self.repositoryManager = repositoryManager
359-
self.identityResolver = identityResolver
360-
self.manifestLoader = manifestLoader
361-
self.currentToolsVersion = currentToolsVersion
362-
self.toolsVersionLoader = toolsVersionLoader
363-
}
364-
365-
public func getContainer(
366-
for package: PackageReference,
367-
skipUpdate: Bool,
368-
on queue: DispatchQueue,
369-
completion: @escaping (Result<PackageContainer, Swift.Error>) -> Void
370-
) {
371-
// If the container is local, just create and return a local package container.
372-
if package.kind != .remote {
373-
return queue.async {
374-
let container = LocalPackageContainer(
375-
package: package,
376-
identityResolver: self.identityResolver,
377-
manifestLoader: self.manifestLoader,
378-
toolsVersionLoader: self.toolsVersionLoader,
379-
currentToolsVersion: self.currentToolsVersion,
380-
fileSystem: self.repositoryManager.fileSystem)
381-
completion(.success(container))
382-
}
383-
}
384-
385-
// Resolve the container using the repository manager.
386-
repositoryManager.lookup(repository: package.repository, skipUpdate: skipUpdate, on: queue) { result in
387-
queue.async {
388-
// Create the container wrapper.
389-
let result = result.tryMap { handle -> PackageContainer in
390-
// Open the repository.
391-
//
392-
// FIXME: Do we care about holding this open for the lifetime of the container.
393-
let repository = try handle.open()
394-
return RepositoryPackageContainer(
395-
package: package,
396-
identityResolver: self.identityResolver,
397-
repository: repository,
398-
manifestLoader: self.manifestLoader,
399-
toolsVersionLoader: self.toolsVersionLoader,
400-
currentToolsVersion: self.currentToolsVersion
401-
)
402-
}
403-
completion(result)
404-
}
405-
}
406-
}
407-
}
328+
// MARK: -
408329

409-
extension Git {
330+
private extension Git {
410331
static func convertTagsToVersionMap(_ tags: [String]) -> [Version: [String]] {
411332
// First, check if we need to restrict the tag set to version-specific tags.
412333
var knownVersions: [Version: [String]] = [:]

‎Sources/Workspace/Workspace.swift

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,6 @@ public class Workspace {
202202
/// Utility to resolve package identifiers
203203
public let identityResolver: IdentityResolver
204204

205-
/// The package container provider.
206-
fileprivate let containerProvider: RepositoryPackageContainerProvider
207-
208205
/// The http client used for downloading binary artifacts.
209206
fileprivate let httpClient: HTTPClient
210207

@@ -309,13 +306,6 @@ public class Workspace {
309306

310307
self.identityResolver = identityResolver ?? DefaultIdentityResolver(locationMapper: config.mirrors.effectiveURL(for:))
311308

312-
self.containerProvider = RepositoryPackageContainerProvider(
313-
repositoryManager: repositoryManager,
314-
identityResolver: self.identityResolver,
315-
manifestLoader: manifestLoader,
316-
currentToolsVersion: currentToolsVersion,
317-
toolsVersionLoader: toolsVersionLoader
318-
)
319309
self.fileSystem = fileSystem
320310

321311
self.pinsStore = LoadableResult {
@@ -1032,6 +1022,53 @@ fileprivate extension PinsStore {
10321022
}
10331023
}
10341024

1025+
// MARK: - PackageContainerProvider
1026+
1027+
extension Workspace: PackageContainerProvider {
1028+
public func getContainer(
1029+
for package: PackageReference,
1030+
skipUpdate: Bool,
1031+
on queue: DispatchQueue,
1032+
completion: @escaping (Result<PackageContainer, Swift.Error>) -> Void
1033+
) {
1034+
// If the container is local, just create and return a local package container.
1035+
if package.kind != .remote {
1036+
return queue.async {
1037+
let container = LocalPackageContainer(
1038+
package: package,
1039+
identityResolver: self.identityResolver,
1040+
manifestLoader: self.manifestLoader,
1041+
toolsVersionLoader: self.toolsVersionLoader,
1042+
currentToolsVersion: self.currentToolsVersion,
1043+
fileSystem: self.repositoryManager.fileSystem)
1044+
completion(.success(container))
1045+
}
1046+
}
1047+
1048+
// Resolve the container using the repository manager.
1049+
repositoryManager.lookup(repository: package.repository, skipUpdate: skipUpdate, on: queue) { result in
1050+
queue.async {
1051+
// Create the container wrapper.
1052+
let result = result.tryMap { handle -> PackageContainer in
1053+
// Open the repository.
1054+
//
1055+
// FIXME: Do we care about holding this open for the lifetime of the container.
1056+
let repository = try handle.open()
1057+
return RepositoryPackageContainer(
1058+
package: package,
1059+
identityResolver: self.identityResolver,
1060+
repository: repository,
1061+
manifestLoader: self.manifestLoader,
1062+
toolsVersionLoader: self.toolsVersionLoader,
1063+
currentToolsVersion: self.currentToolsVersion
1064+
)
1065+
}
1066+
completion(result)
1067+
}
1068+
}
1069+
}
1070+
}
1071+
10351072
// MARK: - TSCUtility Functions
10361073

10371074
extension Workspace {
@@ -1808,7 +1845,7 @@ extension Workspace {
18081845
// We just request the packages here, repository manager will
18091846
// automatically manage the parallelism.
18101847
for pin in pinsStore.pins {
1811-
containerProvider.getContainer(for: pin.packageRef, skipUpdate: true, on: .sharedConcurrent, completion: { _ in })
1848+
getContainer(for: pin.packageRef, skipUpdate: true, on: .sharedConcurrent, completion: { _ in })
18121849
}
18131850

18141851
// Compute the pins that we need to actually clone.
@@ -2245,7 +2282,7 @@ extension Workspace {
22452282
// Get the latest revision from the container.
22462283
// TODO: replace with async/await when available
22472284
guard let container = (try temp_await {
2248-
containerProvider.getContainer(for: packageRef, skipUpdate: true, on: .sharedConcurrent, completion: $0)
2285+
getContainer(for: packageRef, skipUpdate: true, on: .sharedConcurrent, completion: $0)
22492286
}) as? RepositoryPackageContainer else {
22502287
throw InternalError("invalid container for \(packageRef) expected a RepositoryPackageContainer")
22512288
}
@@ -2313,7 +2350,7 @@ extension Workspace {
23132350
let delegate = !delegates.isEmpty ? MultiplexResolverDelegate(delegates) : nil
23142351

23152352
return PubgrubDependencyResolver(
2316-
provider: containerProvider,
2353+
provider: self,
23172354
pinsMap: pinsMap,
23182355
isPrefetchingEnabled: isResolverPrefetchingEnabled,
23192356
skipUpdate: skipUpdate,
@@ -2555,7 +2592,7 @@ extension Workspace {
25552592
// this?
25562593
// FIXME: this should not block
25572594
guard let container = (try temp_await {
2558-
containerProvider.getContainer(for: package, skipUpdate: true, on: .sharedConcurrent, completion: $0)
2595+
getContainer(for: package, skipUpdate: true, on: .sharedConcurrent, completion: $0)
25592596
}) as? RepositoryPackageContainer else {
25602597
throw InternalError("invalid container for \(package) expected a RepositoryPackageContainer")
25612598
}

‎Tests/PackageGraphTests/RepositoryPackageContainerProviderTests.swift

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import PackageModel
1717
import SourceControl
1818
import TSCBasic
1919
import TSCUtility
20+
import Workspace
2021

2122
import SPMTestSupport
2223

@@ -182,11 +183,7 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
182183
fileSystem: fs
183184
)
184185

185-
let provider = RepositoryPackageContainerProvider(
186-
repositoryManager: repositoryManager,
187-
identityResolver: DefaultIdentityResolver(),
188-
manifestLoader: MockManifestLoader(manifests: [:])
189-
)
186+
let provider = Workspace.create(forRootPackage: .root, manifestLoader: MockManifestLoader(manifests: [:]), repositoryManager: repositoryManager, delegate: nil, identityResolver: DefaultIdentityResolver())
190187

191188
let ref = PackageReference.remote(identity: PackageIdentity(path: repoPath), location: repoPath.pathString)
192189
let container = try provider.getContainer(for: ref, skipUpdate: false)
@@ -233,12 +230,16 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
233230
fileSystem: fs
234231
)
235232

236-
func createProvider(_ currentToolsVersion: ToolsVersion) -> RepositoryPackageContainerProvider {
237-
return RepositoryPackageContainerProvider(
238-
repositoryManager: repositoryManager,
239-
identityResolver: DefaultIdentityResolver(),
233+
func createProvider(_ currentToolsVersion: ToolsVersion) -> PackageContainerProvider {
234+
Workspace(
235+
dataPath: .root.appending(component: ".build"),
236+
editablesPath: .root.appending(component: "Packages"),
237+
pinsFile: .root.appending(component: "Package.resolved"),
240238
manifestLoader: MockManifestLoader(manifests: [:]),
241-
currentToolsVersion: currentToolsVersion
239+
repositoryManager: repositoryManager,
240+
currentToolsVersion: currentToolsVersion,
241+
delegate: nil,
242+
identityResolver: DefaultIdentityResolver()
242243
)
243244
}
244245

@@ -317,10 +318,12 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
317318
fileSystem: fs
318319
)
319320

320-
let provider = RepositoryPackageContainerProvider(
321+
let provider = Workspace.create(
322+
forRootPackage: .root,
323+
manifestLoader: MockManifestLoader(manifests: [:]),
321324
repositoryManager: repositoryManager,
322-
identityResolver: DefaultIdentityResolver(),
323-
manifestLoader: MockManifestLoader(manifests: [:])
325+
delegate: nil,
326+
identityResolver: DefaultIdentityResolver()
324327
)
325328
let ref = PackageReference.remote(identity: PackageIdentity(path: repoPath), location: repoPath.pathString)
326329
let container = try provider.getContainer(for: ref, skipUpdate: false)
@@ -358,11 +361,14 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
358361
fileSystem: fs
359362
)
360363

361-
let provider = RepositoryPackageContainerProvider(
364+
let provider = Workspace.create(
365+
forRootPackage: .root,
366+
manifestLoader: MockManifestLoader(manifests: [:]),
362367
repositoryManager: repositoryManager,
363-
identityResolver: DefaultIdentityResolver(),
364-
manifestLoader: MockManifestLoader(manifests: [:])
368+
delegate: nil,
369+
identityResolver: DefaultIdentityResolver()
365370
)
371+
366372
let ref = PackageReference.remote(identity: PackageIdentity(path: repoPath), location: repoPath.pathString)
367373
let container = try provider.getContainer(for: ref, skipUpdate: false)
368374
let v = try container.toolsVersionsAppropriateVersionsDescending().map { $0 }
@@ -546,9 +552,14 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
546552
try TargetDescription(name: packageDir.basename, path: packageDir.pathString),
547553
]
548554
)
549-
let containerProvider = RepositoryPackageContainerProvider(repositoryManager: repositoryManager,
550-
identityResolver: DefaultIdentityResolver(),
551-
manifestLoader: MockManifestLoader(manifests: [.init(url: packageDir.pathString, version: nil): manifest]))
555+
556+
let containerProvider = Workspace.create(
557+
forRootPackage: packageDir,
558+
manifestLoader: MockManifestLoader(manifests: [.init(url: packageDir.pathString, version: nil): manifest]),
559+
repositoryManager: repositoryManager,
560+
delegate: nil,
561+
identityResolver: DefaultIdentityResolver()
562+
)
552563

553564
// Get a hold of the container for the test package.
554565
let packageRef = PackageReference.remote(identity: PackageIdentity(path: packageDir), location: packageDir.pathString)
@@ -621,12 +632,15 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
621632
),
622633
]
623634
)
624-
let containerProvider = RepositoryPackageContainerProvider(
625-
repositoryManager: repositoryManager,
626-
identityResolver: DefaultIdentityResolver(),
635+
636+
let containerProvider = Workspace.create(
637+
forRootPackage: .root,
627638
manifestLoader: MockManifestLoader(
628639
manifests: [.init(url: packageDirectory.pathString, version: Version(1, 0, 0)): manifest]
629-
)
640+
),
641+
repositoryManager: repositoryManager,
642+
delegate: nil,
643+
identityResolver: DefaultIdentityResolver()
630644
)
631645

632646
let packageReference = PackageReference.remote(identity: PackageIdentity(path: packageDirectory), location: packageDirectory.pathString)

0 commit comments

Comments
 (0)