From 3351749906a25806b28aaf084b734f34630395ae Mon Sep 17 00:00:00 2001 From: messense Date: Mon, 9 Jan 2023 22:37:36 +0800 Subject: [PATCH 1/2] Respect `rustflags` settings in cargo configuration file --- Cargo.lock | 46 ++++++++++++++++++++++++++++++++++++++++++---- Cargo.toml | 1 + Changelog.md | 1 + src/compile.rs | 33 +++++++++++++++++++++++++-------- 4 files changed, 69 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 47b45fc4f..3da7f74a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -213,6 +213,20 @@ dependencies = [ "serde", ] +[[package]] +name = "cargo-config2" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59a3dbc2202646fb5a83a0f0f45259be618f006862edc0d20938f247f2678706" +dependencies = [ + "cfg-expr", + "home", + "once_cell", + "serde", + "shell-escape", + "toml_edit", +] + [[package]] name = "cargo-options" version = "0.5.3" @@ -233,19 +247,18 @@ dependencies = [ [[package]] name = "cargo-xwin" -version = "0.13.5" +version = "0.13.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "570ef9c57470aa820aae02ec06456a9e2148149598c9927943ae8afba56ba322" +checksum = "ce9653c9f9630bbdb6c3566f9e77ad568e38eeb4c4849b0913a65aa5c43c12df" dependencies = [ "anyhow", + "cargo-config2", "cargo-options", "clap", "dirs", "fs-err", "indicatif", "path-slash", - "serde", - "serde_json", "which", "xwin", ] @@ -320,6 +333,15 @@ dependencies = [ "uuid", ] +[[package]] +name = "cfg-expr" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a327683d7499ecc47369531a679fe38acdd300e09bf8c852d08b1e10558622bd" +dependencies = [ + "smallvec", +] + [[package]] name = "cfg-if" version = "1.0.0" @@ -1107,6 +1129,15 @@ dependencies = [ "digest 0.9.0", ] +[[package]] +name = "home" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "747309b4b440c06d57b0b25f2aee03ee9b5e5397d288c60e21fc709bb98a7408" +dependencies = [ + "winapi", +] + [[package]] name = "humantime" version = "2.1.0" @@ -1327,6 +1358,7 @@ dependencies = [ "anyhow", "base64", "bytesize", + "cargo-config2", "cargo-options", "cargo-xwin", "cargo-zigbuild", @@ -2338,6 +2370,12 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "shell-escape" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "45bb67a18fa91266cc7807181f62f9178a6873bfad7dc788c42e6430db40184f" + [[package]] name = "shlex" version = "1.1.0" diff --git a/Cargo.toml b/Cargo.toml index 73341c0f4..92b4aca19 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ name = "maturin" anyhow = "1.0.63" base64 = "0.13.0" glob = "0.3.0" +cargo-config2 = "0.1.1" cargo_metadata = "0.15.2" cargo-options = "0.5.2" cbindgen = { version = "0.24.2", default-features = false } diff --git a/Changelog.md b/Changelog.md index fdc9a6662..d23bf9d90 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * **Breaking Change**: Remove deprecated `python-source` option in `Cargo.toml` in [#1335](https://github.com/PyO3/maturin/pull/1335) * **Breaking Change**: Turn `patchelf` version warning into a hard error in [#1335](https://github.com/PyO3/maturin/pull/1335) * **Breaking Change**: [`uniffi_bindgen` CLI](https://mozilla.github.io/uniffi-rs/tutorial/Prerequisites.html#the-uniffi-bindgen-cli-tool) is required for building `uniffi` bindings wheels in [#1352](https://github.com/PyO3/maturin/pull/1352) +* Respect `rustflags` settings in cargo configuration file in [#1405](https://github.com/PyO3/maturin/pull/1405) ## [0.14.9] - 2023-01-10 diff --git a/src/compile.rs b/src/compile.rs index 05d43151c..0cf593156 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -173,7 +173,10 @@ fn compile_target( } } - let mut rust_flags = env::var_os("RUSTFLAGS"); + let target_triple = target.target_triple(); + + let manifest_dir = context.manifest_path.parent().unwrap(); + let mut rust_flags = get_rustflags(manifest_dir, target_triple)?; // We need to pass --bin / --lib match bridge_model { @@ -189,10 +192,11 @@ fn compile_target( // 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() { - debug!("Setting `-C target-features=-crt-static` for musl dylib"); - rust_flags - .get_or_insert_with(Default::default) - .push(" -C target-feature=-crt-static"); + 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"); + } } } } @@ -226,8 +230,9 @@ fn compile_target( } else if target.is_emscripten() { let flags = rust_flags.get_or_insert_with(Default::default); // Allow user to override these default flags - if !flags.to_string_lossy().contains("link-native-libraries") { - flags.push(" -Z link-native-libraries=no"); + if !flags.contains("link-native-libraries") { + debug!("Setting `-Z link-native-libraries=no` for Emscripten"); + flags.push_str(" -Z link-native-libraries=no"); } let mut emscripten_args = Vec::new(); // Allow user to override these default settings @@ -260,7 +265,6 @@ fn compile_target( .extend(["-C".to_string(), "link-arg=-s".to_string()]); } - let target_triple = target.target_triple(); let mut build_command = if target.is_msvc() && target.cross_compiling() { #[cfg(feature = "xwin")] { @@ -505,6 +509,19 @@ 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> { + 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_` and warns /// if it's missing. /// From bc631f137cb06cca97ae6d77df8ed537d4bbbe7c Mon Sep 17 00:00:00 2001 From: messense Date: Tue, 10 Jan 2023 14:14:20 +0800 Subject: [PATCH 2/2] Use `CARGO_ENCODED_RUSTFLAGS` instead of `RUSTFLAGS` 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..rustflags` and `target..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. --- Cargo.lock | 8 ++++---- src/compile.rs | 42 +++++++++++++++++------------------------- 2 files changed, 21 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3da7f74a9..894874b04 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -215,9 +215,9 @@ dependencies = [ [[package]] name = "cargo-config2" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59a3dbc2202646fb5a83a0f0f45259be618f006862edc0d20938f247f2678706" +checksum = "2cac6a46d3472410bc331b230bac4ee58f89e0bb25f02e8cbccb2a211c368f8a" dependencies = [ "cfg-expr", "home", @@ -247,9 +247,9 @@ dependencies = [ [[package]] name = "cargo-xwin" -version = "0.13.6" +version = "0.13.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ce9653c9f9630bbdb6c3566f9e77ad568e38eeb4c4849b0913a65aa5c43c12df" +checksum = "c47154844233c35279d90d5a68beac22907d85442130d7398038197cfc37dbd2" dependencies = [ "anyhow", "cargo-config2", diff --git a/src/compile.rs b/src/compile.rs index 0cf593156..9fbbc3aef 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -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 { @@ -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"); } } } @@ -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 @@ -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 { @@ -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> { - 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_` and warns /// if it's missing. ///