Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[build] Add the flags to enable cross-compiling the corelibs #33724
[build] Add the flags to enable cross-compiling the corelibs #33724
Changes from all commits
a4ff4db
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I need this later in the new
swift_flags()
method.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.
I thought about making this a list like
cross-compile-hosts
but realistically only one cross-compilation host is ever passed in, so this will do for now. The issue is that each cross-compilation host will require its own separate C/C++ cross-compilation toolchain, which there's no way to pass in tobuild-script
yet. We're able to handle a single cross-compilation host with hacked-in flags like--android-ndk
and--native-clang-tools-path
, but for example, we can't even build multiple Android architectures with a singlebuild-script
invocation yet.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.
@buttaface I think the last sentence is incorrect. I imagine that if one hacked in something similar to what we have on Darwin in the stdlib cmake (which is horrible btw and I am not suggesting this).
That being said, shouldn't this just be found in some notion of an SDK? I think that is how on Darwin this would work.
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.
The reason I said "we can't even build multiple Android architectures with a single build-script invocation yet" in my last sentence is that, for example, we pass in the path to a particular libicu built for one arch when invoking
build-script
. Even on Darwin, only compiling the Swift compiler itself for macOS x86_64 was supported until recently, when support for cross-compiling it from macOS x86_64 to arm64 was added but not the other way, swiftlang/swift-package-manager#3500, but again only a single cross-compilation host is supported on Darwin. Of course, one could hack in further changes to support multiple hosts, but it is currently not supported.As for this just being an SDK, it is more general than that. I use this flag below to find dependencies like libxml2 or libsqlite3, which are additional package dependencies that aren't usually included in cross-compilation SDKs. I found this useful when cross-compiling the toolchain for Android, as I pass in the Termux prefix with those prebuilt library packages installed to this flag and the separate Android NDK, that has the libc sysroot but no libraries like libxml2, to the
--android-ndk
flag above.This gives the most flexibility because if they are all in one SDK, you can always pass the same SDK path in to both flags.
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.
Great comment! This is exactly what I wanted!
I think you put in the link twice. But feel free to fix in a subsequent commit.
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.
If you mean the two links here, it is because llbuild calls both
find_package
andfind_program
, which is why I had to tell it not to look in the cross-compilation SDK for programs to run in the host, withCMAKE_FIND_ROOT_PATH_MODE_PROGRAM=NEVER
.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.
Great comment! This is exactly what I wanted!
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.
One question that I have (and I am fine with you doing this in a subsequent commit) is if it would be better to error and force people to specify this. That being said, I don't feel too wedded to this.
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.
(and if we decide to change it, feel free to put it in a different PR)
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.
Hmm, the problem is that wouldn't that break the current cross-compilation of the Apple Silicon toolchain on the macOS x86_64 CI? I tried to keep this as non-intrusive as possible, because I don't want to interfere with other use cases.
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.
Thank you for adding a test!
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.
No problem, I agree with you that it's good to have them. I was just being lazy to not add it, so good that you asked.