From 4dd596d09f6bc68c0b657472cf8d1b0eaf2b6027 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Fri, 10 Jun 2022 17:43:46 +0200 Subject: [PATCH] cargo-apk,ndk-build: Move NDK r23 `-lgcc` workaround to `cargo_ndk` to reuse in `--` subcommand Winit is currently being hit by `-lgcc not found` linker errors because GH Actions' [virtual-environments recently migrated to Android NDK r23], and it turns out it's using the niche `--` subcommand to invoke regular `cargo` commands under an NDK environment. The workaround using `RUSTFLAGS` is only patched into the `build()` command; by moving it into our `cargo_ndk()` environment preparation function this workaround is universally available and usable. Note that this is a breaking change, we're changing the API signature of `ndk-build`. [virtual-environments recently migrated to Android NDK r23]: https://github.com/actions/virtual-environments/issues/5595 --- cargo-apk/CHANGELOG.md | 2 ++ cargo-apk/src/apk.rs | 56 ++++++++++++++---------------------------- ndk-build/CHANGELOG.md | 2 ++ ndk-build/src/cargo.rs | 41 ++++++++++++++++++++++++++++++- 4 files changed, 62 insertions(+), 39 deletions(-) diff --git a/cargo-apk/CHANGELOG.md b/cargo-apk/CHANGELOG.md index 72ffce20..594a7005 100644 --- a/cargo-apk/CHANGELOG.md +++ b/cargo-apk/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- Move NDK r23 `-lgcc` workaround to `ndk-build::cargo::cargo_ndk`, to also apply to our `cargo apk --` invocations. ([#286](https://github.com/rust-windowing/android-ndk-rs/pull/286)) + # 0.9.1 (2022-05-12) - Reimplement NDK r23 `-lgcc` workaround using `RUSTFLAGS`, to apply to transitive `cdylib` compilations (#270) diff --git a/cargo-apk/src/apk.rs b/cargo-apk/src/apk.rs index 5e9d0286..892ba01d 100644 --- a/cargo-apk/src/apk.rs +++ b/cargo-apk/src/apk.rs @@ -99,7 +99,12 @@ impl<'a> ApkBuilder<'a> { pub fn check(&self) -> Result<(), Error> { for target in &self.build_targets { let triple = target.rust_triple(); - let mut cargo = cargo_ndk(&self.ndk, *target, self.min_sdk_version())?; + let mut cargo = cargo_ndk( + &self.ndk, + *target, + self.min_sdk_version(), + self.cmd.target_dir(), + )?; cargo.arg("check"); if self.cmd.target().is_none() { cargo.arg("--target").arg(triple); @@ -174,47 +179,18 @@ impl<'a> ApkBuilder<'a> { .join(artifact) .join(artifact.file_name(CrateType::Cdylib, triple)); - let mut cargo = cargo_ndk(&config.ndk, *target, self.min_sdk_version())?; + let mut cargo = cargo_ndk( + &config.ndk, + *target, + self.min_sdk_version(), + self.cmd.target_dir(), + )?; cargo.arg("build"); if self.cmd.target().is_none() { cargo.arg("--target").arg(triple); } cargo.args(self.cmd.args()); - // Workaround for https://github.com/rust-windowing/android-ndk-rs/issues/149: - // Rust (1.56 as of writing) still requires libgcc during linking, but this does - // not ship with the NDK anymore since NDK r23 beta 3. - // See https://github.com/rust-lang/rust/pull/85806 for a discussion on why libgcc - // is still required even after replacing it with libunwind in the source. - // XXX: Add an upper-bound on the Rust version whenever this is not necessary anymore. - if self.ndk.build_tag() > 7272597 { - let cargo_apk_link_dir = self - .cmd - .target_dir() - .join("cargo-apk-temp-extra-link-libraries"); - std::fs::create_dir_all(&cargo_apk_link_dir)?; - std::fs::write(cargo_apk_link_dir.join("libgcc.a"), "INPUT(-lunwind)") - .expect("Failed to write"); - - // cdylibs in transitive dependencies still get built and also need this - // workaround linker flag, yet arguments passed to `cargo rustc` are only - // forwarded to the final compiler invocation rendering our workaround ineffective. - // The cargo page documenting this discrepancy (https://doc.rust-lang.org/cargo/commands/cargo-rustc.html) - // suggests to resort to RUSTFLAGS, which are updated below: - let mut rustflags = match std::env::var("RUSTFLAGS") { - Ok(val) => val, - Err(std::env::VarError::NotPresent) => "".to_string(), - Err(std::env::VarError::NotUnicode(_)) => { - panic!("RUSTFLAGS environment variable contains non-unicode characters") - } - }; - rustflags += " -L "; - rustflags += cargo_apk_link_dir - .to_str() - .expect("Target dir must be valid UTF-8"); - cargo.env("RUSTFLAGS", rustflags); - } - if !cargo.status()?.success() { return Err(NdkError::CmdFailed(cargo).into()); } @@ -262,9 +238,13 @@ impl<'a> ApkBuilder<'a> { } pub fn default(&self) -> Result<(), Error> { - let ndk = Ndk::from_env()?; for target in &self.build_targets { - let mut cargo = cargo_ndk(&ndk, *target, self.min_sdk_version())?; + let mut cargo = cargo_ndk( + &self.ndk, + *target, + self.min_sdk_version(), + self.cmd.target_dir(), + )?; cargo.args(self.cmd.args()); if !cargo.status()?.success() { return Err(NdkError::CmdFailed(cargo).into()); diff --git a/ndk-build/CHANGELOG.md b/ndk-build/CHANGELOG.md index 9edbaa54..337ad6dd 100644 --- a/ndk-build/CHANGELOG.md +++ b/ndk-build/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- **Breaking:** Provide NDK r23 `-lgcc` workaround in `cargo_ndk` function, now requiring `target_dir` as argument. ([#286](https://github.com/rust-windowing/android-ndk-rs/pull/286)) + # 0.5.0 (2022-05-07) - **Breaking:** Default `target_sdk_version` to `30` or lower (instead of the highest supported SDK version by the detected NDK toolchain) diff --git a/ndk-build/src/cargo.rs b/ndk-build/src/cargo.rs index 65a07fc8..82c77f25 100644 --- a/ndk-build/src/cargo.rs +++ b/ndk-build/src/cargo.rs @@ -1,9 +1,15 @@ use crate::error::NdkError; use crate::ndk::Ndk; use crate::target::Target; +use std::path::Path; use std::process::Command; -pub fn cargo_ndk(ndk: &Ndk, target: Target, sdk_version: u32) -> Result { +pub fn cargo_ndk( + ndk: &Ndk, + target: Target, + sdk_version: u32, + target_dir: impl AsRef, +) -> Result { let triple = target.rust_triple(); let mut cargo = Command::new("cargo"); @@ -16,6 +22,39 @@ pub fn cargo_ndk(ndk: &Ndk, target: Target, sdk_version: u32) -> Result 7272597 { + let cargo_apk_link_dir = target_dir + .as_ref() + .join("cargo-apk-temp-extra-link-libraries"); + std::fs::create_dir_all(&cargo_apk_link_dir)?; + std::fs::write(cargo_apk_link_dir.join("libgcc.a"), "INPUT(-lunwind)") + .expect("Failed to write"); + + // cdylibs in transitive dependencies still get built and also need this + // workaround linker flag, yet arguments passed to `cargo rustc` are only + // forwarded to the final compiler invocation rendering our workaround ineffective. + // The cargo page documenting this discrepancy (https://doc.rust-lang.org/cargo/commands/cargo-rustc.html) + // suggests to resort to RUSTFLAGS, which are updated below: + let mut rustflags = match std::env::var("RUSTFLAGS") { + Ok(val) => val, + Err(std::env::VarError::NotPresent) => "".to_string(), + Err(std::env::VarError::NotUnicode(_)) => { + panic!("RUSTFLAGS environment variable contains non-unicode characters") + } + }; + rustflags += " -L "; + rustflags += cargo_apk_link_dir + .to_str() + .expect("Target dir must be valid UTF-8"); + cargo.env("RUSTFLAGS", rustflags); + } + Ok(cargo) }