-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Disuse unversioned triples on non-Darwin platforms. #3576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disuse unversioned triples on non-Darwin platforms. #3576
Conversation
Btw, once TSC can handle versioned triples with your pull, swiftlang/swift-tools-support-core#229, does SPM properly pass those along to the Swift compiler, particularly when cross-compiling, or is further patching needed in SPM? I need to modify TSC to handle Android versions too, which are appended to the environment, not the OS. |
I can only really speak affirmatively to the platforms I have access to, but the |
4b7acf2
to
7e5db61
Compare
I think this should work, but why not switch macOS to versioned triples too instead? @aciidb0mb3r, any reason not to do that now that TSC would support it with his TSC pull? |
@swift-ci please smoke test |
f57e544
to
2ec5b89
Compare
Tests have been updated and some code cleaned up a little to factor this recurring pattern. |
@swift-ci please smoke test |
2ec5b89
to
795a205
Compare
@swift-ci please smoke test |
795a205
to
4ac062a
Compare
The extension is moved to Basics now to make sure that it is accessible everywhere, including in all relevant tests. All uses of |
@swift-ci please smoke test |
1 similar comment
@swift-ci please smoke test |
4ac062a
to
d52eb0f
Compare
As edited, amended filter |
@neonichu What is the next step here? |
Btw, I was just looking at the much more extensive Triple struct in SwiftDriver, which already supports API versioning for Android though perhaps not OpenBSD, as it was a port of the LLVM triple. Should we switch over to that instead, rather than using this more limited TSCUtility triple, which predates that Swift port? |
Migrating the Triple module from the Driver is probably a bigger lift for another PR, I suspect, but the others on the pr may still want to comment on whether that works on a more tactical basis. |
I think in principle that should work, but IIRC we did not want to depend on the driver outside of the targets that already depend on it to avoid the data model library product taking on that dependency. It looks like @abertelrud might have more context. |
5c034dd
to
858b97a
Compare
b7e2e59
to
eb54bad
Compare
eb54bad
to
7ea746d
Compare
7ea746d
to
69d8f60
Compare
The target flag should take the versioned triple, otherwise on platforms with versioned triples, the standard library won't be found. On Darwin, the unversioned triple should still be used throughout. This necessitates special-casing in the bootstrap script and when making subdirectories during the build. This platform-specific branch is encapsulated in a small extension on Triple in swiftpm. All tests that use the `tripleString` to construct the `.build` subdirectory are updated accordingly. See TSC pr swiftlang#229.
69d8f60
to
685885a
Compare
Does this need to be run through the CI too or can the three pulls be merged together now? |
swiftlang/swift-tools-support-core#229 |
Ping, these three pulls appear ready, two just need to be approved before all are merged together. |
Yep, I am planning to take a final look this week and approve. |
All three PRs look good to me, we should wait for Ben to approve swiftlang/sourcekit-lsp#430 and then we can land this. |
Some platforms require the normal, versioned triple to be supplied for the target flag, as Swift modules use this versioned triple as a subdirectory. If the unversioned triple is used instead, the standard library won't be found on these platforms. Instead, use the unversioned triple only on Darwin. This also should be a noop on Linux. This behavior matches swiftlang/swift-package-manager#3576 and swiftlang/swift-tools-support-core#229 which contain additional context.
Some platforms require the normal, versioned triple to be supplied for the target flag, as Swift modules use this versioned triple as a subdirectory. If the unversioned triple is used instead, the standard library won't be found on these platforms. Instead, use the unversioned triple only on Darwin. This also should be a noop on Linux. This behavior matches swiftlang/swift-package-manager#3576 and swiftlang/swift-tools-support-core#229 which contain additional context.
Some platforms require the normal, versioned triple to be supplied for the target flag, as Swift modules use this versioned triple as a subdirectory. If the unversioned triple is used instead, the standard library won't be found on these platforms. Instead, use the unversioned triple only on Darwin. This also should be a noop on Linux. This behavior matches swiftlang/swift-package-manager#3576 and swiftlang/swift-tools-support-core#229 which contain additional context.
Some platforms require the normal, versioned triple to be supplied for the target flag, as Swift modules use this versioned triple as a subdirectory. If the unversioned triple is used instead, the standard library won't be found on these platforms. Instead, use the unversioned triple only on Darwin. This also should be a noop on Linux. This behavior matches swiftlang/swift-package-manager#3576 and swiftlang/swift-tools-support-core#229 which contain additional context.
Some platforms require the normal, versioned triple to be supplied for the target flag, as Swift modules use this versioned triple as a subdirectory. If the unversioned triple is used instead, the standard library won't be found on these platforms. Instead, use the unversioned triple only on Darwin. This also should be a noop on Linux. This behavior matches swiftlang/swift-package-manager#3576 and swiftlang/swift-tools-support-core#229 which contain additional context.
Disuse unversioned triples on non-Darwin platforms.
Motivation:
The target flag should take the versioned triple, otherwise on platforms
with versioned triples, the standard library won't be found.
On Darwin, the unversioned triple should still be used throughout. This
necessitates special-casing in the bootstrap script and when making
subdirectories during the build.
See TSC pr #229.
Modifications:
Utilities/bootstrap
: get the versionedtriple
from swiftc, not theunversionedTriple
.Sources/Basics/Triple+Extensions.swift
andSources/Basics/CMakeLists.txt
: introduce a new extension to and method onTriple
from TSC to get the correct.build
subdirectory for Darwin and non-Darwin platforms,platformBuildPathComponent
Sources/Commands/SwiftTool.swift
: ensureplatformBuildPathComponent
is used.BinaryTarget+Extensions.swift
to deal with comparingtripleStrings
instead of triple structs, so we can properly do the unversioning on Darwin.Result:
When bootstrapping swiftpm, the target will be filled with the versioned triple on non-Darwin systems.
This must be applied in concert with the TSC pr.