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

Allow the library consumer to specify the Android API level #570

Closed
wants to merge 3 commits into from

Conversation

danakj
Copy link

@danakj danakj commented Oct 13, 2023

The build.rs script will use the C preprocessor to pull the API level out of C headers by default. But for build systems
that wish to control each build step, and rely on remote compilation, this presents problems.

If --cfg=no_cc is specified, the the C compiler will not be used. Instead the __ANDROID_API__ environment variable
is used to specify the API level.

The build.rs script will use the C preprocessor to pull the API level
out of C headers by default. But for build systems that wish to control
each build step, and rely on remote compilation, this presents problems.

Remove the requirement to run the C preprocessor if the API level is
specified via `--cfg`:
* android_api_at_least_0: No minimum API level is guaranteed.
* android_api_at_least_21: The API level will be at least 21.
The 'no_cc' cfg var controls whether cc is used or the env var.
Copy link

github-actions bot commented Mar 8, 2024

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

  • Original binary size: 140,800 B
  • Updated binary size: 138,752 B
  • Difference: -2,048 B (-1.45%)

@workingjubilee
Copy link
Member

@danakj Sorry for not getting around to this for a bit. I'm feeling a bit thick and now rust-lang/rust is using cc-rs 1.0.90, so where are we in wanting this?

@danakj
Copy link
Author

danakj commented Mar 11, 2024

@danakj Sorry for not getting around to this for a bit. I'm feeling a bit thick and now rust-lang/rust is using cc-rs 1.0.90, so where are we in wanting this?

No problem. In rust-lang/rust#116318 (comment) you noted that the NDK R26 supports API L21 as its minimum, at which point the build-time check here can go away entirely, and we just always do the dl_iterate_phdr thing.

We saw that Rust was still on NDK R25, and it looks to still be.

So if we want to leave Rust on R25 but use the backtrace-rs build script in std, then we would still want to put cc behind a no_cc cfg flag. I can update this PR as it has merge conflicts if you think that's the right direction to go still.

@workingjubilee
Copy link
Member

@danakj It looks like rust-lang/rust#120593 will advance, and when that does, you can simply make this the PR that removes the build check.

@danakj
Copy link
Author

danakj commented Mar 12, 2024

Oh excellent. Thanks!

@danakj
Copy link
Author

danakj commented Jul 26, 2024

Wow it finally merged! I am about to go on vacation though, unlucky. If there's no rush I will address this after I am back. (Feel free to steal the work if someone would like to.)

@workingjubilee
Copy link
Member

yeah I am surprised it took this long lmao.

@fengys1996
Copy link
Contributor

Wow it finally merged! I am about to go on vacation though, unlucky. If there's no rush I will address this after I am back. (Feel free to steal the work if someone would like to.)

I'm interested in this and want to try it. I have submitted a new PR. #656 PTAL if u have time. @workingjubilee

@workingjubilee
Copy link
Member

656 was merged, so!

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.

3 participants