From 21b11feff1b612656558d435908249faf78c980f Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Fri, 28 Oct 2022 17:28:02 +0200 Subject: [PATCH] nkd-build: Propagate `RUSTFLAGS` into `CARGO_ENCODED_RUSTFLAGS` (#357) ndk-build uses `CARGO_ENCODED_RUSTFLAGS` to get around flags/arguments with spaces in `RUSTFLAGS`, with no defined way to escape them. For correctness this implementation reads `CARGO_ENCODED_RUSTFLAGS` instead of blindly overwriting its contents, but as per [`build.rustflags` precedence rules] setting this environment variable would disregard any contents in the more common `RUSTFLAGS` environment variable. To make this more convenient for users, propagate the contents of `RUSTFLAGS` into `CARGO_ENCODED_RUSTFLAGS` to ensure its contents don't get ignored. [`build.rustflags` precedence rules]: https://doc.rust-lang.org/cargo/reference/config.html#buildrustflags --- ndk-build/CHANGELOG.md | 4 +++- ndk-build/src/cargo.rs | 42 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/ndk-build/CHANGELOG.md b/ndk-build/CHANGELOG.md index 8afad037..277ef739 100644 --- a/ndk-build/CHANGELOG.md +++ b/ndk-build/CHANGELOG.md @@ -1,6 +1,8 @@ # Unreleased - Add `ndk::DEFAULT_DEV_KEYSTORE_PASSWORD` and make `apk::ApkConfig::apk` public. ([#358](https://github.com/rust-windowing/android-ndk-rs/pull/358)) +- `RUSTFLAGS` is now considered if `CARGO_ENCODED_RUSTFLAGS` is not present allowing `cargo apk build` to not break users' builds if they depend on `RUSTFLAGS` being set prior to the build, + as `CARGO_ENCODED_RUSTFLAGS` set by `ndk-build` before invoking `cargo` will take precedence over [all other sources of build flags](https://doc.rust-lang.org/cargo/reference/config.html#buildrustflags). ([#357](https://github.com/rust-windowing/android-ndk-rs/pull/357)) (0.8.1, released on 2022-10-14, was yanked due to violating semver.) @@ -34,7 +36,7 @@ - **Breaking:** Default `target_sdk_version` to `30` or lower (instead of the highest supported SDK version by the detected NDK toolchain) for more consistent interaction with Android backwards compatibility handling and its increasingly strict usage rules: - https://developer.android.com/distribute/best-practices/develop/target-sdk + - **Breaking:** Remove default insertion of `MAIN` intent filter through a custom serialization function, this is better filled in by the default setup in `cargo-apk`. ([#241](https://github.com/rust-windowing/android-ndk-rs/pull/241)) - Add `android:exported` attribute to the manifest's `Activity` element. ([#242](https://github.com/rust-windowing/android-ndk-rs/pull/242)) diff --git a/ndk-build/src/cargo.rs b/ndk-build/src/cargo.rs index dd7e5f66..d3abf348 100644 --- a/ndk-build/src/cargo.rs +++ b/ndk-build/src/cargo.rs @@ -14,12 +14,40 @@ pub fn cargo_ndk( let clang_target = format!("--target={}{}", target.ndk_llvm_triple(), sdk_version); let mut cargo = Command::new("cargo"); - // Read initial RUSTFLAGS + const SEP: &str = "\x1f"; + + // Read initial CARGO_ENCODED_/RUSTFLAGS let mut rustflags = match std::env::var("CARGO_ENCODED_RUSTFLAGS") { - Ok(val) => val, - Err(std::env::VarError::NotPresent) => "".to_string(), + Ok(val) => { + if std::env::var_os("RUSTFLAGS").is_some() { + panic!( + "Both `CARGO_ENCODED_RUSTFLAGS` and `RUSTFLAGS` were found in the environment, please clear one or the other before invoking this script" + ); + } + + val + } + Err(std::env::VarError::NotPresent) => { + match std::env::var("RUSTFLAGS") { + Ok(val) => { + cargo.env_remove("RUSTFLAGS"); + + // Same as cargo + // https://github.com/rust-lang/cargo/blob/f6de921a5d807746e972d9d10a4d8e1ca21e1b1f/src/cargo/core/compiler/build_context/target_info.rs#L682-L690 + val.split(' ') + .map(str::trim) + .filter(|s| !s.is_empty()) + .collect::>() + .join(SEP) + } + Err(std::env::VarError::NotPresent) => String::new(), + Err(std::env::VarError::NotUnicode(_)) => { + panic!("RUSTFLAGS environment variable contains non-unicode characters") + } + } + } Err(std::env::VarError::NotUnicode(_)) => { - panic!("RUSTFLAGS environment variable contains non-unicode characters") + panic!("CARGO_ENCODED_RUSTFLAGS environment variable contains non-unicode characters") } }; @@ -36,7 +64,7 @@ pub fn cargo_ndk( // 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(SEP); } rustflags.push_str("-Clink-arg="); rustflags.push_str(&clang_target); @@ -67,7 +95,9 @@ pub fn cargo_ndk( // 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(SEP); + rustflags.push_str("-L"); + rustflags.push_str(SEP); rustflags.push_str( cargo_apk_link_dir .to_str()