Skip to content

Commit

Permalink
Fix rustflags handling
Browse files Browse the repository at this point in the history
  • Loading branch information
bbqsrc committed Jan 24, 2023
1 parent 60ac1b1 commit 1be6fc5
Showing 1 changed file with 58 additions and 15 deletions.
73 changes: 58 additions & 15 deletions src/cargo.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::BTreeMap;
use std::ffi::{OsStr, OsString};
use std::io::{Result, Write};
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -88,6 +89,54 @@ fn create_libgcc_linker_script_workaround(target_dir: &Utf8PathBuf) -> Result<Ut
Ok(libgcc_workaround_dir)
}

enum RustFlags {
Empty,
Encoded(String),
Plain(String),
}

impl RustFlags {
fn from_env() -> Self {
if let Ok(encoded) = std::env::var("CARGO_ENCODED_RUSTFLAGS") {
return Self::Encoded(encoded);
}

if let Ok(plain) = std::env::var("RUSTFLAGS") {
return Self::Plain(plain);
}

Self::Empty
}

fn append(&mut self, flag: &str) {
match self {
RustFlags::Empty => {
*self = Self::Plain(flag.into());
}
RustFlags::Encoded(encoded) => {
if !encoded.is_empty() {
encoded.push('\x1f');
}
encoded.push_str(flag);
}
RustFlags::Plain(plain) => {
if !plain.is_empty() {
plain.push(' ');
}
plain.push_str(flag);

This comment has been minimized.

Copy link
@MarijnS95

MarijnS95 Jan 24, 2023

The whole reason we convert to CARGO_ENCODED_RUSTFLAGS in ndk-build is to prevent issues with spaces in input arguments, such as -L{libdir} filepaths... By appending this to RUSTFLAGS here you risk breaking that, typically on Windows with the Android/Sdk folder in a users' home directory with a space in their username...

Please also see the existing discussion in rust-mobile/ndk#357.

This comment has been minimized.

Copy link
@bbqsrc

bbqsrc via email Jan 24, 2023

Author Owner

This comment has been minimized.

Copy link
@MarijnS95

MarijnS95 Jan 24, 2023

It's your crate 😉

This comment has been minimized.

Copy link
@bbqsrc

bbqsrc via email Jan 24, 2023

Author Owner

This comment has been minimized.

Copy link
@MarijnS95

MarijnS95 Jan 24, 2023

This isn't a competition, and I'm baffled you see it as such. I'm pointing out an issue in your implementation that CARGO_ENCODED_RUSTFLAGS aims to solve, but is being nullified here.

You should be more open to community review when providing a public crate like this, with pull requests for your own changes instead of pushing straight to main.

But alas, if you feel this is how the "community" around your crate should be, I won't waste my own time being helpful anymore. Good luck handling the fallout.

}
}
}

fn as_env_var(&self) -> Option<(&str, &str)> {
Some(match self {
RustFlags::Encoded(x) => ("CARGO_ENCODED_RUSTFLAGS", x),
RustFlags::Plain(x) => ("RUSTFLAGS", x),
RustFlags::Empty => return None,
})
}
}

#[allow(clippy::too_many_arguments)]
pub(crate) fn run(
dir: &Path,
Expand Down Expand Up @@ -117,6 +166,11 @@ pub(crate) fn run(
let bindgen_clang_args_key = format!("BINDGEN_EXTRA_CLANG_ARGS_{}", &triple.replace('-', "_"));
let cargo_bin = std::env::var("CARGO").unwrap_or_else(|_| "cargo".into());

log::trace!(
"Env: {:#?}",
std::env::vars().into_iter().collect::<BTreeMap<_, _>>()
);

log::debug!("cargo: {}", &cargo_bin);
log::debug!("{}={}", &ar_key, &target_ar.display());
log::debug!("{}={}", &cc_key, &target_linker.display());
Expand All @@ -139,14 +193,7 @@ pub(crate) fn run(
log::debug!("Args: {:?}", &cargo_args);

// 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(_)) => {
log::error!("RUSTFLAGS environment variable contains non-unicode characters");
std::process::exit(1);
}
};
let mut rustflags = RustFlags::from_env();

// Insert Cargo arguments before any `--` arguments.
let arg_insertion_position = cargo_args
Expand Down Expand Up @@ -188,13 +235,9 @@ pub(crate) fn run(
// Note that we don't use `cargo rustc` to pass custom library search paths to
// rustc and instead use `CARGO_ENCODED_RUSTFLAGS` because it affects the building
// of all transitive cdylibs (which all need this workaround).
if !rustflags.is_empty() {
// Avoid creating an empty '' rustc argument
rustflags.push('\x1f');
}
rustflags.push_str("-L\x1f");
rustflags.push_str(libdir.as_str());
cargo_cmd.env("CARGO_ENCODED_RUSTFLAGS", rustflags);
rustflags.append(&format!("-L{libdir}"));
let (k, v) = rustflags.as_env_var().unwrap();
cargo_cmd.env(k, v);
}
Err(e) => {
log::error!("Failed to create libgcc.a linker script workaround");
Expand Down

0 comments on commit 1be6fc5

Please sign in to comment.