Skip to content

Commit 809e35a

Browse files
hartbitaciidgh
authored andcommitted
Resolved review comments.
1 parent df9088e commit 809e35a

File tree

2 files changed

+25
-50
lines changed

2 files changed

+25
-50
lines changed

Sources/Workspace/LocalContainerProvider.swift renamed to Sources/Workspace/ResolverPrecomputationProvider.swift

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import SourceControl
1717
/// PackageContainerProvider implementation used by Workspace to do a dependency pre-calculation using the cached
1818
/// dependency information (Workspace.DependencyManifests) to check if dependency resolution is required before
1919
/// performing a full resolution.
20-
struct LocalContainerProvider: PackageContainerProvider {
20+
struct ResolverPrecomputationProvider: PackageContainerProvider {
2121
/// The package graph inputs.
2222
let root: PackageGraphRoot
2323

@@ -65,6 +65,7 @@ struct LocalContainerProvider: PackageContainerProvider {
6565
}
6666

6767
// Continue searching from the Workspace's root manifests.
68+
// FIXME: We might want to use a dictionary for faster lookups.
6869
if let index = dependencyManifests.root.packageRefs.firstIndex(of: identifier) {
6970
let container = LocalPackageContainer(
7071
manifest: dependencyManifests.root.manifests[index],
@@ -83,10 +84,12 @@ struct LocalContainerProvider: PackageContainerProvider {
8384

8485
private struct LocalPackageContainer: PackageContainer {
8586
let manifest: Manifest
87+
/// The managed dependency if the package is not a root package.
8688
let dependency: ManagedDependency?
8789
let config: SwiftPMConfig
8890
let currentToolsVersion: ToolsVersion
8991

92+
// Gets the package reference from the managed dependency or computes it for root packages.
9093
var identifier: PackageReference {
9194
if let identifier = dependency?.packageRef {
9295
return identifier
@@ -122,14 +125,29 @@ private struct LocalPackageContainer: PackageContainer {
122125
}
123126

124127
func getDependencies(at version: Version) throws -> [PackageContainerConstraint] {
128+
// Throw an error when the dependency is not at the correct version to fail resolution.
129+
guard dependency?.checkoutState?.version == version else {
130+
throw Diagnostics.fatalError
131+
}
132+
125133
return manifest.dependencyConstraints(config: config)
126134
}
127135

128136
func getDependencies(at revision: String) throws -> [PackageContainerConstraint] {
137+
// Throw an error when the dependency is not at the correct revision to fail resolution.
138+
guard dependency?.checkoutState?.revision.identifier == revision else {
139+
throw Diagnostics.fatalError
140+
}
141+
129142
return manifest.dependencyConstraints(config: config)
130143
}
131144

132145
func getUnversionedDependencies() throws -> [PackageContainerConstraint] {
146+
// Throw an error when the dependency is not unversioned to fail resolution.
147+
guard dependency?.state.isCheckout != true else {
148+
throw Diagnostics.fatalError
149+
}
150+
133151
return manifest.dependencyConstraints(config: config)
134152
}
135153

Sources/Workspace/Workspace.swift

Lines changed: 6 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1429,64 +1429,21 @@ extension Workspace {
14291429
extraConstraints
14301430

14311431
let diagnostics = DiagnosticsEngine()
1432-
let localProvider = LocalContainerProvider(
1432+
let precomputationProvider = ResolverPrecomputationProvider(
14331433
root: root,
14341434
dependencyManifests: dependencyManifests,
14351435
config: config,
14361436
diagnostics: diagnostics
14371437
)
14381438

1439-
let resolver = PubgrubDependencyResolver(localProvider)
1439+
let resolver = PubgrubDependencyResolver(precomputationProvider)
14401440
let result = resolver.solve(dependencies: constraints, pinsStore: pinsStore)
14411441

1442-
guard !diagnostics.hasErrors else { return true }
1443-
guard case .success(let bindings) = result else { return true }
1444-
1445-
// Otherwise, check checkouts and pins.
1446-
for (container, boundVersion) in bindings {
1447-
let url = container.path
1448-
let dependencyState = managedDependencies[forURL: url]?.state
1449-
let pinState = pinsStore.pinsMap[container.identity]?.state
1450-
1451-
switch (boundVersion, dependencyState, pinState) {
1452-
// If the package is excluded, but it is a dependency or a pin, we need to re-resolve.
1453-
case (.excluded, .some, _),
1454-
(.excluded, _, .some):
1455-
return true
1456-
1457-
case (_, .checkout(let checkoutState)?, _):
1458-
// If this constraint is not same as the checkout state, we need to re-resolve.
1459-
if boundVersion.description != checkoutState.description {
1460-
return true
1461-
}
1462-
1463-
// Ensure that the pin is not out of sync.
1464-
if checkoutState != pinState {
1465-
return true
1466-
}
1467-
1468-
// We have a local package but the requirement is now different.
1469-
case (.version, .local?, _),
1470-
(.revision, .local?, _):
1471-
return true
1472-
1473-
case (.unversioned, .local?, _):
1474-
continue
1475-
1476-
case (_, .edited?, _):
1477-
continue
1478-
1479-
case (_, nil, _):
1480-
// Ignore root packages.
1481-
if root.packageRefs.contains(container) {
1482-
continue
1483-
}
1484-
// We don't have a checkout.
1485-
return true
1486-
}
1442+
if !diagnostics.hasErrors, case .success = result {
1443+
return false
1444+
} else {
1445+
return true
14871446
}
1488-
1489-
return false
14901447
}
14911448

14921449
/// Validates that each checked out managed dependency has an entry in pinsStore.

0 commit comments

Comments
 (0)