[Workspace] Detect changes in URLs when identity remains the same

Validates that the URL of the required dependencies are actually the ones that we currently have in the managed dependencies. This fixes a major bug where SwiftPM would not detect if fork of a package is added or removed. <rdar://problem/34302690> Detect changes in dependency URLs when the package identity remains same

Author: aciidgh

Sources/TestSupport/TestWorkspace.swift

@@ -71,9 +71,10 @@ public final class TestWorkspace {
  var manifests: [MockManifestLoader.Key: Manifest] = [:]
 
  func create(package: TestPackage, basePath: AbsolutePath, isRoot: Bool) throws {
- let packagePath = basePath.appending(component: package.path ?? package.name)
+ let packagePath = basePath.appending(RelativePath(package.path ?? package.name))
+
  let sourcesDir = packagePath.appending(component: "Sources")
- let url = (isRoot ? packagePath : packagesDir.appending(component: package.path ?? package.name)).asString
+ let url = (isRoot ? packagePath : packagesDir.appending(RelativePath(package.path ?? package.name))).asString
  let specifier = RepositorySpecifier(url: url)
 
  // Create targets on disk.
@@ -161,7 +162,7 @@ public final class TestWorkspace {
 
  fileprivate func convert(_ packagesDir: AbsolutePath) -> PackageGraphRootInput.PackageDependency {
  return PackageGraphRootInput.PackageDependency(
- url: packagesDir.appending(component: name).asString,
+ url: packagesDir.appending(RelativePath(name)).asString,
  requirement: requirement,
  location: name
  )
@@ -445,7 +446,7 @@ public struct TestDependency {
  }
 
  public func convert(baseURL: AbsolutePath) -> PackageDependencyDescription {
- return PackageDependencyDescription(url: baseURL.appending(component: name).asString, requirement: requirement)
+ return PackageDependencyDescription(url: baseURL.appending(RelativePath(name)).asString, requirement: requirement)
  }
 }
 

Sources/Workspace/PinsStore.swift

@@ -127,12 +127,19 @@ public final class PinsStore {
 
  /// Unpin all of the currently pinnned dependencies.
  ///
- /// This method does not automatically write to state file.
+ /// This method does not automatically write to the state file.
  public func unpinAll() {
  // Reset the pins map.
  pinsMap = [:]
  }
 
+ /// Unpin the given package.
+ ///
+ /// This method does not automatically write to the state file.
+ public func unpin(_ package: PackageReference) {
+ pinsMap[package.identity] = nil
+ }
+
  /// Creates constraints based on the pins in the store.
  public func createConstraints() -> [RepositoryPackageConstraint] {
  return pins.map({ pin in

Sources/Workspace/Workspace.swift

@@ -880,6 +880,32 @@ extension Workspace {
  return manifestLoader.interpreterFlags(for: toolsVersion.manifestVersion)
  }
 
+ /// Checks if the managed dependency for a given package reference is still valid.
+ func validateManagedDependency(for inputRef: PackageReference, diagnostics: DiagnosticsEngine) {
+ // There is nothing to validate if there is no managed dependency for this package.
+ guard let dependencyRef = managedDependencies[forIdentity: inputRef.identity]?.packageRef else {
+ return
+ }
+
+ // Ensure that the package paths are equal.
+ if dependencyRef.path == inputRef.path {
+ return
+ }
+
+ // Otherwise, the managed dependency is out-of-sync and we need to
+ // remove it. This usually happens when a fork is added or removed for
+ // a dependency.
+ diagnostics.wrap {
+ // Remove the dependency.
+ try self.remove(package: dependencyRef)
+
+ // Unpin it.
+ let pinsStore = try self.pinsStore.load()
+ pinsStore.unpin(dependencyRef)
+ try pinsStore.saveState()
+ }
+ }
+
  /// Load the manifests for the current dependency tree.
  ///
  /// This will load the manifests for the root package as well as all the
@@ -905,8 +931,13 @@ extension Workspace {
  return DependencyManifests(root: root, dependencies: [], workspace: self)
  }
 
- let rootDependencyManifests = root.dependencies.compactMap({
- return loadManifest(for: $0.createPackageRef(), diagnostics: diagnostics)
+ let rootDependencyManifests: [Manifest] = root.dependencies.compactMap({
+ let dependencyRef = $0.createPackageRef()
+
+ validateManagedDependency(for: dependencyRef, diagnostics: diagnostics)
+ if diagnostics.hasErrors { return nil }
+
+ return loadManifest(for: dependencyRef, diagnostics: diagnostics)
  })
  let inputManifests = root.manifests + rootDependencyManifests
 
@@ -917,6 +948,10 @@ extension Workspace {
  let dependencies = transitiveClosure(inputManifests.map({ KeyedPair($0, key: $0.name) })) { node in
  return node.item.dependencies.compactMap({ dependency in
  let ref = dependency.createPackageRef()
+
+ validateManagedDependency(for: ref, diagnostics: diagnostics)
+ if diagnostics.hasErrors { return nil }
+
  let manifest = loadedManifests[ref.identity] ?? loadManifest(for: ref, diagnostics: diagnostics)
  loadedManifests[ref.identity] = manifest
  return manifest.flatMap({ KeyedPair($0, key: $0.name) })

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -2175,6 +2175,86 @@ final class WorkspaceTests: XCTestCase {
  }
  }
 
+ func testDependencySwitchWithSameIdentity() 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: []),
+ ],
+ products: [],
+ dependencies: []
+ ),
+ ],
+ packages: [
+ TestPackage(
+ name: "Foo",
+ targets: [
+ TestTarget(name: "Foo"),
+ ],
+ products: [
+ TestProduct(name: "Foo", targets: ["Foo"]),
+ ],
+ versions: [nil]
+ ),
+ TestPackage(
+ name: "Foo",
+ path: "Nested/Foo",
+ targets: [
+ TestTarget(name: "Foo"),
+ ],
+ products: [
+ TestProduct(name: "Foo", targets: ["Foo"]),
+ ],
+ versions: [nil]
+ ),
+ ]
+ )
+
+ // Test that switching between two same local packages placed at
+ // different locations works correctly.
+
+ var deps: [TestWorkspace.PackageDependency] = [
+ .init(name: "Foo", requirement: .localPackage),
+ ]
+ workspace.checkPackageGraph(roots: ["Root"], deps: deps) { (graph, diagnostics) in
+ PackageGraphTester(graph) { result in
+ result.check(roots: "Root")
+ result.check(packages: "Foo", "Root")
+ }
+ XCTAssertNoDiagnostics(diagnostics)
+ }
+ workspace.checkManagedDependencies() { result in
+ result.check(dependency: "foo", at: .local)
+ }
+ do {
+ let ws = workspace.createWorkspace()
+ let dependency = ws.managedDependencies[forIdentity: "foo"]
+ XCTAssertEqual(dependency?.packageRef.path, "/tmp/ws/pkgs/Foo")
+ }
+
+ deps = [
+ .init(name: "Nested/Foo", requirement: .localPackage),
+ ]
+ workspace.checkPackageGraph(roots: ["Root"], deps: deps) { (_, diagnostics) in
+ XCTAssertNoDiagnostics(diagnostics)
+ }
+ workspace.checkManagedDependencies() { result in
+ result.check(dependency: "foo", at: .local)
+ }
+ do {
+ let ws = workspace.createWorkspace()
+ let dependency = ws.managedDependencies[forIdentity: "foo"]
+ XCTAssertEqual(dependency?.packageRef.path, "/tmp/ws/pkgs/Nested/Foo")
+ }
+ }
+
  func testResolvedFileUpdate() throws {
  let sandbox = AbsolutePath("/tmp/ws/")
  let fs = InMemoryFileSystem()

Tests/WorkspaceTests/XCTestManifests.swift

@@ -49,6 +49,7 @@ extension WorkspaceTests {
  ("testDeletedCheckoutDirectory", testDeletedCheckoutDirectory),
  ("testDependencyManifestLoading", testDependencyManifestLoading),
  ("testDependencyResolutionWithEdit", testDependencyResolutionWithEdit),
+ ("testDependencySwitchWithSameIdentity", testDependencySwitchWithSameIdentity),
  ("testEditDependency", testEditDependency),
  ("testGraphData", testGraphData),
  ("testGraphRootDependencies", testGraphRootDependencies),