From dba876b9dbe7fdf90756d05fa091354a89037970 Mon Sep 17 00:00:00 2001 From: Boris Buegling Date: Fri, 31 Jan 2025 15:10:22 -0800 Subject: [PATCH] WIP: Improve `settingsForProductReferenceTarget` Currently, we only consider direct dependencies when looking up configured targets in the product plan, leading to use of the fallback in more cases than necessary. I am intentionally keeping the `.only` here since we don't have a great way of deciding the correct target if there's more than one, though maybe we should just bias towards the platform of `clientTarget` since the fallback will do that anyway? Another thing I am not sure about is what impact this will have on perf since computing the transitive closure will definitely be a heavier operation than what we are currently doing. I'd also like to add a test which exposes a case where using the primary case instead of the fallback is beneficial. --- Sources/SWBCore/BuildFileResolution.swift | 3 ++- .../ProductPlanning/ProductPlan.swift | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Sources/SWBCore/BuildFileResolution.swift b/Sources/SWBCore/BuildFileResolution.swift index 98df3b52..1b818513 100644 --- a/Sources/SWBCore/BuildFileResolution.swift +++ b/Sources/SWBCore/BuildFileResolution.swift @@ -31,6 +31,7 @@ extension BuildFileResolution { public protocol GlobalTargetInfoProvider { func getTargetSettings(_ configuredTarget: ConfiguredTarget) -> Settings func dependencies(of target: ConfiguredTarget) -> [ConfiguredTarget] + func transitiveDependencies(of: ConfiguredTarget) -> [ConfiguredTarget] var dynamicallyBuildingTargets: Set { get } } @@ -41,7 +42,7 @@ extension BuildFileResolution { /// - remark: If the producing target is a dependency of the current target (likely, but not guaranteed), then the `Settings` will be looked up from the `TargetInfoProvider` (which is likely the `GlobalProductPlan`). Otherwise a more complicated heuristic is used. public func settingsForProductReferenceTarget(_ productRefTarget: Target, parameters: BuildParameters) -> Settings { // If we have a concrete client target, we can look up the exact configured target that is producing the referenced product. - if let clientConfiguredTarget = self.configuredTarget, let productRefConfiguredTarget = globalTargetInfoProvider.dependencies(of: clientConfiguredTarget).filter({ $0.target.guid == productRefTarget.guid }).only { + if let clientConfiguredTarget = self.configuredTarget, let productRefConfiguredTarget = globalTargetInfoProvider.transitiveDependencies(of: clientConfiguredTarget).filter({ $0.target.guid == productRefTarget.guid }).only { return globalTargetInfoProvider.getTargetSettings(productRefConfiguredTarget) } else { // FIXME: A more reliable fallback might be to use GlobalProductPlan.productPathsToProducingTargets (where GlobalProductPlan conforms to TargetInfoProvider) if the absolute path of the product reference in this context were passed in. diff --git a/Sources/SWBTaskConstruction/ProductPlanning/ProductPlan.swift b/Sources/SWBTaskConstruction/ProductPlanning/ProductPlan.swift index b00ebdd4..90bdec4b 100644 --- a/Sources/SWBTaskConstruction/ProductPlanning/ProductPlan.swift +++ b/Sources/SWBTaskConstruction/ProductPlanning/ProductPlan.swift @@ -98,6 +98,24 @@ package final class GlobalProductPlan: GlobalTargetInfoProvider resolvedDependencies(of: target).map { $0.target } } + package func transitiveDependencies(of target: ConfiguredTarget) -> [ConfiguredTarget] { + // Use the static target for lookup if necessary. + let configuredTarget: ConfiguredTarget + if let staticTarget = staticallyBuildingTargetsWithDiamondLinkage[target.target] { + configuredTarget = target.replacingTarget(staticTarget) + } else { + configuredTarget = target + } + + return transitiveClosure([configuredTarget], successors: planRequest.buildGraph.dependencies(of:)).0.map { dependency in + if let dynamicTarget = dynamicallyBuildingTargetsWithDiamondLinkage[dependency.target] { + return dependency.replacingTarget(dynamicTarget) + } else { + return dependency + } + } + } + package func resolvedDependencies(of target: ConfiguredTarget) -> [ResolvedTargetDependency] { // Use the static target for lookup if necessary. let configuredTarget: ConfiguredTarget