Skip to content

Commit 27f444f

Browse files
neonichuneonichu
authored
Temporarily disable target-based dependency resolution (#2998)
We are seeing some issues with the full implementation of target-based dependency resolution done in #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.
1 parent ae748dd commit 27f444f

File tree

9 files changed

+79
-7
lines changed

9 files changed

+79
-7
lines changed

‎Sources/PackageGraph/DependencyResolutionNode.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ public enum DependencyResolutionNode {
7878

7979
/// Assembles the product filter to use on the manifest for this node to determine its dependencies.
8080
public var productFilter: ProductFilter {
81+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
8182
switch self {
8283
case .empty:
8384
return .specific([])
@@ -86,6 +87,9 @@ public enum DependencyResolutionNode {
8687
case .root:
8788
return .everything
8889
}
90+
#else
91+
return .everything
92+
#endif
8993
}
9094

9195
/// Returns the dependency that a product has on its own package, if relevant.

‎Sources/PackageModel/Manifest.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ public final class Manifest: ObjectIdentifierProtocol {
131131

132132
/// Returns the targets required for a particular product filter.
133133
public func targetsRequired(for productFilter: ProductFilter) -> [TargetDescription] {
134+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
134135
// If we have already calcualted it, returned the cached value.
135136
if let targets = _requiredTargets[productFilter] {
136137
return targets
@@ -147,6 +148,9 @@ public final class Manifest: ObjectIdentifierProtocol {
147148
_requiredTargets[productFilter] = targets
148149
return targets
149150
}
151+
#else
152+
return self.targets
153+
#endif
150154
}
151155

152156
/// Returns the package dependencies required for a particular products filter.
@@ -212,6 +216,7 @@ public final class Manifest: ObjectIdentifierProtocol {
212216
}
213217

214218
return dependencies.compactMap { dependency in
219+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
215220
if let filter = associations[dependency.name] {
216221
return FilteredDependencyDescription(declaration: dependency, productFilter: filter)
217222
} else if keepUnused {
@@ -221,6 +226,9 @@ public final class Manifest: ObjectIdentifierProtocol {
221226
// Dependencies known to not have any relevant products are discarded.
222227
return nil
223228
}
229+
#else
230+
return FilteredDependencyDescription(declaration: dependency, productFilter: .everything)
231+
#endif
224232
}
225233
}
226234

‎Tests/BuildTests/BuildPlanTests.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,18 +1316,25 @@ final class BuildPlanTests: XCTestCase {
13161316
]
13171317
)
13181318

1319+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
13191320
XCTAssertEqual(diagnostics.diagnostics.count, 1)
13201321
let firstDiagnostic = diagnostics.diagnostics.first.map({ $0.message.text })
13211322
XCTAssert(
13221323
firstDiagnostic == "dependency 'C' is not used by any target",
13231324
"Unexpected diagnostic: " + (firstDiagnostic ?? "[none]")
13241325
)
1326+
#endif
13251327

13261328
let graphResult = PackageGraphResult(graph)
13271329
graphResult.check(reachableProducts: "aexec", "BLibrary")
13281330
graphResult.check(reachableTargets: "ATarget", "BTarget1")
1331+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
13291332
graphResult.check(products: "aexec", "BLibrary")
13301333
graphResult.check(targets: "ATarget", "BTarget1")
1334+
#else
1335+
graphResult.check(products: "BLibrary", "bexec", "aexec", "cexec")
1336+
graphResult.check(targets: "ATarget", "BTarget1", "BTarget2", "CTarget")
1337+
#endif
13311338

13321339
let planResult = BuildPlanResult(plan: try BuildPlan(
13331340
buildParameters: mockBuildParameters(),
@@ -1336,8 +1343,13 @@ final class BuildPlanTests: XCTestCase {
13361343
fileSystem: fileSystem
13371344
))
13381345

1346+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
13391347
planResult.checkProductsCount(2)
13401348
planResult.checkTargetsCount(2)
1349+
#else
1350+
planResult.checkProductsCount(4)
1351+
planResult.checkTargetsCount(4)
1352+
#endif
13411353
}
13421354

13431355
func testReachableBuildProductsAndTargets() throws {

‎Tests/PackageGraphTests/PackageGraphTests.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,9 @@ class PackageGraphTests: XCTestCase {
739739

740740
DiagnosticsEngineTester(diagnostics) { result in
741741
result.check(diagnostic: "dependency 'Baz' is not used by any target", behavior: .warning)
742+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
742743
result.check(diagnostic: "dependency 'Biz' is not used by any target", behavior: .warning)
744+
#endif
743745
}
744746
}
745747

@@ -1077,7 +1079,12 @@ class PackageGraphTests: XCTestCase {
10771079
}
10781080
}
10791081

1080-
func testUnreachableProductsSkipped() {
1082+
func testUnreachableProductsSkipped() throws {
1083+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
1084+
#else
1085+
try XCTSkipIf(true)
1086+
#endif
1087+
10811088
let fs = InMemoryFileSystem(emptyFiles:
10821089
"/Root/Sources/Root/Root.swift",
10831090
"/Immediate/Sources/ImmediateUsed/ImmediateUsed.swift",

‎Tests/PackageGraphTests/PubgrubTests.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1058,7 +1058,12 @@ final class PubgrubTests: XCTestCase {
10581058
])
10591059
}
10601060

1061-
func testUnreachableProductsSkipped() {
1061+
func testUnreachableProductsSkipped() throws {
1062+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
1063+
#else
1064+
try XCTSkipIf(true)
1065+
#endif
1066+
10621067
builder.serve("root", at: .unversioned, with: [
10631068
"root": ["immediate": (.versionSet(v1Range), .specific(["ImmediateUsed"]))]
10641069
])

‎Tests/PackageGraphTests/RepositoryPackageContainerProviderTests.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,11 @@ class RepositoryPackageContainerProviderTests: XCTestCase {
360360
}
361361

362362
func testDependencyConstraints() throws {
363+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
364+
#else
365+
try XCTSkipIf(true)
366+
#endif
367+
363368
let dependencies = [
364369
PackageDependencyDescription(name: "Bar1", url: "/Bar1", requirement: .upToNextMajor(from: "1.0.0")),
365370
PackageDependencyDescription(name: "Bar2", url: "/Bar2", requirement: .upToNextMajor(from: "1.0.0")),

‎Tests/PackageModelTests/ManifestTests.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,13 @@ class ManifestTests: XCTestCase {
5656
targets: targets
5757
)
5858

59+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
5960
XCTAssertEqual(manifest.targetsRequired(for: .specific(["Foo", "Bar"])).map({ $0.name }).sorted(), [
6061
"Bar",
6162
"Baz",
6263
"Foo",
6364
])
65+
#endif
6466
}
6567
}
6668

@@ -150,11 +152,13 @@ class ManifestTests: XCTestCase {
150152
targets: targets
151153
)
152154

155+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
153156
XCTAssertEqual(manifest.dependenciesRequired(for: .specific(["Foo"])).map({ $0.declaration.name }).sorted(), [
154157
"Bar1", // Foo → Foo1 → Bar1
155158
"Bar2", // Foo → Foo1 → Foo2 → Bar2
156159
// (Bar3 is unreachable.)
157160
])
161+
#endif
158162
}
159163
}
160164
}

‎Tests/WorkspaceTests/WorkspaceTests.swift

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,11 +1470,15 @@ final class WorkspaceTests: XCTestCase {
14701470
// Run update.
14711471
workspace.checkUpdateDryRun(roots: ["Root"]) { changes, diagnostics in
14721472
XCTAssertNoDiagnostics(diagnostics)
1473+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
1474+
let stateChange = Workspace.PackageStateChange.updated(.init(requirement: .version(Version("1.5.0")), products: .specific(["Foo"])))
1475+
#else
1476+
let stateChange = Workspace.PackageStateChange.updated(.init(requirement: .version(Version("1.5.0")), products: .everything))
1477+
#endif
1478+
14731479
let expectedChange = (
14741480
PackageReference(identity: "foo", path: "/tmp/ws/pkgs/Foo"),
1475-
Workspace.PackageStateChange.updated(
1476-
.init(requirement: .version(Version("1.5.0")), products: .specific(["Foo"]))
1477-
)
1481+
stateChange
14781482
)
14791483
guard let change = changes?.first, changes?.count == 1 else {
14801484
XCTFail()
@@ -2030,11 +2034,13 @@ final class WorkspaceTests: XCTestCase {
20302034
]
20312035
)
20322036

2037+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
20332038
workspace.checkPackageGraph(roots: ["Root"]) { (graph, diagnostics) in
20342039
DiagnosticsEngineTester(diagnostics) { result in
20352040
result.check(diagnostic: .contains("Foo[Foo] 1.0.0..<2.0.0"), behavior: .error)
20362041
}
20372042
}
2043+
#endif
20382044
}
20392045

20402046
func testToolsVersionRootPackages() throws {
@@ -2867,9 +2873,11 @@ final class WorkspaceTests: XCTestCase {
28672873
result.check(packages: "Foo")
28682874
result.check(targets: "Foo")
28692875
}
2876+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
28702877
DiagnosticsEngineTester(diagnostics) { result in
28712878
result.check(diagnostic: .contains("Bar[Bar] {1.0.0..<1.5.0, 1.5.1..<2.0.0} is forbidden"), behavior: .error)
28722879
}
2880+
#endif
28732881
}
28742882
}
28752883

@@ -3998,6 +4006,11 @@ final class WorkspaceTests: XCTestCase {
39984006
}
39994007

40004008
func testTargetBasedDependency() throws {
4009+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
4010+
#else
4011+
try XCTSkipIf(true)
4012+
#endif
4013+
40014014
let sandbox = AbsolutePath("/tmp/ws/")
40024015
let fs = InMemoryFileSystem()
40034016

‎Tests/XCBuildSupportTests/PIFBuilderTests.swift

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,20 @@ class PIFBuilderTests: XCTestCase {
8686
let targetAExeDependencies = pif.workspace.projects[0].targets[0].dependencies
8787
XCTAssertEqual(targetAExeDependencies.map{ $0.targetGUID }, ["PACKAGE-PRODUCT:blib", "PACKAGE-TARGET:A2", "PACKAGE-TARGET:A3"])
8888
let projectBTargetNames = pif.workspace.projects[1].targets.map({ $0.name })
89+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
8990
XCTAssertEqual(projectBTargetNames, ["blib", "B2"])
91+
#else
92+
XCTAssertEqual(projectBTargetNames, ["bexe", "blib", "B2"])
93+
#endif
9094
}
9195
}
9296

93-
func testProject() {
97+
func testProject() throws {
98+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
99+
#else
100+
try XCTSkipIf(true)
101+
#endif
102+
94103
let fs = InMemoryFileSystem(emptyFiles:
95104
"/Foo/Sources/foo/main.swift",
96105
"/Foo/Tests/FooTests/tests.swift",
@@ -675,7 +684,12 @@ class PIFBuilderTests: XCTestCase {
675684
}
676685
}
677686

678-
func testTestProducts() {
687+
func testTestProducts() throws {
688+
#if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION
689+
#else
690+
try XCTSkipIf(true)
691+
#endif
692+
679693
let fs = InMemoryFileSystem(emptyFiles:
680694
"/Foo/Sources/FooTests/FooTests.swift",
681695
"/Foo/Sources/CFooTests/CFooTests.m",

0 commit comments

Comments
 (0)