-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Cross‐Compilation Overrides #2604
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
The head ref may contain hidden characters: "sdg\u2010destination"
Conversation
|
Thanks! This looks good, just one minor comment. |
| to: { $0.customCompileDestination = $1.path }) | ||
| binder.bind( | ||
| option: parser.add(option: "--triple", kind: String.self), | ||
| to: { $0.customCompileTriple = try Triple($1) }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a part of this PR but at some point, we should probably allow taking an array of triples in order to produce fat binaries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, can we call this --target? CLI frontends like clang and swiftc typically use the target terminology, whereas triple is more of an internal terminology.
"triple" is also inaccurate these days since most "triples" (Mac Catalyst, iOS/tvOS/watchOS Simulator, Linux, Android, Windows) actually have four components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn’t --target already exist and select a target module to build as opposed to a target platform?
It’s what I started with, and then when I realized it would need to be --target-triple to disambiguate. At that point the word target seemed redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup.
$ swift build --help
[...]
--target Build the specified targetThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m still open to alternate suggestions though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an internal flag right and the term Triple is well know so I fine with both --triple or --target-triple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Argh, forgot about that. Alright, let's go with --triple I guess.
|
I made the requested change. I’m rerunning the external CI to make sure I didn’t typo anything. The log is here. I’ll ping again when I notice it has finished. |
|
Oops. I forgot to update the tests’s flags to match. The new run is here. |
|
It succeeded. |
aciidgh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
@swift-ci smoke test |
|
@swift-ci smoke test linux |
This adds command line options to override individual aspects of the destination.
It enables cross‐compilation to be done without the need to write out a separate JSON file, and it simplifies one‐off experiments during testing. It also paves the way for SwiftPM to start looking for things in standard locations based on the supplied triple (but that is not implemented here).
The new options are just as hidden as the original
--destinationoption that served as their inspiration.The aspects of the JSON file not touched on here were already available through other means anyway. (e.g.
-Xswiftc,-Xcc)Here is a diff that also shows the simplification it allows to an external integration test, which passes before and after.