Skip to content

Conversation

hamishknight
Copy link
Contributor

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

@hamishknight
Copy link
Contributor Author

Linux failure is unrelated, same as e.g https://ci.swift.org/job/pr-swift-build-linux/1074/

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Just checking I understand this - we override the build parameters for the host tool and then those parameters are passed down to all its dependencies and thus the platform check in isTargetSuitableForPlatformForIndex still passes?

@hamishknight
Copy link
Contributor Author

Just checking I understand this - we override the build parameters for the host tool and then those parameters are passed down to all its dependencies and thus the platform check in isTargetSuitableForPlatformForIndex still passes?

Yes that's correct

Factor out the creation of the common package dependencies between
these two tests.
Use Swift Testing parameterization to test different run destinations.
This better matches what we actually generate for packages. Also
set all available platforms as supported.
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.
…ption

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
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.
@hamishknight
Copy link
Contributor Author

@swift-ci please test

@jakepetroules
Copy link
Collaborator

@swift-ci test linux

@jakepetroules
Copy link
Collaborator

Ignore Focal CI failures, that was removed in #676.

@hamishknight hamishknight merged commit c2a6449 into swiftlang:main Jul 29, 2025
41 of 45 checks passed
@hamishknight hamishknight deleted the desc-golf branch July 29, 2025 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants