Skip to content

Commit

Permalink
Use CARGO_ENCODED_RUSTFLAGS instead of RUSTFLAGS
Browse files Browse the repository at this point in the history
https://doc.rust-lang.org/cargo/reference/config.html#buildrustflags

There are four mutually exclusive sources of extra flags. They are checked in order, with the first one being used:

1. `CARGO_ENCODED_RUSTFLAGS` environment variable.
2. `RUSTFLAGS` environment variable.
3. All matching `target.<triple>.rustflags` and `target.<cfg>.rustflags` config entries joined together.
4. `build.rustflags` config value.

This requires Rust 1.55+, our MSRV is 1.62 ATM so it's fine.
  • Loading branch information
messense committed Jan 10, 2023
1 parent 1409676 commit b6aad5b
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 27 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 17 additions & 25 deletions src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,9 @@ fn compile_target(
let target_triple = target.target_triple();

let manifest_dir = context.manifest_path.parent().unwrap();
let mut rust_flags = get_rustflags(manifest_dir, target_triple)?;
let mut rustflags = cargo_config2::Config::load_with_cwd(manifest_dir)?
.rustflags(target_triple)?
.unwrap_or_default();

// We need to pass --bin / --lib
match bridge_model {
Expand All @@ -191,12 +193,15 @@ fn compile_target(
// https://github.com/rust-lang/rust/issues/59302#issue-422994250
// We must only do this for libraries as it breaks binaries
// For some reason this value is ignored when passed as rustc argument
if context.target.is_musl_target() {
let flags = rust_flags.get_or_insert_with(Default::default);
if !flags.contains("target-feature=-crt-static") {
debug!("Setting `-C target-features=-crt-static` for musl dylib");
flags.push_str(" -C target-feature=-crt-static");
}
if context.target.is_musl_target()
&& !rustflags
.flags
.iter()
.any(|f| f == "target-feature=-crt-static")
{
debug!("Setting `-C target-features=-crt-static` for musl dylib");
rustflags.push("-C");
rustflags.push("target-feature=-crt-static");
}
}
}
Expand Down Expand Up @@ -228,11 +233,11 @@ fn compile_target(
cargo_rustc.args.extend(mac_args);
}
} else if target.is_emscripten() {
let flags = rust_flags.get_or_insert_with(Default::default);
// Allow user to override these default flags
if !flags.contains("link-native-libraries") {
if !rustflags.flags.iter().any(|f| f == "link-native-libraries") {
debug!("Setting `-Z link-native-libraries=no` for Emscripten");
flags.push_str(" -Z link-native-libraries=no");
rustflags.push("-Z");
rustflags.push("link-native-libraries=no");
}
let mut emscripten_args = Vec::new();
// Allow user to override these default settings
Expand Down Expand Up @@ -336,8 +341,8 @@ fn compile_target(
// but forwarding stderr is still useful in case there some non-json error
.stderr(Stdio::inherit());

if let Some(flags) = rust_flags {
build_command.env("RUSTFLAGS", flags);
if !rustflags.flags.is_empty() {
build_command.env("CARGO_ENCODED_RUSTFLAGS", rustflags.encode()?);
}

if let BridgeModel::BindingsAbi3(_, _) = bridge_model {
Expand Down Expand Up @@ -509,19 +514,6 @@ fn compile_target(
Ok(artifacts)
}

/// Get `RUSTFLAGS` in the following order:
///
/// 1. `RUSTFLAGS` environment variable
/// 2. `rustflags` cargo configuration
fn get_rustflags(workdir: &Path, target: &str) -> Result<Option<String>> {
let cargo_config = cargo_config2::Config::load_with_cwd(workdir)?;
let rustflags = cargo_config.rustflags(target)?;
match rustflags {
Some(rustflags) => Ok(Some(rustflags.encode_space_separated()?)),
None => Ok(None),
}
}

/// Checks that the native library contains a function called `PyInit_<module name>` and warns
/// if it's missing.
///
Expand Down

0 comments on commit b6aad5b

Please sign in to comment.