-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Support the x86_64-linux-android build target. #127
Conversation
The for loop for installing targets is probably overkill at the moment but it'll be one less thing to fix if/when support for multi-target packages is added. It's clear someone planned for that eventuality and I'll probably have to keep using other solutions until that works. I settled on removing the target from the linker phase when the target and the host are the same. Android isn't supported as a host (x exits with an error saying as much) so there's no way to test that functions as expected. As it's unreachable now, it shouldn't matter. I'll take a look at making Android a viable build host as a separate PR, though, as it looks like |
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.
Seems simple enough to at least fix these hardcoded strings, that's all? Haven't tested this myself yet :)
Tested, compilation now succeeds on a simple sample project. |
That sounds like an arbitrary barrier to me. If the host has |
I've started to solve them (for Termux) in #138 but it's a bit of a mess. |
xbuild/src/download.rs
Outdated
for target in self.env().target().compile_targets() { | ||
self.rustup_target(target.rust_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.
While we're at it, it looks like we should make this more generic for all platforms.
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.
Excerpt from #127 expanded to all platforms: we already have `rustup_triple()` for all target architectures available, instead of hardcoding one specific architecture per platform.
Excerpt from #127 expanded to all platforms: we already have `rustup_triple()` for all target architectures available, instead of hardcoding one specific architecture per platform.
@jsclary can you rebase this so that we can merge it? |
That should do it? I'm not usually two steps removed from the repo I'm working on so hopefully I did that right. Everything I can see still looks correct and it's building fine for me. |
Oh, wait... there is a mistake. I missed that you merged in the generalization already so it's in there twice for Android. |
I think I fixed it. |
* Initial support for x86_64-linux-android targets. * Improve target selection for clang link phase. * Only add clang target if it differs from the host.
Changes clang link phase --target parameter to match the build target triple.
resolves #126