Skip to content

Commit

Permalink
nkd-build: Propagate RUSTFLAGS into CARGO_ENCODED_RUSTFLAGS (#357)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Jake-Shadle authored Oct 28, 2022
1 parent 3f9df7f commit 21b11fe
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 7 deletions.
4 changes: 3 additions & 1 deletion ndk-build/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.)

Expand Down Expand Up @@ -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
<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))
Expand Down
42 changes: 36 additions & 6 deletions ndk-build/src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>()
.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")
}
};

Expand All @@ -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);
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 21b11fe

Please sign in to comment.