cargo-apk,ndk-build: Move NDK r23 -lgcc workaround to cargo_ndk t…

…o 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]: actions/runner-images#5595

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)

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());

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)

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<Command, NdkError> {
+pub fn cargo_ndk(
+    ndk: &Ndk,
+    target: Target,
+    sdk_version: u32,
+    target_dir: impl AsRef<Path>,
+) -> Result<Command, NdkError> {
     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<Command,
     cargo.env(format!("AR_{}", triple), &ar);
     cargo.env(cargo_env_target_cfg("AR", triple), &ar);
 
+    // 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 ndk.build_tag() > 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)
 }