rust icon indicating copy to clipboard operation
rust copied to clipboard

Invoke `backtrace-rs`' buildscript in `std`'s buildscript for Android targets

Open Arc-blroth opened this issue 2 years ago • 5 comments

On Android targets, backtrace-rs uses the buildscript to detect if the Android API level, which is needed to enable backtrace symbolication on targets with Android API >= 21. This PR ensures that backtrace-rs's buildscript is invoked as part of the standard library directly including backtrace.

Alongside the patch from rust-lang/cc-rs#705, this should fix backtraces not being symbolicated on Android, though I have only tested this when building from Windows.

Note that this PR will be essentially a no-op as of right now, since the Rust CI builds with a minimum Android API level of 14 (see rust-lang/release-team#9). A custom toolchain build with a higher minimum API level is required to actually get backtraces with symbols as of right now.

A config.toml for building with the latest Android NDK
profile = "library"
changelog-seen = 2

[build] target = ["x86_64-pc-windows-msvc", "armv7-linux-androideabi"]
[target.armv7-linux-androideabi] android-ndk = "C:/Users/arcdev/AppData/Local/Android/Sdk/ndk/21.1.6352462/toolchains/llvm/prebuilt/windows-x86_64" cc = "C:/Users/arcdev/AppData/Local/Android/Sdk/ndk/21.1.6352462/toolchains/llvm/prebuilt/windows-x86_64/bin/armv7a-linux-androideabi21-clang.cmd" cxx = "C:/Users/arcdev/AppData/Local/Android/Sdk/ndk/21.1.6352462/toolchains/llvm/prebuilt/windows-x86_64/bin/armv7a-linux-androideabi21-clang++.cmd" linker = "C:/Users/arcdev/AppData/Local/Android/Sdk/ndk/21.1.6352462/toolchains/llvm/prebuilt/windows-x86_64/bin/armv7a-linux-androideabi21-clang++.cmd" llvm-libunwind = "system" ar = "C:/msys64/mingw64/bin/ar.exe"

According to the latest Android Platform API Version Distribution, only about 0.4% of Android devices are on Android API version 19 or below, though the latest NDK still supports a lowest API version of 19 (but only for armv7a-linux-androideabi and i686-linux-android and not the other architectures ???).

Arc-blroth avatar Jul 29 '22 01:07 Arc-blroth

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon.

Please see the contribution instructions for more information.

rust-highfive avatar Jul 29 '22 01:07 rust-highfive

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

rustbot avatar Jul 29 '22 01:07 rustbot

The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
  --> library/std/../backtrace/build.rs:13:9
   |
13 |     let expansion = match cc::Build::new().file("src/android-api.c").try_expand() {
   |
   = help: the trait `Sized` is not implemented for `[u8]`
   = note: all local variables must have a statically known size
   = help: unsized locals are gated as an unstable feature

rust-log-analyzer avatar Jul 29 '22 01:07 rust-log-analyzer

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

rust-log-analyzer avatar Jul 29 '22 01:07 rust-log-analyzer

:umbrella: The latest upstream changes (presumably #101946) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Sep 17 '22 22:09 bors

cc https://github.com/rust-lang/rust/issues/103673, which will raise the NDK version to r25b. I'm not sure which API version that corresponds to.

I think the only impact that should have is that this PR will be more urgent, since people will start losing backtraces. cc @alex-pinkus @chriswailes

jyn514 avatar Dec 27 '22 16:12 jyn514

Status? The NDK change is in.

@rustbot author

workingjubilee avatar Jul 26 '23 03:07 workingjubilee

@Arc-blroth

ping from triage - can you post your status on this PR? There hasn't been an update in a few months. Thanks!

FYI: when a PR is ready for review, send a message containing @rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

JohnCSimon avatar Oct 01 '23 04:10 JohnCSimon

Thanks for checking in John. I had forgotten to open my alternative PR after the backtrace-rs change for it went through.

https://github.com/rust-lang/rust/pull/116318

pitaj avatar Oct 01 '23 15:10 pitaj

By adding a dependency on cc to the standard library, ~this~ https://github.com/rust-lang/rust/pull/116318 greatly complicates the lives of projects which build the standard library in our own build systems (Blueprint, GN, Blaze, Bazel, etc). In these build systems we model all build edges explicitly, and invoking a compiler through cc is incompatible with our systems doing remote complication based on the edges of our build systems.

This has blocked our ability to roll the latest Rust into Chromium as we're no longer able to build the standard library. We will need to find a way to remove the dependency on cc. If we can do that in a way that doesn't involve patching, that would be very appreciated.

Edit: I shall repost the comment there, I mistook which PR landed.

danakj avatar Oct 13 '23 18:10 danakj

Superseded by #116318

Arc-blroth avatar Oct 13 '23 19:10 Arc-blroth