Skip to content

Commit 9a4b1b2

Browse files
committed
Fix issues with excluding macros/plugins from dependency computation
While fixing this, I also noticed the original issue also exists for executable targets, so this gets fixed here as well. There's one unfortunate nuance here for test targets since using a macro/plugin is indistinguishable from needing to link it because it is being tested. We err on the side of caution here and will always link. (sidenote: theoretically, plugins *do* distinguish between linkage and use in the package manifest, but this distinction is not carried forward into the actual model) Partially fixes https://github.com/apple/swift/issues/67371 since the underlying project also does not declare a dependency on the macro that is being tested. (cherry picked from commit b31c19a)
1 parent 316a3d2 commit 9a4b1b2

File tree

2 files changed

+32
-8
lines changed

2 files changed

+32
-8
lines changed

Fixtures/Miscellaneous/Plugins/MySourceGenPlugin/Package.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// swift-tools-version: 5.6
1+
// swift-tools-version: 5.9
22
import PackageDescription
33

44
let package = Package(

Sources/Build/BuildPlan.swift

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -698,13 +698,28 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
698698
libraryBinaryPaths: Set<AbsolutePath>,
699699
availableTools: [String: AbsolutePath]
700700
) {
701-
/* Prior to tools-version 5.9, we used to errorneously recursively traverse plugin dependencies and statically include their
701+
/* Prior to tools-version 5.9, we used to errorneously recursively traverse executable/plugin dependencies and statically include their
702702
targets. For compatibility reasons, we preserve that behavior for older tools-versions. */
703-
let shouldExcludePlugins: Bool
703+
let shouldExcludeExecutablesAndPlugins: Bool
704704
if let toolsVersion = self.graph.package(for: product)?.manifest.toolsVersion {
705-
shouldExcludePlugins = toolsVersion >= .v5_9
705+
shouldExcludeExecutablesAndPlugins = toolsVersion >= .v5_9
706706
} else {
707-
shouldExcludePlugins = false
707+
shouldExcludeExecutablesAndPlugins = false
708+
}
709+
710+
// For test targets, we need to consider the first level of transitive dependencies since the first level is always test targets.
711+
let topLevelDependencies: [PackageModel.Target]
712+
if product.type == .test {
713+
topLevelDependencies = product.targets.flatMap { $0.underlyingTarget.dependencies }.compactMap {
714+
switch $0 {
715+
case .product:
716+
return nil
717+
case .target(let target, _):
718+
return target
719+
}
720+
}
721+
} else {
722+
topLevelDependencies = []
708723
}
709724

710725
// Sort the product targets in topological order.
@@ -713,10 +728,19 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
713728
switch dependency {
714729
// Include all the dependencies of a target.
715730
case .target(let target, _):
716-
if target.type == .macro {
731+
let isTopLevel = topLevelDependencies.contains(target.underlyingTarget) || product.targets.contains(target)
732+
let topLevelIsExecutable = isTopLevel && product.type == .executable
733+
let topLevelIsMacro = isTopLevel && product.type == .macro
734+
let topLevelIsPlugin = isTopLevel && product.type == .plugin
735+
let topLevelIsTest = isTopLevel && product.type == .test
736+
737+
if !topLevelIsMacro && !topLevelIsTest && target.type == .macro {
738+
return []
739+
}
740+
if shouldExcludeExecutablesAndPlugins, !topLevelIsPlugin && !topLevelIsTest && target.type == .plugin {
717741
return []
718742
}
719-
if shouldExcludePlugins, target.type == .plugin {
743+
if shouldExcludeExecutablesAndPlugins, !topLevelIsExecutable && topLevelIsTest && target.type == .executable {
720744
return []
721745
}
722746
return target.dependencies.filter { $0.satisfies(self.buildEnvironment) }
@@ -733,7 +757,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
733757
case .library(.automatic), .library(.static):
734758
return productDependencies
735759
case .plugin:
736-
return shouldExcludePlugins ? [] : productDependencies
760+
return shouldExcludeExecutablesAndPlugins ? [] : productDependencies
737761
case .library(.dynamic), .test, .executable, .snippet, .macro:
738762
return []
739763
}

0 commit comments

Comments
 (0)