From 9a4b1b2b5e99d47d9329b493dcf517d61b5684b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20B=C3=BCgling?= Date: Tue, 18 Jul 2023 20:09:28 -0700 Subject: [PATCH] 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 b31c19ab3154779fc43b51a560260463c5985fc5) --- .../Plugins/MySourceGenPlugin/Package.swift | 2 +- Sources/Build/BuildPlan.swift | 38 +++++++++++++++---- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/Fixtures/Miscellaneous/Plugins/MySourceGenPlugin/Package.swift b/Fixtures/Miscellaneous/Plugins/MySourceGenPlugin/Package.swift index 42f1fe382d8..62de168b7ad 100644 --- a/Fixtures/Miscellaneous/Plugins/MySourceGenPlugin/Package.swift +++ b/Fixtures/Miscellaneous/Plugins/MySourceGenPlugin/Package.swift @@ -1,4 +1,4 @@ -// swift-tools-version: 5.6 +// swift-tools-version: 5.9 import PackageDescription let package = Package( diff --git a/Sources/Build/BuildPlan.swift b/Sources/Build/BuildPlan.swift index 5e6e13c74a9..24e1b8c7f25 100644 --- a/Sources/Build/BuildPlan.swift +++ b/Sources/Build/BuildPlan.swift @@ -698,13 +698,28 @@ public class BuildPlan: SPMBuildCore.BuildPlan { libraryBinaryPaths: Set, availableTools: [String: AbsolutePath] ) { - /* Prior to tools-version 5.9, we used to errorneously recursively traverse plugin dependencies and statically include their + /* Prior to tools-version 5.9, we used to errorneously recursively traverse executable/plugin dependencies and statically include their targets. For compatibility reasons, we preserve that behavior for older tools-versions. */ - let shouldExcludePlugins: Bool + let shouldExcludeExecutablesAndPlugins: Bool if let toolsVersion = self.graph.package(for: product)?.manifest.toolsVersion { - shouldExcludePlugins = toolsVersion >= .v5_9 + shouldExcludeExecutablesAndPlugins = toolsVersion >= .v5_9 } else { - shouldExcludePlugins = false + shouldExcludeExecutablesAndPlugins = false + } + + // For test targets, we need to consider the first level of transitive dependencies since the first level is always test targets. + let topLevelDependencies: [PackageModel.Target] + if product.type == .test { + topLevelDependencies = product.targets.flatMap { $0.underlyingTarget.dependencies }.compactMap { + switch $0 { + case .product: + return nil + case .target(let target, _): + return target + } + } + } else { + topLevelDependencies = [] } // Sort the product targets in topological order. @@ -713,10 +728,19 @@ public class BuildPlan: SPMBuildCore.BuildPlan { switch dependency { // Include all the dependencies of a target. case .target(let target, _): - if target.type == .macro { + let isTopLevel = topLevelDependencies.contains(target.underlyingTarget) || product.targets.contains(target) + let topLevelIsExecutable = isTopLevel && product.type == .executable + let topLevelIsMacro = isTopLevel && product.type == .macro + let topLevelIsPlugin = isTopLevel && product.type == .plugin + let topLevelIsTest = isTopLevel && product.type == .test + + if !topLevelIsMacro && !topLevelIsTest && target.type == .macro { + return [] + } + if shouldExcludeExecutablesAndPlugins, !topLevelIsPlugin && !topLevelIsTest && target.type == .plugin { return [] } - if shouldExcludePlugins, target.type == .plugin { + if shouldExcludeExecutablesAndPlugins, !topLevelIsExecutable && topLevelIsTest && target.type == .executable { return [] } return target.dependencies.filter { $0.satisfies(self.buildEnvironment) } @@ -733,7 +757,7 @@ public class BuildPlan: SPMBuildCore.BuildPlan { case .library(.automatic), .library(.static): return productDependencies case .plugin: - return shouldExcludePlugins ? [] : productDependencies + return shouldExcludeExecutablesAndPlugins ? [] : productDependencies case .library(.dynamic), .test, .executable, .snippet, .macro: return [] }