Skip to content

Commit b08be46

Browse files
committed
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
1 parent bbe452f commit b08be46

File tree

2 files changed

+13
-17
lines changed

2 files changed

+13
-17
lines changed

Sources/SWBCore/DependencyResolution.swift

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -623,23 +623,15 @@ extension SpecializationParameters {
623623

624624
/// Determines whether a target should be configured for the given platform in the index arena.
625625
///
626-
/// The arena is used for two purposes:
627-
/// 1. To retrieve settings for a given target
628-
/// 2. To produce products of source dependencies for compilation purposes (it does not produce binaries)
626+
/// When building a workspace build description, we configure for all possible platforms. As such,
627+
/// we want to avoid unnecessarily configuring targets for unsupported platforms. When building a
628+
/// target or package description, we are only configuring for a single platform and can therefore
629+
/// avoid this check since we assume any dependency will necessary.
629630
///
630-
/// Thus, in general if a target doesn't support a platform, we don't want to configure it for that platform. If a
631-
/// dependency is not supported for the platform of the dependent, presumably the dependent will not be able
632-
/// to use its products for compilation purposes, since the source products will be put in a different platform
633-
/// directory and/or they will not be usable by the dependent (e.g. the module will not be importable from a
634-
/// different platform). If the dependency was intended to be usable from that platform for compilation purposes,
635-
/// it would be a supported platform.
636-
///
637-
/// There's an exception for this for a dependent host tool, which are required for compilation and must therefore
638-
/// be configured (and registered as a dependency) regardless.
631+
/// Note there's an exception for this for host build tools, which are required for compilation
632+
/// and must therefore be configured (and registered as a dependency) regardless
639633
nonisolated func isTargetSuitableForPlatformForIndex(_ target: Target, parameters: BuildParameters, imposedParameters: SpecializationParameters?, dependencies: OrderedSet<ConfiguredTarget>? = nil) -> Bool {
640-
if !buildRequest.enableIndexBuildArena {
641-
return true
642-
}
634+
guard buildRequest.buildsIndexWorkspaceDescription else { return true }
643635

644636
// Host tools case, always supported we'll override the parameters with that of the host regardless.
645637
if target.isHostBuildTool || dependencies?.contains(where: { $0.target.isHostBuildTool }) == true {

Tests/SWBCoreTests/IndexTargetDependencyResolverTests.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -949,8 +949,12 @@ import SWBUtil
949949
#expect(results.targets(packageProduct).map{ results.targetNameAndPlatform($0) } == ["PackageLibProduct-iphoneos"])
950950
#expect(results.targets(unreferencedPackageLib).map{ results.targetNameAndPlatform($0) } == ["UnreferencedPackageLib-iphoneos"])
951951

952-
try results.checkDependencies(of: .init(packageLib2, "iphoneos"), are: [])
953-
952+
switch results.graphType {
953+
case .dependency:
954+
try results.checkDependencies(of: packageLib2, are: [.init(packageTool, "macos")])
955+
case .linkage:
956+
try results.checkDependencies(of: packageLib2, are: [])
957+
}
954958
results.delegate.checkNoDiagnostics()
955959
}
956960
}

0 commit comments

Comments
 (0)