Skip to content
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

Merged
merged 3 commits into from
Oct 11, 2023

Conversation

jsclary
Copy link
Contributor

@jsclary jsclary commented Aug 15, 2023

Changes clang link phase --target parameter to match the build target triple.

resolves #126

@jsclary
Copy link
Contributor Author

jsclary commented Aug 15, 2023

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 rustup target is the only current blocker and all it does is grab static.rust-lang.org/dist/channel-rust-stable.toml, download the appropriate archive and extract it into a lib folder. The only things missing from x doctor on my Termux setup are rustup, ideviceinstaller and mksquashfs and it wouldn't take long for me to make a rustup script that just does the target command.

@jsclary jsclary marked this pull request as ready for review August 15, 2023 15:53
Copy link
Member

@MarijnS95 MarijnS95 left a 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 :)

@MarijnS95
Copy link
Member

MarijnS95 commented Sep 14, 2023

Tested, compilation now succeeds on a simple sample project.

@MarijnS95
Copy link
Member

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.

That sounds like an arbitrary barrier to me. If the host has clang it should all just work?

@MarijnS95
Copy link
Member

I've started to solve them (for Termux) in #138 but it's a bit of a mess.

Comment on lines 141 to 144
for target in self.env().target().compile_targets() {
self.rustup_target(target.rust_triple()?)?;
Copy link
Member

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MarijnS95 added a commit that referenced this pull request Sep 14, 2023
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.
MarijnS95 added a commit that referenced this pull request Sep 14, 2023
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.
@MarijnS95
Copy link
Member

@jsclary can you rebase this so that we can merge it?

@jsclary
Copy link
Contributor Author

jsclary commented Oct 10, 2023

@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.

@jsclary
Copy link
Contributor Author

jsclary commented Oct 10, 2023

@jsclary can you rebase this so that we can merge it?

Oh, wait... there is a mistake. I missed that you merged in the generalization already so it's in there twice for Android.

@jsclary
Copy link
Contributor Author

jsclary commented Oct 10, 2023

@jsclary can you rebase this so that we can merge it?

I think I fixed it.

@MarijnS95 MarijnS95 merged commit 268939a into rust-mobile:master Oct 11, 2023
34 checks passed
Athosvk pushed a commit to Traverse-Research/xbuild that referenced this pull request Oct 22, 2024
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Builds for x86_64-linux-android link with aarch64 target and fail.
2 participants