Skip to content

Commit

Permalink
Revert "[Workspace] Improve how we handle changes to Package.resolved"
Browse files Browse the repository at this point in the history 
This reverts commit 32e144d.

<rdar://problem/53085717>

(cherry picked from commit 35e7c91)
  • Loading branch information
@ankitspd
ankitspd committed Jul 16, 2019
1 parent f07afcf commit 39b444c
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 159 deletions.
4 changes: 0 additions & 4 deletions Sources/Workspace/PinsStore.swift
View file
Expand Up @@ -99,10 +99,6 @@ 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.
Expand Down
92 changes: 41 additions & 51 deletions Sources/Workspace/Workspace.swift
View file
Expand Up @@ -1236,27 +1236,21 @@ extension Workspace {
return currentManifests
}

// Load the pinstore and check if its outdated.
guard let pinsStore = diagnostics.wrap({ try pinsStore.load() }) else {
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 {
return currentManifests
}
let isPinStoreOutdated = pinsStore.isOutdated(
dependencyManifests: currentManifests,
managedDependencies: managedDependencies
)

// Compute the missing package identities and scrub any out-of-date entry from the pin store..
// Compute the missing package identities.
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 missing packages.
// there are extra constraints.
if missingPackageURLs.isEmpty {
// Use root constraints, dependency manifest constraints and extra
// constraints to compute if a new resolution is required.
Expand All @@ -1269,11 +1263,9 @@ extension Workspace {

let result = isResolutionRequired(root: graphRoot, dependencies: dependencies, pinsStore: pinsStore)

// 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 {
// If we don't need resolution and there are no extra constraints,
// just validate pinsStore and return.
if !result.resolve && extraConstraints.isEmpty {
return currentManifests
}

Expand Down Expand Up @@ -1472,7 +1464,38 @@ extension Workspace {
}
}

return (false, validPins)
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)
}
}

/// This enum represents state of an external package.
Expand Down Expand Up @@ -1993,36 +2016,3 @@ public final class LoadableResult<Value> {
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
}
}
105 changes: 2 additions & 103 deletions Tests/WorkspaceTests/WorkspaceTests.swift
View file
Expand Up @@ -861,7 +861,7 @@ final class WorkspaceTests: XCTestCase {
], pinsStore: pinsStore)

XCTAssertEqual(result.resolve, false)
XCTAssertEqual(result.validPins.map({$0.identifier.repository.url}).sorted(), ["/A", "/B", "/C"])
XCTAssertEqual(result.validPins, [])
}
}

Expand Down Expand Up @@ -2575,7 +2575,7 @@ final class WorkspaceTests: XCTestCase {
XCTAssertNoDiagnostics(diagnostics)
}
workspace.checkManagedDependencies() { result in
result.check(notPresent: "foo")
result.check(dependency: "foo", at: .checkout(.version("1.0.0")))
}
workspace.checkResolved() { result in
result.check(notPresent: "foo")
Expand Down Expand Up @@ -3049,107 +3049,6 @@ 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")))
}
}

func testRootPackagesOverrideBasenameMismatch() throws {
let sandbox = AbsolutePath("/tmp/ws/")
let fs = InMemoryFileSystem()
Expand Down
1 change: 0 additions & 1 deletion Tests/WorkspaceTests/XCTestManifests.swift
View file
Expand Up @@ -86,7 +86,6 @@ extension WorkspaceTests {
("testToolsVersionRootPackages", testToolsVersionRootPackages),
("testTransitiveDependencySwitchWithSameIdentity", testTransitiveDependencySwitchWithSameIdentity),
("testUpdate", testUpdate),
("testUpdateToResolvedFileResolvesCorrectly", testUpdateToResolvedFileResolvesCorrectly),
]
}

Expand Down

0 comments on commit 39b444c

Please sign in to comment.