Only check isTargetSuitableForPlatformForIndex for workspace description (#671)

* [test] NFC: Factor out `withHostToolsPackages`

Factor out the creation of the common package dependencies between
these two tests.

* [test] Parameterize two host tools index tests

Use Swift Testing parameterization to test different run destinations.

* [test] Set `SDKROOT: auto` at project level for `withHostToolsPackages`

This better matches what we actually generate for packages. Also
set all available platforms as supported.

* [test] Add same-package host tool dependency to `withHostToolsPackages`

This exposes the issue where `isTargetSuitableForPlatformForIndex`
returns `false` in the target and package build. Also change
`testHostToolsAndDependenciesAreBuiltDuringIndexingPreparationForPackage`
to use a dependency package and test both the target and package
build descriptions.

* Only check `isTargetSuitableForPlatformForIndex` for workspace description

For the package and target build description we can end up incorrectly
dropping build tool dependencies since not all clients pass in the
correct dependency information. It's not clear that we actually gain
much from doing this check for the target and package build
descriptions though, its primary purpose is to avoid configuring
unsupported targets in the workspace build description where we try
to configure for all available platforms. As such, switch to only
checking for the workspace build description. Note we don't encounter
this issue in the workspace case since there we override the build
parameters for host build tools to always build for the host platform.

rdar://152012769

* Remove dependency check from `isTargetSuitableForPlatformForIndex`

This gets applied pretty inconsistently, only 1 client actually
passes the correct dependency information. We ought to be able to rely
on the host platform being imposed for the workspace build description,
so it shouldn't be necessary.

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

@@ -122,8 +122,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)
             }
         }
 
@@ -141,7 +140,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
 
@@ -167,7 +166,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)
@@ -195,7 +194,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)
         }
     }
 
@@ -657,7 +656,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

@@ -1553,6 +1553,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,
@@ -1561,6 +1562,7 @@ package final class BuildOperationTester {
     ) async throws -> T {
         let buildRequest = try Self.buildRequestForIndexOperation(
             workspace: workspace,
+            buildTargets: buildTargets,
             prepareTargets: prepareTargets,
             workspaceOperation: workspaceOperation,
             runDestination: runDestination,