diff --git a/Sources/Workspace/PinsStore.swift b/Sources/Workspace/PinsStore.swift index fd2449c2e5f..7627f39430a 100644 --- a/Sources/Workspace/PinsStore.swift +++ b/Sources/Workspace/PinsStore.swift @@ -99,6 +99,10 @@ public final class PinsStore { pinsMap[pin.packageRef.identity] = pin } + func remove(identity: String) { + pinsMap[identity] = nil + } + /// Pin a managed dependency at its checkout state. /// /// This method does nothing if the dependency is in edited state. diff --git a/Sources/Workspace/Workspace.swift b/Sources/Workspace/Workspace.swift index 51232d5db4e..de4a12b6958 100644 --- a/Sources/Workspace/Workspace.swift +++ b/Sources/Workspace/Workspace.swift @@ -1236,21 +1236,27 @@ extension Workspace { return currentManifests } - validatePinsStore(dependencyManifests: currentManifests, diagnostics: diagnostics) - - // Abort if pinsStore is unloadable or if diagnostics has errors. - guard !diagnostics.hasErrors, let pinsStore = diagnostics.wrap({ try pinsStore.load() }) else { + // Load the pinstore and check if its outdated. + guard let pinsStore = diagnostics.wrap({ try pinsStore.load() }) else { return currentManifests } + let isPinStoreOutdated = pinsStore.isOutdated( + dependencyManifests: currentManifests, + managedDependencies: managedDependencies + ) - // Compute the missing package identities. + // Compute the missing package identities and scrub any out-of-date entry from the pin store.. let missingPackageURLs = currentManifests.missingPackageURLs() + for missingURL in missingPackageURLs { + pinsStore.remove(identity: missingURL.identity) + try? pinsStore.saveState() + } // The pins to use in case we need to run the resolution. var validPins = pinsStore.createConstraints() // Compute if we need to run the resolver. We always run the resolver if - // there are extra constraints. + // there are missing packages. if missingPackageURLs.isEmpty { // Use root constraints, dependency manifest constraints and extra // constraints to compute if a new resolution is required. @@ -1263,9 +1269,11 @@ extension Workspace { let result = isResolutionRequired(root: graphRoot, dependencies: dependencies, pinsStore: pinsStore) - // If we don't need resolution and there are no extra constraints, - // just validate pinsStore and return. - if !result.resolve && extraConstraints.isEmpty { + // We don't need to resolve if: + // 1. Nothing in the manifest changed. + // 2. PinStore is valid. + // 3. We have no extra constraints. + if !result.resolve && extraConstraints.isEmpty && !isPinStoreOutdated { return currentManifests } @@ -1442,38 +1450,7 @@ extension Workspace { } } - return (false, []) - } - - /// Validates that each checked out managed dependency has an entry in pinsStore. - private func validatePinsStore(dependencyManifests: DependencyManifests, diagnostics: DiagnosticsEngine) { - guard let pinsStore = diagnostics.wrap({ try pinsStore.load() }) else { - return - } - - let pins = pinsStore.pinsMap.keys - let requiredURLs = dependencyManifests.computePackageURLs().required - - for dependency in managedDependencies.values { - switch dependency.state { - case .checkout: break - case .edited, .local: continue - } - - let identity = dependency.packageRef.identity - - if requiredURLs.contains(where: { $0.path == dependency.packageRef.path }) { - // If required identity contains this dependency, it should be in the pins store. - if let pin = pinsStore.pinsMap[identity], pin.packageRef.path == dependency.packageRef.path { - continue - } - } else if !pins.contains(identity) { - // Otherwise, it should *not* be in the pins store. - continue - } - - return self.pinAll(dependencyManifests: dependencyManifests, pinsStore: pinsStore, diagnostics: diagnostics) - } + return (false, validPins) } /// This enum represents state of an external package. @@ -1994,3 +1971,36 @@ public final class LoadableResult { return try loadResult().dematerialize() } } + +extension PinsStore { + /// Returns true if the pin store is outdated with the content in the managed dependencies. + func isOutdated( + dependencyManifests: Workspace.DependencyManifests, + managedDependencies: ManagedDependencies + ) -> Bool { + let pins = pinsMap.keys + let requiredURLs = dependencyManifests.computePackageURLs().required + + for dependency in managedDependencies.values { + switch dependency.state { + case .checkout: break + case .edited, .local: continue + } + + let identity = dependency.packageRef.identity + + if requiredURLs.contains(where: { $0.path == dependency.packageRef.path }) { + // If required identity contains this dependency, it should be in the pins store. + if let pin = pinsMap[identity], pin.packageRef.path == dependency.packageRef.path { + continue + } + } else if !pins.contains(identity) { + // Otherwise, it should *not* be in the pins store. + continue + } + + return true + } + return false + } +} diff --git a/Tests/WorkspaceTests/WorkspaceTests.swift b/Tests/WorkspaceTests/WorkspaceTests.swift index 0e15ebd5038..59edd3fd3c8 100644 --- a/Tests/WorkspaceTests/WorkspaceTests.swift +++ b/Tests/WorkspaceTests/WorkspaceTests.swift @@ -861,7 +861,7 @@ final class WorkspaceTests: XCTestCase { ], pinsStore: pinsStore) XCTAssertEqual(result.resolve, false) - XCTAssertEqual(result.validPins, []) + XCTAssertEqual(result.validPins.map({$0.identifier.repository.url}).sorted(), ["/A", "/B", "/C"]) } } @@ -2575,7 +2575,7 @@ final class WorkspaceTests: XCTestCase { XCTAssertNoDiagnostics(diagnostics) } workspace.checkManagedDependencies() { result in - result.check(dependency: "foo", at: .checkout(.version("1.0.0"))) + result.check(notPresent: "foo") } workspace.checkResolved() { result in result.check(notPresent: "foo") @@ -3048,6 +3048,107 @@ final class WorkspaceTests: XCTestCase { } } } + + func testUpdateToResolvedFileResolvesCorrectly() throws { + let sandbox = AbsolutePath("/tmp/ws/") + let fs = InMemoryFileSystem() + + let workspace = try TestWorkspace( + sandbox: sandbox, + fs: fs, + roots: [ + TestPackage( + name: "Root", + targets: [ + TestTarget(name: "Root", dependencies: ["Bar"]), + ], + products: [], + dependencies: [ + TestDependency(name: "Bar", requirement: .upToNextMajor(from: "1.0.0")), + ] + ), + ], + packages: [ + TestPackage( + name: "Bar", + targets: [ + TestTarget(name: "Bar", dependencies: ["Foo"]), + ], + products: [ + TestProduct(name: "Bar", targets: ["Bar"]), + ], + dependencies: [ + TestDependency(name: "Foo", requirement: .upToNextMajor(from: "1.0.0")), + ], + versions: ["1.0.0"] + ), + TestPackage( + name: "Bar", + targets: [ + TestTarget(name: "Bar", dependencies: []), + ], + products: [ + TestProduct(name: "Bar", targets: ["Bar"]), + ], + dependencies: [ + ], + versions: ["1.1.0"] + ), + TestPackage( + name: "Foo", + targets: [ + TestTarget(name: "Foo"), + ], + products: [ + TestProduct(name: "Foo", targets: ["Foo"]), + ], + versions: ["1.0.0"] + ), + ] + ) + + let deps: [TestWorkspace.PackageDependency] = [ + .init(name: "Bar", requirement: .exact("1.0.0")), + ] + workspace.checkPackageGraph(roots: ["Root"], deps: deps) { (graph, diagnostics) in + PackageGraphTester(graph) { result in + result.check(roots: "Root") + result.check(packages: "Bar", "Foo", "Root") + } + XCTAssertNoDiagnostics(diagnostics) + } + workspace.checkManagedDependencies() { result in + result.check(dependency: "foo", at: .checkout(.version("1.0.0"))) + result.check(dependency: "bar", at: .checkout(.version("1.0.0"))) + } + + // Emulate a complete change in Package.resolved file. + do { + let ws = workspace.createWorkspace() + let pinsStore = try ws.pinsStore.load() + let barPin = pinsStore.pins.first(where: { $0.packageRef.identity == "bar" })! + + let barRepo = workspace.repoProvider.specifierMap[RepositorySpecifier(url: barPin.packageRef.path)]! + let revision = try barRepo.resolveRevision(tag: "1.1.0") + let newState = CheckoutState(revision: revision, version: "1.1.0") + + pinsStore.unpinAll() + pinsStore.pin(packageRef: barPin.packageRef, state: newState) + try pinsStore.saveState() + } + + workspace.checkPackageGraph(roots: ["Root"]) { (graph, diagnostics) in + PackageGraphTester(graph) { result in + result.check(roots: "Root") + result.check(packages: "Bar", "Root") + } + XCTAssertNoDiagnostics(diagnostics) + } + workspace.checkManagedDependencies() { result in + result.check(notPresent: "foo") + result.check(dependency: "bar", at: .checkout(.version("1.1.0"))) + } + } } extension PackageGraph { diff --git a/Tests/WorkspaceTests/XCTestManifests.swift b/Tests/WorkspaceTests/XCTestManifests.swift index fab435ee63b..80a8a41008b 100644 --- a/Tests/WorkspaceTests/XCTestManifests.swift +++ b/Tests/WorkspaceTests/XCTestManifests.swift @@ -85,6 +85,7 @@ extension WorkspaceTests { ("testToolsVersionRootPackages", testToolsVersionRootPackages), ("testTransitiveDependencySwitchWithSameIdentity", testTransitiveDependencySwitchWithSameIdentity), ("testUpdate", testUpdate), + ("testUpdateToResolvedFileResolvesCorrectly", testUpdateToResolvedFileResolvesCorrectly), ] }