Avoid loading duplicated packages if a root overrides a local dependency (#5507)

This was quite subtle. Normally, we resolve before calling `loadDependencyManifests` and roots will override any type of dependency in such a way that they don't become part of managed dependencies which means if we try to load via `loadManagedManifest`, we will get no result, essentially filtering them from dependency manifests. Now when requiring a resolved file, we are producing managed dependencies from the resolved file, plus we were adding entries for any local dependencies unconditionaly which would mean we would get results from `loadManagedManifest`, resulting in an additional entry in dependency manifests, compared to the "normal" mode. This PR changes the behavior such that we don't add a local dependency to managed dependencies if there's a root with the same identity which matches the results of the "normal" mode.

Fixes rdar://92622234, rdar://83337879

Author: neonichu

Sources/Workspace/Workspace.swift

@@ -1910,15 +1910,17 @@ extension Workspace {
         let allManifestsWithPossibleDuplicates = try topologicalSort(input) { pair in
             // optimization: preload manifest we know about in parallel
             let dependenciesRequired = pair.item.dependenciesRequired(for: pair.key.productFilter)
-            // pre-populate managed dependencies if we are asked to do so
-            // FIXME: this seems like hack, needs further investigation why this is needed
+            let dependenciesToLoad = dependenciesRequired.map{ $0.createPackageRef() }.filter { !loadedManifests.keys.contains($0.identity) }
+            // pre-populate managed dependencies if we are asked to do so (this happens when resolving to a resolved file)
             if automaticallyAddManagedDependencies {
-                try dependenciesRequired.filter { $0.isLocal }.forEach { dependency in
-                    try self.state.dependencies.add(.fileSystem(packageRef: dependency.createPackageRef()))
+                try dependenciesToLoad.forEach { ref in
+                    // Since we are creating managed dependencies based on the resolved file in this mode, but local packages aren't part of that file, they will be missing from it. So we're eagerly adding them here, but explicitly don't add any that are overridden by a root with the same identity since that would lead to loading the given package twice, once as a root and once as a dependency  which violates various assumptions.
+                    if case .fileSystem = ref.kind, !root.manifests.keys.contains(ref.identity) {
+                        try self.state.dependencies.add(.fileSystem(packageRef: ref))
+                    }
                 }
                 observabilityScope.trap { try self.state.save() }
             }
-            let dependenciesToLoad = dependenciesRequired.map{ $0.createPackageRef() }.filter { !loadedManifests.keys.contains($0.identity) }
             let dependenciesManifests = try temp_await { self.loadManagedManifests(for: dependenciesToLoad, observabilityScope: observabilityScope, completion: $0) }
             dependenciesManifests.forEach { loadedManifests[$0.key] = $0.value }
             return dependenciesRequired.compactMap { dependency in

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -4146,6 +4146,57 @@ final class WorkspaceTests: XCTestCase {
         }
     }
 
+    func testForceResolveToResolvedVersionsDuplicateLocalDependency() throws {
+        let sandbox = AbsolutePath("/tmp/ws/")
+        let fs = InMemoryFileSystem()
+
+        let workspace = try MockWorkspace(
+            sandbox: sandbox,
+            fileSystem: fs,
+            roots: [
+                MockPackage(
+                    name: "Root",
+                    targets: [
+                        MockTarget(name: "Root", dependencies: ["Foo", "Bar"]),
+                    ],
+                    products: [],
+                    dependencies: [
+                        .sourceControl(path: "./Foo", requirement: .upToNextMajor(from: "1.0.0")),
+                        .fileSystem(path: "./Bar"),
+                    ]
+                ),
+                MockPackage(
+                    name: "Bar",
+                    targets: [
+                        MockTarget(name: "Bar"),
+                    ],
+                    products: [
+                        MockProduct(name: "Bar", targets: ["Bar"]),
+                    ]
+                ),
+            ],
+            packages: [
+                MockPackage(
+                    name: "Foo",
+                    targets: [
+                        MockTarget(name: "Foo"),
+                    ],
+                    products: [
+                        MockProduct(name: "Foo", targets: ["Foo"]),
+                    ],
+                    versions: ["1.0.0", "1.2.0", "1.3.2"]
+                ),
+            ]
+        )
+
+        try workspace.checkPackageGraph(roots: ["Root", "Bar"]) { _, diagnostics in
+            XCTAssertNoDiagnostics(diagnostics)
+        }
+        try workspace.checkPackageGraph(roots: ["Root", "Bar"], forceResolvedVersions: true) { _, diagnostics in
+            XCTAssertNoDiagnostics(diagnostics)
+        }
+    }
+
     func testForceResolveWithNoResolvedFile() throws {
         let sandbox = AbsolutePath("/tmp/ws/")
         let fs = InMemoryFileSystem()