Temporarily disable target-based dependency resolution (swiftlang#2998)

We are seeing some issues with the full implementation of target-based dependency resolution done in swiftlang#2749 and need to temporarily disable it to unblock affected projects. The changes are still available behind a compile-time define `ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION` and the tests have been modified accordingly to work in both cases (some tests are skipped by default for now and only enabled when enabling target-based dependency resolution). I haven't yet distilled the issues into test cases, but once I have I'll start a branch of re-enabling this again.
federicobucchi · Jan 6, 2021 · 912e443 · 912e443
1 parent 8e5013e
commit 912e443
Show file tree

Hide file tree

Showing 9 changed files with 79 additions and 7 deletions.
diff --git a/Sources/PackageGraph/DependencyResolutionNode.swift b/Sources/PackageGraph/DependencyResolutionNode.swift
@@ -78,6 +78,7 @@ public enum DependencyResolutionNode {
 
     /// Assembles the product filter to use on the manifest for this node to determine its dependencies.
     public var productFilter: ProductFilter {
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
         switch self {
         case .empty:
             return .specific([])
@@ -86,6 +87,9 @@ public enum DependencyResolutionNode {
         case .root:
             return .everything
         }
+        #else
+        return .everything
+        #endif
     }
 
     /// Returns the dependency that a product has on its own package, if relevant.

diff --git a/Sources/PackageModel/Manifest.swift b/Sources/PackageModel/Manifest.swift
@@ -131,6 +131,7 @@ public final class Manifest: ObjectIdentifierProtocol {
 
     /// Returns the targets required for a particular product filter.
     public func targetsRequired(for productFilter: ProductFilter) -> [TargetDescription] {
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
         // If we have already calcualted it, returned the cached value.
         if let targets = _requiredTargets[productFilter] {
             return targets
@@ -147,6 +148,9 @@ public final class Manifest: ObjectIdentifierProtocol {
             _requiredTargets[productFilter] = targets
             return targets
         }
+        #else
+        return self.targets
+        #endif
     }
 
     /// Returns the package dependencies required for a particular products filter.
@@ -212,6 +216,7 @@ public final class Manifest: ObjectIdentifierProtocol {
         }
 
         return dependencies.compactMap { dependency in
+            #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
             if let filter = associations[dependency.name] {
                 return FilteredDependencyDescription(declaration: dependency, productFilter: filter)
             } else if keepUnused {
@@ -221,6 +226,9 @@ public final class Manifest: ObjectIdentifierProtocol {
                 // Dependencies known to not have any relevant products are discarded.
                 return nil
             }
+            #else
+            return FilteredDependencyDescription(declaration: dependency, productFilter: .everything)
+            #endif
         }
     }
 

diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift
@@ -1316,18 +1316,25 @@ final class BuildPlanTests: XCTestCase {
             ]
         )
 
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
         XCTAssertEqual(diagnostics.diagnostics.count, 1)
         let firstDiagnostic = diagnostics.diagnostics.first.map({ $0.message.text })
         XCTAssert(
             firstDiagnostic == "dependency 'C' is not used by any target",
             "Unexpected diagnostic: " + (firstDiagnostic ?? "[none]")
         )
+        #endif
 
         let graphResult = PackageGraphResult(graph)
         graphResult.check(reachableProducts: "aexec", "BLibrary")
         graphResult.check(reachableTargets: "ATarget", "BTarget1")
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
         graphResult.check(products: "aexec", "BLibrary")
         graphResult.check(targets: "ATarget", "BTarget1")
+        #else
+        graphResult.check(products: "BLibrary", "bexec", "aexec", "cexec")
+        graphResult.check(targets: "ATarget", "BTarget1", "BTarget2", "CTarget")
+        #endif
 
         let planResult = BuildPlanResult(plan: try BuildPlan(
             buildParameters: mockBuildParameters(),
@@ -1336,8 +1343,13 @@ final class BuildPlanTests: XCTestCase {
             fileSystem: fileSystem
         ))
 
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
         planResult.checkProductsCount(2)
         planResult.checkTargetsCount(2)
+        #else
+        planResult.checkProductsCount(4)
+        planResult.checkTargetsCount(4)
+        #endif
     }
 
     func testReachableBuildProductsAndTargets() throws {

diff --git a/Tests/PackageGraphTests/PackageGraphTests.swift b/Tests/PackageGraphTests/PackageGraphTests.swift
@@ -739,7 +739,9 @@ class PackageGraphTests: XCTestCase {
 
         DiagnosticsEngineTester(diagnostics) { result in
             result.check(diagnostic: "dependency 'Baz' is not used by any target", behavior: .warning)
+            #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
             result.check(diagnostic: "dependency 'Biz' is not used by any target", behavior: .warning)
+            #endif
         }
     }
 
@@ -1077,7 +1079,12 @@ class PackageGraphTests: XCTestCase {
         }
     }
 
-    func testUnreachableProductsSkipped() {
+    func testUnreachableProductsSkipped() throws {
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
+        #else
+        try XCTSkipIf(true)
+        #endif
+
         let fs = InMemoryFileSystem(emptyFiles:
             "/Root/Sources/Root/Root.swift",
             "/Immediate/Sources/ImmediateUsed/ImmediateUsed.swift",

diff --git a/Tests/PackageGraphTests/PubgrubTests.swift b/Tests/PackageGraphTests/PubgrubTests.swift
@@ -1058,7 +1058,12 @@ final class PubgrubTests: XCTestCase {
         ])
     }
 
-    func testUnreachableProductsSkipped() {
+    func testUnreachableProductsSkipped() throws {
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
+        #else
+        try XCTSkipIf(true)
+        #endif
+
         builder.serve("root", at: .unversioned, with: [
             "root": ["immediate": (.versionSet(v1Range), .specific(["ImmediateUsed"]))]
         ])

diff --git a/Tests/PackageGraphTests/RepositoryPackageContainerProviderTests.swift b/Tests/PackageGraphTests/RepositoryPackageContainerProviderTests.swift
@@ -360,6 +360,11 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
     }
 
     func testDependencyConstraints() throws {
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
+        #else
+        try XCTSkipIf(true)
+        #endif
+
         let dependencies = [
             PackageDependencyDescription(name: "Bar1", url: "/Bar1", requirement: .upToNextMajor(from: "1.0.0")),
             PackageDependencyDescription(name: "Bar2", url: "/Bar2", requirement: .upToNextMajor(from: "1.0.0")),

diff --git a/Tests/PackageModelTests/ManifestTests.swift b/Tests/PackageModelTests/ManifestTests.swift
@@ -56,11 +56,13 @@ class ManifestTests: XCTestCase {
                 targets: targets
             )
 
+            #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
             XCTAssertEqual(manifest.targetsRequired(for: .specific(["Foo", "Bar"])).map({ $0.name }).sorted(), [
                 "Bar",
                 "Baz",
                 "Foo",
             ])
+            #endif
         }
     }
 
@@ -150,11 +152,13 @@ class ManifestTests: XCTestCase {
                 targets: targets
             )
 
+            #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
             XCTAssertEqual(manifest.dependenciesRequired(for: .specific(["Foo"])).map({ $0.declaration.name }).sorted(), [
                 "Bar1", // Foo → Foo1 → Bar1
                 "Bar2", // Foo → Foo1 → Foo2 → Bar2
                 // (Bar3 is unreachable.)
             ])
+            #endif
         }
     }
 }
diff --git a/Tests/WorkspaceTests/WorkspaceTests.swift b/Tests/WorkspaceTests/WorkspaceTests.swift
@@ -1470,11 +1470,15 @@ final class WorkspaceTests: XCTestCase {
         // Run update.
         workspace.checkUpdateDryRun(roots: ["Root"]) { changes, diagnostics in
             XCTAssertNoDiagnostics(diagnostics)
+            #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
+            let stateChange = Workspace.PackageStateChange.updated(.init(requirement: .version(Version("1.5.0")), products: .specific(["Foo"])))
+            #else
+            let stateChange = Workspace.PackageStateChange.updated(.init(requirement: .version(Version("1.5.0")), products: .everything))
+            #endif
+
             let expectedChange = (
                 PackageReference(identity: "foo", path: "/tmp/ws/pkgs/Foo"),
-                Workspace.PackageStateChange.updated(
-                    .init(requirement: .version(Version("1.5.0")), products: .specific(["Foo"]))
-                )
+                stateChange
             )
             guard let change = changes?.first, changes?.count == 1 else {
                 XCTFail()
@@ -2030,11 +2034,13 @@ final class WorkspaceTests: XCTestCase {
             ]
         )
 
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
         workspace.checkPackageGraph(roots: ["Root"]) { (graph, diagnostics) in
             DiagnosticsEngineTester(diagnostics) { result in
                 result.check(diagnostic: .contains("Foo[Foo] 1.0.0..<2.0.0"), behavior: .error)
             }
         }
+        #endif
     }
 
     func testToolsVersionRootPackages() throws {
@@ -2867,9 +2873,11 @@ final class WorkspaceTests: XCTestCase {
                 result.check(packages: "Foo")
                 result.check(targets: "Foo")
             }
+            #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
             DiagnosticsEngineTester(diagnostics) { result in
                 result.check(diagnostic: .contains("Bar[Bar] {1.0.0..<1.5.0, 1.5.1..<2.0.0} is forbidden"), behavior: .error)
             }
+            #endif
         }
     }
 
@@ -3998,6 +4006,11 @@ final class WorkspaceTests: XCTestCase {
     }
 
     func testTargetBasedDependency() throws {
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
+        #else
+        try XCTSkipIf(true)
+        #endif
+
         let sandbox = AbsolutePath("/tmp/ws/")
         let fs = InMemoryFileSystem()
 

diff --git a/Tests/XCBuildSupportTests/PIFBuilderTests.swift b/Tests/XCBuildSupportTests/PIFBuilderTests.swift
@@ -86,11 +86,20 @@ class PIFBuilderTests: XCTestCase {
             let targetAExeDependencies = pif.workspace.projects[0].targets[0].dependencies
             XCTAssertEqual(targetAExeDependencies.map{ $0.targetGUID }, ["PACKAGE-PRODUCT:blib", "PACKAGE-TARGET:A2", "PACKAGE-TARGET:A3"])
             let projectBTargetNames = pif.workspace.projects[1].targets.map({ $0.name })
+            #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
             XCTAssertEqual(projectBTargetNames, ["blib", "B2"])
+            #else
+            XCTAssertEqual(projectBTargetNames, ["bexe", "blib", "B2"])
+            #endif
         }
     }
 
-    func testProject() {
+    func testProject() throws {
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
+        #else
+        try XCTSkipIf(true)
+        #endif
+
         let fs = InMemoryFileSystem(emptyFiles:
             "/Foo/Sources/foo/main.swift",
             "/Foo/Tests/FooTests/tests.swift",
@@ -675,7 +684,12 @@ class PIFBuilderTests: XCTestCase {
         }
     }
 
-    func testTestProducts() {
+    func testTestProducts() throws {
+        #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
+        #else
+        try XCTSkipIf(true)
+        #endif
+
         let fs = InMemoryFileSystem(emptyFiles:
             "/Foo/Sources/FooTests/FooTests.swift",
             "/Foo/Sources/CFooTests/CFooTests.m",