From bc6eb22b2871cfbef73f64445d4c38ff998791b4 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Thu, 23 Jun 2022 11:39:50 +0200 Subject: [PATCH] ndk-build: Invoke `clang` directly instead of through wrapper scripts The NDK's wrapper scripts for `clang`/`clang++` currently only, and will only ever pass `--target` to the compiler. That is something we can do ourselves, especially now that we're already updating `RUSTFLAGS` anyway. [Upstream even clarified] that it is recommended to pass `--target` yourself instead of incurring extra overhead while going through the wrapper scripts (especially costly on Windows), further guaranteeing that we won't miss out on any flags possibly being added to the wrapper scripts in the future. [Upstream even clarified]: https://r.android.com/2134712 --- ndk-build/CHANGELOG.md | 1 + ndk-build/src/apk.rs | 2 +- ndk-build/src/cargo.rs | 59 ++++++++++++++++++++++++++++-------------- ndk-build/src/error.rs | 4 +-- ndk-build/src/lib.rs | 28 ++++++++++---------- ndk-build/src/ndk.rs | 25 +++++++++--------- 6 files changed, 70 insertions(+), 49 deletions(-) diff --git a/ndk-build/CHANGELOG.md b/ndk-build/CHANGELOG.md index 3d7c9514..e2288961 100644 --- a/ndk-build/CHANGELOG.md +++ b/ndk-build/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased - Allow NDK r23 `-lgcc` workaround to work for target directories containing spaces. ([#298](https://github.com/rust-windowing/android-ndk-rs/pull/298)) +- Invoke `clang` directly instead of through the NDK's wrapper scripts. ([#306](https://github.com/rust-windowing/android-ndk-rs/pull/306)) # 0.6.0 (2022-06-11) diff --git a/ndk-build/src/apk.rs b/ndk-build/src/apk.rs index 1610a4e6..2ba1e047 100644 --- a/ndk-build/src/apk.rs +++ b/ndk-build/src/apk.rs @@ -116,7 +116,7 @@ impl<'a> UnalignedApk<'a> { search_paths: &[&Path], ) -> Result<(), NdkError> { let abi_dir = path.join(target.android_abi()); - for entry in fs::read_dir(&abi_dir).map_err(|e| NdkError::IoPathError(e, abi_dir))? { + for entry in fs::read_dir(&abi_dir).map_err(|e| NdkError::IoPathError(abi_dir, e))? { let entry = entry?; let path = entry.path(); if path.extension() == Some(OsStr::new("so")) { diff --git a/ndk-build/src/cargo.rs b/ndk-build/src/cargo.rs index d46474d5..dd7e5f66 100644 --- a/ndk-build/src/cargo.rs +++ b/ndk-build/src/cargo.rs @@ -11,12 +11,35 @@ pub fn cargo_ndk( target_dir: impl AsRef, ) -> Result { let triple = target.rust_triple(); + let clang_target = format!("--target={}{}", target.ndk_llvm_triple(), sdk_version); let mut cargo = Command::new("cargo"); - let (clang, clang_pp) = ndk.clang(target, sdk_version)?; + // Read initial RUSTFLAGS + let mut rustflags = match std::env::var("CARGO_ENCODED_RUSTFLAGS") { + Ok(val) => val, + Err(std::env::VarError::NotPresent) => "".to_string(), + Err(std::env::VarError::NotUnicode(_)) => { + panic!("RUSTFLAGS environment variable contains non-unicode characters") + } + }; + + let (clang, clang_pp) = ndk.clang()?; + + // Configure cross-compiler for `cc` crate + // https://github.com/rust-lang/cc-rs#external-configuration-via-environment-variables cargo.env(format!("CC_{}", triple), &clang); + cargo.env(format!("CFLAGS_{}", triple), &clang_target); cargo.env(format!("CXX_{}", triple), &clang_pp); + cargo.env(format!("CXXFLAGS_{}", triple), &clang_target); + + // Configure LINKER for `rustc` + // https://doc.rust-lang.org/beta/cargo/reference/environment-variables.html#configuration-environment-variables cargo.env(cargo_env_target_cfg("LINKER", triple), &clang); + if !rustflags.is_empty() { + rustflags.push('\x1f'); + } + rustflags.push_str("-Clink-arg="); + rustflags.push_str(&clang_target); let ar = ndk.toolchain_bin("ar", target)?; cargo.env(format!("AR_{}", triple), &ar); @@ -32,32 +55,28 @@ pub fn cargo_ndk( 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"); + std::fs::create_dir_all(&cargo_apk_link_dir) + .map_err(|e| NdkError::IoPathError(cargo_apk_link_dir.clone(), e))?; + let libgcc = cargo_apk_link_dir.join("libgcc.a"); + std::fs::write(&libgcc, "INPUT(-lunwind)").map_err(|e| NdkError::IoPathError(libgcc, e))?; // 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("CARGO_ENCODED_RUSTFLAGS") { - Ok(val) => val, - Err(std::env::VarError::NotPresent) => "".to_string(), - Err(std::env::VarError::NotUnicode(_)) => { - panic!("RUSTFLAGS environment variable contains non-unicode characters") - } - }; - if !rustflags.is_empty() { - rustflags.push('\x1f'); - } - rustflags += "-L\x1f"; - rustflags += cargo_apk_link_dir - .to_str() - .expect("Target dir must be valid UTF-8"); - cargo.env("CARGO_ENCODED_RUSTFLAGS", rustflags); + // suggests to resort to RUSTFLAGS. + // Note that `rustflags` will never be empty because of an unconditional `.push_str` above, + // so we can safely start with appending \x1f here. + rustflags.push_str("\x1f-L\x1f"); + rustflags.push_str( + cargo_apk_link_dir + .to_str() + .expect("Target dir must be valid UTF-8"), + ); } + cargo.env("CARGO_ENCODED_RUSTFLAGS", rustflags); + Ok(cargo) } diff --git a/ndk-build/src/error.rs b/ndk-build/src/error.rs index 7fa08cf9..2055300f 100644 --- a/ndk-build/src/error.rs +++ b/ndk-build/src/error.rs @@ -39,8 +39,8 @@ pub enum NdkError { UnsupportedHost(String), #[error(transparent)] Io(#[from] IoError), - #[error("{0:?}: `{1}`")] - IoPathError(#[source] IoError, PathBuf), + #[error("IoError on `{0:?}`: {1}")] + IoPathError(PathBuf, #[source] IoError), #[error("Invalid semver")] InvalidSemver, #[error("Command `{}` had a non-zero exit code.", format!("{:?}", .0).replace('"', ""))] diff --git a/ndk-build/src/lib.rs b/ndk-build/src/lib.rs index 2f556c9f..a4323002 100644 --- a/ndk-build/src/lib.rs +++ b/ndk-build/src/lib.rs @@ -1,21 +1,21 @@ macro_rules! bin { - ($bin:expr) => {{ - #[cfg(not(target_os = "windows"))] - let bin = $bin; - #[cfg(target_os = "windows")] - let bin = concat!($bin, ".exe"); - bin - }}; + ($bin:expr) => { + if cfg!(target_os = "windows") { + concat!($bin, ".exe") + } else { + $bin + } + }; } macro_rules! bat { - ($bat:expr) => {{ - #[cfg(not(target_os = "windows"))] - let bat = $bat; - #[cfg(target_os = "windows")] - let bat = concat!($bat, ".bat"); - bat - }}; + ($bat:expr) => { + if cfg!(target_os = "windows") { + concat!($bat, ".bat") + } else { + $bat + } + }; } pub mod apk; diff --git a/ndk-build/src/ndk.rs b/ndk-build/src/ndk.rs index 3d20bd28..3310e1ed 100644 --- a/ndk-build/src/ndk.rs +++ b/ndk-build/src/ndk.rs @@ -228,21 +228,21 @@ impl Ndk { Ok(toolchain_dir) } - pub fn clang(&self, target: Target, platform: u32) -> Result<(PathBuf, PathBuf), NdkError> { - #[cfg(target_os = "windows")] - let ext = ".cmd"; - #[cfg(not(target_os = "windows"))] - let ext = ""; + pub fn clang(&self) -> Result<(PathBuf, PathBuf), NdkError> { + let ext = if cfg!(target_os = "windows") { + "exe" + } else { + "" + }; - let bin_name = format!("{}{}-clang", target.ndk_llvm_triple(), platform); let bin_path = self.toolchain_dir()?.join("bin"); - let clang = bin_path.join(format!("{}{}", &bin_name, ext)); + let clang = bin_path.join("clang").with_extension(ext); if !clang.exists() { return Err(NdkError::PathNotFound(clang)); } - let clang_pp = bin_path.join(format!("{}++{}", &bin_name, ext)); + let clang_pp = bin_path.join("clang++").with_extension(ext); if !clang_pp.exists() { return Err(NdkError::PathNotFound(clang_pp)); } @@ -251,10 +251,11 @@ impl Ndk { } pub fn toolchain_bin(&self, name: &str, target: Target) -> Result { - #[cfg(target_os = "windows")] - let ext = ".exe"; - #[cfg(not(target_os = "windows"))] - let ext = ""; + let ext = if cfg!(target_os = "windows") { + ".exe" + } else { + "" + }; let toolchain_path = self.toolchain_dir()?.join("bin");