Actually skip not needed dependencies during dependency resolution (#3166) (#3185)

Actually skip not needed dependencies during dependency resolution

This is a follow-up to #3162. While that brought back the code for `dependenciesRequired(for:)`, the guard based on `productFilter` actually made it so that it was practically never executed. Instead, the guard should be based on whether the package in question is a root package.

Additionally, this made the order of the returned dependencies unstable, due to use of `Set`. Since `dependenciesRequired(for: keepUnused:)` is now only used if `ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION` is set, I removed some dead code from there as well.

(cherry picked from commit da4d643)

Author: neonichu

Sources/PackageModel/Manifest.swift

@@ -165,21 +165,21 @@ public final class Manifest: ObjectIdentifierProtocol {
             return dependencies
         }
         #else
-        guard toolsVersion >= .v5_2 && productFilter != .everything else {
+        guard toolsVersion >= .v5_2 && packageKind != .root else {
             return dependencies
         }
-
-        var requiredDependencies: Set<PackageDependencyDescription> = []
-
+        
+        var requiredDependencyURLs: Set<String> = []
+        
         for target in targetsRequired(for: products) {
             for targetDependency in target.dependencies {
                 if let dependency = packageDependency(referencedBy: targetDependency) {
-                    requiredDependencies.insert(dependency)
+                    requiredDependencyURLs.insert(dependency.url)
                 }
             }
         }
-
-        return Array(requiredDependencies)
+        
+        return dependencies.filter { requiredDependencyURLs.contains($0.url) }
         #endif
     }
 
@@ -233,7 +233,6 @@ public final class Manifest: ObjectIdentifierProtocol {
         }
 
         return dependencies.compactMap { dependency in
-            #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
             if let filter = associations[dependency.name] {
                 return dependency.filtered(by: filter)
             } else if keepUnused {
@@ -243,9 +242,6 @@ public final class Manifest: ObjectIdentifierProtocol {
                 // Dependencies known to not have any relevant products are discarded.
                 return nil
             }
-            #else
-            return dependency.filtered(by: .everything)
-            #endif
         }
     }
 

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -4067,14 +4067,17 @@ final class WorkspaceTests: XCTestCase {
     }
 
     func testTargetBasedDependency() throws {
+        let sandbox = AbsolutePath("/tmp/ws/")
+        let fs = InMemoryFileSystem()
+        
+        let barProducts: [MockProduct]
         #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
+        barProducts = [MockProduct(name: "Bar", targets: ["Bar"]), MockProduct(name: "BarUnused", targets: ["BarUnused"])]
         #else
-        try XCTSkipIf(true)
+        // Whether a product is being used does not affect dependency resolution in this case, so we omit the unused product.
+        barProducts = [MockProduct(name: "Bar", targets: ["Bar"])]
         #endif
 
-        let sandbox = AbsolutePath("/tmp/ws/")
-        let fs = InMemoryFileSystem()
-
         let workspace = try MockWorkspace(
             sandbox: sandbox,
             fs: fs,
@@ -4119,10 +4122,7 @@ final class WorkspaceTests: XCTestCase {
                         MockTarget(name: "BarUnused", dependencies: ["Biz"]),
                         MockTarget(name: "BarTests", dependencies: ["TestHelper2"], type: .test),
                     ],
-                    products: [
-                        MockProduct(name: "Bar", targets: ["Bar"]),
-                        MockProduct(name: "BarUnused", targets: ["BarUnused"]),
-                    ],
+                    products: barProducts,
                     dependencies: [
                         MockDependency(name: "TestHelper2", requirement: .upToNextMajor(from: "1.0.0")),
                         MockDependency(name: "Biz", requirement: .upToNextMajor(from: "1.0.0")),
@@ -5068,7 +5068,8 @@ final class WorkspaceTests: XCTestCase {
                     dependencies: [
                         MockDependency(name: nil, path: "bar", requirement: .upToNextMajor(from: "1.0.0")),
                     ],
-                    versions: ["1.0.0"]
+                    versions: ["1.0.0"],
+                    toolsVersion: .v5
                 ),
                 MockPackage(
                     name: "BarPackage",
@@ -5084,7 +5085,8 @@ final class WorkspaceTests: XCTestCase {
                     dependencies: [
                         MockDependency(name: nil, path: "other/utility", requirement: .upToNextMajor(from: "1.0.0")),
                     ],
-                    versions: ["1.0.0"]
+                    versions: ["1.0.0"],
+                    toolsVersion: .v5
                 ),
                 // this package never gets loaded since its identity is the same as "FooPackage"
                 MockPackage(