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

Cargo APK 0.9.3 and later does not appear to respect rustflags in config.toml #22

Open
Hoodad opened this issue Nov 18, 2022 · 6 comments · Fixed by rust-mobile/ndk#357

Comments

@Hoodad
Copy link

Hoodad commented Nov 18, 2022

Hello!

It appears that when building using cargo-apk 0.9.3 and later the RUSTFLAGS set in /.cargo/config.toml are not being included. In our code-base we depend on the flag rust flag "--cfg tokio_unstable" being set. However when building this flag is not being set. Earlier version of cargo-apk like 0.9.2 does not show this behavior.

Quickly skimming the diff there appears to have been some changes to how flags are being handled, though I haven't looked to close yet.

@MarijnS95
Copy link
Member

Your colleague already addressed that in rust-mobile/ndk#357, but I have yet to make a release pending reviews on my other cargo-apk related PRs.

@Hoodad
Copy link
Author

Hoodad commented Nov 18, 2022

Oh 😅! Thanks, closing this issue then!

@Hoodad Hoodad closed this as completed Nov 18, 2022
@MarijnS95
Copy link
Member

Fwiw the breakage in cargo-apk 0.9.2 => 0.9.3 came paired with ndk-build 0.6.0 => 0.7.0, where rust-mobile/ndk#298 landed.

@MarijnS95
Copy link
Member

@Hoodad @Jake-Shadle you may also be interested in rust-mobile/ndk#260 / rust-mobile/ndk#365 regarding your internal usage of cargo-apk and other android-ndk-rs crates; community feedback from actual users on moving forward is invaluable, let me know how you feel about this situation!

@Hoodad
Copy link
Author

Hoodad commented Apr 5, 2023

Thanks for the heads up and sorry for being a bit vacant and not replying to your question @MarijnS95 There will be some additional speed on Android side now going forward. We're currently inventorying what we need going forward and looking at what options are available.

Regarding the reason why I created the issue in the first place. I have tested latest version of cargo-apk and it still fails to respect the rust flags. I created a minimal example reproducing the issue here https://github.com/Hoodad/cargo-apk-rust-flags-repro

Even if cargo-apk will become (or maybe already is) deprecated I wanted to feedback that the issue still persist.

@Hoodad Hoodad reopened this Apr 5, 2023
@MarijnS95 MarijnS95 transferred this issue from rust-mobile/ndk Apr 5, 2023
@MarijnS95
Copy link
Member

So the issue is simply that all these ways of setting rustflags are mutually exclusive. Meaning that because cargo-apk (and IIRC also xbuild) set CARGO_ENCODED_RUSTFLAGS we are suddenly responsible for scanning for all the four sources of rustflags with the given precedence ordering and append them to CARGO_ENCODED_RUSTFLAGS. We currently only do this for RUSTFLAGS and CARGO_ENCODED_RUSTFLAGS, but would have to expand to target.<triple>.rustflags + target.<cfg>.rustflags and build.rustflags shown here.

@MarijnS95 MarijnS95 changed the title Cargo APK 0.9.3 and later does not appear to respect RUSTFLAGS Cargo APK 0.9.3 and later does not appear to respect rustflags in config.toml May 17, 2023
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 a pull request may close this issue.

2 participants