Skip to content

Use the toolset paths from sdk configure to override an SDK bundle's toolset paths#10016

Open
finagolfin wants to merge 1 commit into
swiftlang:mainfrom
finagolfin:toolset
Open

Use the toolset paths from sdk configure to override an SDK bundle's toolset paths#10016
finagolfin wants to merge 1 commit into
swiftlang:mainfrom
finagolfin:toolset

Conversation

@finagolfin
Copy link
Copy Markdown
Member

#9229 finally got swift sdk configure working to apply local config overrides to target triples in SDK bundles when compiling with the native build system, but I noticed when working on that that the configured toolset paths are not applied. This is a better version of the commit I had added there, but more deliberation is still needed.

Right now, this will do the same as when the --toolset flag is added on the command-line, ie merge any newly configured toolset flags into the toolset from the original SDK bundle. However, unlike the --toolset flag, those using sdk configure probably expect to override the original toolset in the SDK bundle altogether, as they do with the other paths.

That will require more changes wiping the original toolset that is probably decoded from the SDK bundle and separately initialized.

Since this swift sdk configure feature never affected compilation until now, we have a free hand here to decide how it should work for toolsets for the first time.

@owenv, you will have to decide this for your similar swift-build pull, #10013, let me know what you're doing there.

@MaxDesiatov and @jakepetroules, let me know what you think this should do for user-configured toolset paths.

)
let originalToolsetPaths = swiftSDK.pathsConfiguration.toolsetPaths ?? []
try configurationStore.readConfiguration(sdkID: ID, sdk: &swiftSDK)
loadedToolsetPaths = !originalToolsetPaths.elementsEqual(swiftSDK.pathsConfiguration.toolsetPaths ?? [])
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This makes sure the user-configured toolset path is different from the one in the bundle, otherwise do nothing below.

@finagolfin
Copy link
Copy Markdown
Member Author

@swift-ci test linux

@owenv
Copy link
Copy Markdown
Contributor

owenv commented May 1, 2026

Personally I think the toolsets should stack, but maybe others will disagree. I would like us to clearly document this to set expectations going forward. Initially maybe just in the command help but I'd like to also add an article to the DocC catalog when I have a moment

@finagolfin
Copy link
Copy Markdown
Member Author

finagolfin commented May 3, 2026

Of course some of them will stack, but the question is which ones?

There are currently three ways to set the toolsets for an SDK bundle:

  1. In the SDK bundle itself
  2. Using the swift sdk configure feature, though that toolset currently is not passed to compilation, which this pull fixes
  3. Using the --toolset CLI flag

Since only 1. and 3. are currently enabled in SwiftPM, any toolsets set by 3. are simply merged into those from 1.

I think the right move for 2., once we enable it in this pull, is to override 1., ie 1. is no longer respected once 2. is set. 3. will still merge into 2. or 1., whichever is being used.

This will give users a way to completely override 1., which they currently cannot do with 3. and is how swift sdk configure works for all the other SDK config flags, so it would be more consistent with how that subcommand works.

Since swift sdk configure simply copies the toolset path from the bundle by default, ie if you don't set a new toolset path but change other SDK flags, this override approach will still keep things working as they are in the common case, while giving more advanced users a way to completely override the SDK bundle toolsets if wanted.

@owenv and @jakepetroules, let me know what you think.

@finagolfin finagolfin changed the title Merge the toolset paths from sdk configure with the other toolset paths Use the toolset paths from sdk configure to override an SDK bundle's toolset paths May 4, 2026
@finagolfin finagolfin marked this pull request as ready for review May 4, 2026 16:41
@finagolfin
Copy link
Copy Markdown
Member Author

This implements the above override logic, will add some tests next, if this override approach is agreed to.

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.

2 participants