Merge pull request #803 from hamishknight/desc-golf-6.2

[6.2] Only check `isTargetSuitableForPlatformForIndex` for workspace description

Author: hamishknight

Sources/SWBCore/DependencyResolution.swift

@@ -623,26 +623,20 @@ extension SpecializationParameters {
 
     /// Determines whether a target should be configured for the given platform in the index arena.
     ///
-    /// The arena is used for two purposes:
-    ///   1. To retrieve settings for a given target
-    ///   2. To produce products of source dependencies for compilation purposes (it does not produce binaries)
+    /// When building a workspace build description, we configure for all possible platforms. As such,
+    /// we want to avoid unnecessarily configuring targets for unsupported platforms. When building a
+    /// target or package description, we are only configuring for a single platform and can therefore
+    /// avoid this check since we assume any dependency will necessary.
     ///
-    /// Thus, in general if a target doesn't support a platform, we don't want to configure it for that platform. If a
-    /// dependency is not supported for the platform of the dependent, presumably the dependent will not be able
-    /// to use its products for compilation purposes, since the source products will be put in a different platform
-    /// directory and/or they will not be usable by the dependent (e.g. the module will not be importable from a
-    /// different platform). If the dependency was intended to be usable from that platform for compilation purposes,
-    /// it would be a supported platform.
-    ///
-    /// There's an exception for this for a dependent host tool, which are required for compilation and must therefore
-    /// be configured (and registered as a dependency) regardless.
-    nonisolated func isTargetSuitableForPlatformForIndex(_ target: Target, parameters: BuildParameters, imposedParameters: SpecializationParameters?, dependencies: OrderedSet<ConfiguredTarget>? = nil) -> Bool {
-        if !buildRequest.enableIndexBuildArena {
-            return true
-        }
-
-        // Host tools case, always supported we'll override the parameters with that of the host regardless.
-        if target.isHostBuildTool || dependencies?.contains(where: { $0.target.isHostBuildTool }) == true {
+    /// Note there's an exception for this for host build tools, which are required for compilation
+    /// and must therefore be configured (and registered as a dependency) regardless
+    nonisolated func isTargetSuitableForPlatformForIndex(_ target: Target, parameters: BuildParameters, imposedParameters: SpecializationParameters?) -> Bool {
+        guard buildRequest.buildsIndexWorkspaceDescription else { return true }
+
+        // Host tools case, always supported since we'll override the parameters with that of the
+        // host regardless. Any dependencies will have the host platform imposed on them through
+        // `imposedParameters`.
+        if target.isHostBuildTool {
             return true
         }
 

Sources/SWBCore/LinkageDependencyResolver.swift

@@ -111,8 +111,7 @@ actor LinkageDependencyResolver {
                 if Task.isCancelled { return }
                 let configuredTarget = topLevelTargetsToDiscover[i]
                 let imposedParameters = resolver.specializationParameters(configuredTarget, workspaceContext: workspaceContext, buildRequest: buildRequest, buildRequestContext: buildRequestContext)
-                let dependenciesOnPath = LinkageDependencies()
-                await linkageDependencies(for: configuredTarget, imposedParameters: imposedParameters, dependenciesOnPath: dependenciesOnPath)
+                await linkageDependencies(for: configuredTarget, imposedParameters: imposedParameters)
             }
         }
 
@@ -130,7 +129,7 @@ actor LinkageDependencyResolver {
     private var dependenciesPerTarget = [ConfiguredTarget: [ResolvedTargetDependency]]()
     private var visitedDiscoveredTargets = Set<ConfiguredTarget>()
 
-    private func linkageDependencies(for configuredTarget: ConfiguredTarget, imposedParameters: SpecializationParameters?, dependenciesOnPath: LinkageDependencies) async {
+    private func linkageDependencies(for configuredTarget: ConfiguredTarget, imposedParameters: SpecializationParameters?) async {
         // Track that we have visited this target.
         let visited = !visitedDiscoveredTargets.insert(configuredTarget).inserted
 
@@ -156,7 +155,7 @@ actor LinkageDependencyResolver {
                 return nil
             }
             let buildParameters = resolver.buildParametersByTarget[target] ?? configuredTarget.parameters
-            if await !resolver.isTargetSuitableForPlatformForIndex(target, parameters: buildParameters, imposedParameters: imposedParameters, dependencies: dependenciesOnPath.path) {
+            if await !resolver.isTargetSuitableForPlatformForIndex(target, parameters: buildParameters, imposedParameters: imposedParameters) {
                 return nil
             }
             let effectiveImposedParameters = imposedParameters?.effectiveParameters(target: configuredTarget, dependency: ConfiguredTarget(parameters: buildParameters, target: target), dependencyResolver: resolver)
@@ -184,7 +183,7 @@ actor LinkageDependencyResolver {
             } else {
                 imposedParametersForDependency = resolver.specializationParameters(dependency.target, workspaceContext: workspaceContext, buildRequest: buildRequest, buildRequestContext: buildRequestContext)
             }
-            await self.linkageDependencies(for: dependency.target, imposedParameters: imposedParametersForDependency, dependenciesOnPath: dependenciesOnPath)
+            await self.linkageDependencies(for: dependency.target, imposedParameters: imposedParametersForDependency)
         }
     }
 
@@ -605,7 +604,3 @@ private extension Path {
         return basenameWithoutSuffix.nilIfEmpty
     }
 }
-
-fileprivate actor LinkageDependencies {
-    var path: OrderedSet<ConfiguredTarget> = []
-}

Sources/SWBCore/TargetDependencyResolver.swift

@@ -680,7 +680,7 @@ fileprivate extension TargetDependencyResolver {
         }
 
         // Add the discovered info.
-        let discoveredInfo = await computeDiscoveredTargetInfo(for: configuredTarget, imposedParameters: imposedParameters, dependencyPath: nil, resolver: resolver)
+        let discoveredInfo = await computeDiscoveredTargetInfo(for: configuredTarget, imposedParameters: imposedParameters, resolver: resolver)
         discoveredTargets[configuredTarget] = discoveredInfo
 
         // If we have no dependencies, we are done.
@@ -742,7 +742,7 @@ fileprivate extension TargetDependencyResolver {
             discoveredInfo = info
         } else {
             if resolver.makeAggregateTargetsTransparentForSpecialization {
-                discoveredInfo = await computeDiscoveredTargetInfo(for: configuredTarget, imposedParameters: imposedParameters, dependencyPath: dependencyPath, resolver: resolver)
+                discoveredInfo = await computeDiscoveredTargetInfo(for: configuredTarget, imposedParameters: imposedParameters, resolver: resolver)
             } else {
                 var immediateDependencies = [ResolvedTargetDependency]()
                 var packageProductDependencies = [PackageProductTarget]()
@@ -820,14 +820,14 @@ fileprivate extension TargetDependencyResolver {
     }
 
     /// Discover the info for a configured target with the given imposed parameters.
-    private func computeDiscoveredTargetInfo(for configuredTarget: ConfiguredTarget, imposedParameters: SpecializationParameters?, dependencyPath: OrderedSet<ConfiguredTarget>?, resolver: isolated DependencyResolver) async -> DiscoveredTargetInfo {
+    private func computeDiscoveredTargetInfo(for configuredTarget: ConfiguredTarget, imposedParameters: SpecializationParameters?, resolver: isolated DependencyResolver) async -> DiscoveredTargetInfo {
         var immediateDependencies = [ResolvedTargetDependency]()
         var packageProductDependencies = [PackageProductTarget]()
         for dependency in resolver.explicitDependencies(for: configuredTarget) {
             if let asPackageProduct = dependency as? PackageProductTarget {
                 packageProductDependencies.append(asPackageProduct)
             } else {
-                if !resolver.isTargetSuitableForPlatformForIndex(dependency, parameters: configuredTarget.parameters, imposedParameters: imposedParameters, dependencies: dependencyPath) {
+                if !resolver.isTargetSuitableForPlatformForIndex(dependency, parameters: configuredTarget.parameters, imposedParameters: imposedParameters) {
                     continue
                 }
 

Sources/SWBTestSupport/BuildOperationTester.swift

@@ -1650,6 +1650,7 @@ package final class BuildOperationTester {
     /// Construct 'prepare' index build operation, and test the result.
     package func checkIndexBuild<T>(
         prepareTargets: [String],
+        buildTargets: [any TestTarget]? = nil,
         workspaceOperation: Bool = true,
         runDestination: RunDestinationInfo? = nil,
         persistent: Bool = false,
@@ -1658,6 +1659,7 @@ package final class BuildOperationTester {
     ) async throws -> T {
         let buildRequest = try Self.buildRequestForIndexOperation(
             workspace: workspace,
+            buildTargets: buildTargets,
             prepareTargets: prepareTargets,
             workspaceOperation: workspaceOperation,
             runDestination: runDestination,