Skip to content

Commit 309e231

Browse files
authored
Fix conditional dependencies in root packages (#5784)
SwiftPM's build system always build every target that's part of a root package, regardless of its reachability in the graph, supposedly to ensure all the code in the root package will get exercised. This is of course fundamentally incompatible with the idea of conditional dependencies and therefore causes user confusion. The most reasonable way to resolve this seems to be to exclude any target that's conditionally depended upon with a condition unfullfiled by the current build environment from building automatically because of its presence in the root package. This does not change what gets built based on dependencies, so degenerate cases where a target is depended upon both conditionally and unconditionally will not be affected. Because of this, it seems reasonable to not tie the new behavior to the tools-version. rdar://97751068
1 parent d98de63 commit 309e231

File tree

6 files changed

+61
-5
lines changed

6 files changed

+61
-5
lines changed
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// swift-tools-version: 5.7
2+
3+
import PackageDescription
4+
5+
let package = Package(
6+
name: "client",
7+
products: [
8+
.library(name: "client", targets: ["client"]),
9+
],
10+
dependencies: [
11+
],
12+
targets: [
13+
.target(name: "client", dependencies: [
14+
.target(name: "linuxOnly", condition: .when(platforms: [.linux]))
15+
]),
16+
.target(name: "linuxOnly"),
17+
]
18+
)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
public struct client {
2+
public private(set) var text = "Hello, World!"
3+
4+
public init() {
5+
}
6+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
public struct linuxOnly {
2+
public private(set) var text = "Hello, World!"
3+
4+
public init() {
5+
#if os(Linux)
6+
print("bestOS")
7+
#else
8+
#error("not linux")
9+
#endif
10+
}
11+
}

Sources/Build/LLBuildManifestBuilder.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ extension LLBuildManifestBuilder {
649649
inputs: cmdOutputs,
650650
outputs: [targetOutput]
651651
)
652-
if plan.graph.isInRootPackages(target.target) {
652+
if plan.graph.isInRootPackages(target.target, satisfying: self.buildEnvironment) {
653653
if !target.isTestTarget {
654654
addNode(targetOutput, toTarget: .main)
655655
}
@@ -810,7 +810,7 @@ extension LLBuildManifestBuilder {
810810
outputs: [output]
811811
)
812812

813-
if plan.graph.isInRootPackages(target.target) {
813+
if plan.graph.isInRootPackages(target.target, satisfying: self.buildEnvironment) {
814814
if !target.isTestTarget {
815815
addNode(output, toTarget: .main)
816816
}

Sources/PackageGraph/PackageGraph.swift

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,23 @@ public struct PackageGraph {
7979
/// in the graph due to loading errors. This set doesn't include the root packages.
8080
public let requiredDependencies: Set<PackageReference>
8181

82-
/// Returns true if a given target is present in root packages.
83-
public func isInRootPackages(_ target: ResolvedTarget) -> Bool {
82+
/// Returns true if a given target is present in root packages and is not excluded for the given build environment.
83+
public func isInRootPackages(_ target: ResolvedTarget, satisfying buildEnvironment: BuildEnvironment) -> Bool {
8484
// FIXME: This can be easily cached.
85-
return rootPackages.flatMap({ $0.targets }).contains(target)
85+
return rootPackages.flatMap({ (package: ResolvedPackage) -> Set<ResolvedTarget> in
86+
let allDependencies = package.targets.flatMap { $0.dependencies }
87+
let unsatisfiedDependencies = allDependencies.filter { !$0.satisfies(buildEnvironment) }
88+
let unsatisfiedDependencyTargets = unsatisfiedDependencies.compactMap { (dep: ResolvedTarget.Dependency) -> ResolvedTarget? in
89+
switch dep {
90+
case .target(let target, _):
91+
return target
92+
default:
93+
return nil
94+
}
95+
}
96+
97+
return Set(package.targets).subtracting(unsatisfiedDependencyTargets)
98+
}).contains(target)
8699
}
87100

88101
public func isRootPackage(_ package: ResolvedPackage) -> Bool {

Tests/FunctionalTests/MiscellaneousTests.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,4 +623,12 @@ class MiscellaneousTestCase: XCTestCase {
623623
XCTAssertNoMatch(stdout + stderr, .contains("'Deprecated2' is deprecated"))
624624
}
625625
}
626+
627+
func testRootPackageWithConditionals() throws {
628+
try fixture(name: "Miscellaneous/RootPackageWithConditionals") { path in
629+
let (_, stderr) = try SwiftPMProduct.SwiftBuild.execute([], packagePath: path)
630+
let errors = stderr.components(separatedBy: .newlines).filter { !$0.contains("[logging] misuse") && !$0.isEmpty }
631+
XCTAssertEqual(errors, [], "unexpected errors: \(errors)")
632+
}
633+
}
626634
}

0 commit comments

Comments
 (0)