[6.3] Error if multiple Swift SDKs match --swift-sdk <selector> and fix searching for SDKs when a --triple is specified alongside#9998
Conversation
|
@swift-ci test |
|
Thanks - Given Mishal's announcement in https://forums.swift.org/t/development-open-for-swift-6-3-1-for-linux-and-windows/85715/8 about the very short timeframe for the 6.3.2 release, in my opinion this is a little too risky to include despite the benefits of getting the improved behavior out to users faster, as preferably we'd have more time to live on the change before it shipped to ensure there's no unexpected emergent behavior. |
|
These bugfix pulls are small and pretty easy to examine to see what they do, plus they only affect SDK search. I think the risk is pretty low, and the fact that all CI just passed suggests there is no unexpected change. |
|
Updated to pull in the one-line library search path fix from #10008 also. This code has been carefully reviewed and is locally contained to only SDK search. Given how bad the devex is with choosing between multiple SDK bundles installed right now, I think we should get these bugfixes in for 6.3.2. If it's a matter of accountability, I will fix any issues that come up from this change, which is easy for me to say, as I expect none. 😉 |
|
@swift-ci test |
|
@swift-ci test windows |
|
This change won’t be included in the Swift 6.3.2 release. If it’s important, please contact the release-mangers on forums.swift.org as soon as possible. |
|
@shahmishal, not important, the SwiftPM managers don't want this in 6.3.2, so I will wait. |
|
@swift-ci test windows |
|
@swift-ci test self hosted windows |
|
@swift-ci test self hosted windows |
…SDK name Extend `selectBundle()` to use the existing `selectSwiftSDK(id:hostTriple:targetTriple)` overload, plus fix the latter to check the full triple string, which affects `swift sdk configure` also. Fixes swiftlang#7973 and swiftlang#9220
### Motivation:
SDK search assumed you knew what you're doing and would pick any SDK that matched, time to make that more strict.
### Modifications:
Collect all matches instead, then spit out different errors if there were multiple matches.
### Result:
Emphasize to SDK users that they need to have their installed SDKs not contain the same target triples, when selecting with a triple alone.
|
I had to rebase to get the CI to apply others' merged testing fixes from the 6.3 branch. @swift-ci test |
|
@swift-ci smoke test |
1 similar comment
|
@swift-ci smoke test |
|
@swift-ci test |
|
@swift-ci test windows |
|
Passed all CI, just waiting on review. |
|
@owenv or @jakepetroules, can we get this into 6.3.3, or should we wait till a patch release that ships for mac too? |
|
@rconnell9 is the branch manager. |
|
I think we want to primarily restrict patch releases to high-impact regressions with low risk fixes since the window for users to provide feedback tends to be short. I'd consider this a low-to-moderate risk change and not fixing a recent regression, so IMO it should release with 6.4 (where it's already on the branch) |
|
I don't think many people have been using the You're right that these bugs have been around for years, but given the recent release of the Android SDK and its seeming popularity, I think this is worth fixing in 6.3 now. |
|
I agree with @owenv; it definitely has the potential to trip up workflows and that doesn't seem like something appropriate for a dot-release - thank you for the PR, but let's hold off until the 6.4 release. |
|
Trip up workflows how? This changes no existing correct behavior: it only fixes bugs or correctly flags them and errors. I think you might be getting these three trunk pulls confused with other ones that changed workflows, which these don't. |
|
Hi @finagolfin, as a few others mentioned above, this change is better suited for the next major release. I believe it should already be available in the 6.4 snapshots. |
|
@rconnell9, yes, it will be fixed in 6.4 for sure. I think we should get this into 6.3 also since it is a pure bugfix pull with minimal build impact: it only makes SDK devs' lives much easier when searching for and using SDKs, plus fixes that |
|
This change won’t be included in the 6.3.3 release because the window for merging is now closed. |
|
@shahmishal, sure, I knew we had missed that June 12 deadline by now. If there will be another 6.3 patch release, I think we should get these fixes in, as I don't think the above concerns are warranted, ie this is a very low-risk fix that only touches SDK search and one line of SDK config, so it cannot break anything while greatly improving the devex of the minority who use the new Swift SDK bundles. |
|
Ping @owenv and @shahmishal, I'd like to get this in for the upcoming 6.3.4 release. We have several months of data now that these bugfix pulls have caused zero problems on 6.4 and 6.5 where they have been merged already. It might seem that there is not much benefit to merging this mostly devex change so late in the 6.3 cycle, but many keep testing Swift packages for a year or two with older releases, so all those CI invocations will all have to special case this older SwiftPM 6.3 that doesn't allow properly selecting the SDK triple with |
|
Because 6.3.4 was announced in https://forums.swift.org/t/development-open-for-swift-6-3-4-for-non-darwin-platforms/88288 as a non-Darwin release, I don't think it makes sense to take this as it would cause CLI behavior to diverge on different host platforms |
|
Yeah, I saw that, think it's okay because most CI are only running this SDK selection on linux anyway: our own official workflows do not even integrate building with these SDK bundles on macOS yet. By the time anyone does, 6.4 will be out. 😉 |
6 participants
Explanation: The
--tripleflag has never worked with SDK bundles, in part because--swift-sdkwould just return the first SDK that matched, ie the results were indeterminate if there were multiple matches. Start erroring instead for the latter and make sure the former works. Also, fix a bug in reading library search paths from SDK bundles.Scope: Only affects SDK selection from bundles, plus the small library search bugfix
Issues:
--swift-sdkoption cannot uniquely select a Swift SDK #7973 and SwiftPM selects wrong architecture from multi-arch Swift SDK when using --swift-sdk and --triple #9220Original PRs: Error if multiple Swift SDKs match
--swift-sdk <selector>#9937, Fix searching for SDKs when a--tripleis specified along with the SDK name #9955, Fix initializinglibrarySearchPathsin UserToolchain #10008Risk: Low, works well in trunk
Testing: Passed all trunk CI and my local usage
Reviewers: @jakepetroules and @owenv