From 1a0776758734d49f0f4b338ab21ebd3561292805 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 20 Sep 2022 10:56:56 +0100 Subject: [PATCH 1/2] add failing test for CARGO_CFG_TARGET_FEATURE When setting target features via rustflags via `[build]` config key, cargo correctly propagates them to rustc (via -C flag) and to build.rs (via CARGO_CFG_TARGET_FEATURE env var). However, if `[target.cfg]` is used instead, build.rs doesn't get the flags (rustc still gets them though). --- tests/testsuite/build_script_env.rs | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/testsuite/build_script_env.rs b/tests/testsuite/build_script_env.rs index 7de9dd23487..febae9c0cde 100644 --- a/tests/testsuite/build_script_env.rs +++ b/tests/testsuite/build_script_env.rs @@ -173,3 +173,38 @@ fn rustc_bootstrap() { .with_status(101) .run(); } + +#[cargo_test] +#[cfg(target_arch = "x86_64")] +fn build_script_sees_cfg_target_feature() { + let build_rs = r#" + fn main() { + let cfg = std::env::var("CARGO_CFG_TARGET_FEATURE").unwrap(); + eprintln!("CARGO_CFG_TARGET_FEATURE={cfg}"); + } + "#; + + let configs = [ + r#" + [build] + rustflags = ["-Ctarget-feature=+sse4.1,+sse4.2"] + "#, + r#" + [target.'cfg(target_arch = "x86_64")'] + rustflags = ["-Ctarget-feature=+sse4.1,+sse4.2"] + "#, + ]; + + for config in configs { + let p = project() + .file(".cargo/config.toml", config) + .file("src/lib.rs", r#""#) + .file("build.rs", build_rs) + .build(); + + p.cargo("build -vv") + .with_stderr_contains("[foo 0.0.1] CARGO_CFG_TARGET_FEATURE=[..]sse4.2[..]") + .with_stderr_contains("[..]-Ctarget-feature=[..]+sse4.2[..]") + .run(); + } +} From c333b0a7be088de14b7b70cd28fbbc8deaf8da7d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 20 Sep 2022 12:01:50 +0100 Subject: [PATCH 2/2] when learning target information, run a little fixedpoint iteration loop --- .../compiler/build_context/target_info.rs | 222 ++++++++++-------- tests/testsuite/build_script_env.rs | 31 +++ 2 files changed, 155 insertions(+), 98 deletions(-) diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index 72c7c225de1..d37b1b9614d 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -138,7 +138,7 @@ impl TargetInfo { rustc: &Rustc, kind: CompileKind, ) -> CargoResult { - let rustflags = env_args( + let mut rustflags = env_args( config, requested_kinds, &rustc.host, @@ -146,114 +146,140 @@ impl TargetInfo { kind, Flags::Rust, )?; - let extra_fingerprint = kind.fingerprint_hash(); - let mut process = rustc.workspace_process(); - process - .arg("-") - .arg("--crate-name") - .arg("___") - .arg("--print=file-names") - .args(&rustflags) - .env_remove("RUSTC_LOG"); - - if let CompileKind::Target(target) = kind { - process.arg("--target").arg(target.rustc_target()); - } - - let crate_type_process = process.clone(); - const KNOWN_CRATE_TYPES: &[CrateType] = &[ - CrateType::Bin, - CrateType::Rlib, - CrateType::Dylib, - CrateType::Cdylib, - CrateType::Staticlib, - CrateType::ProcMacro, - ]; - for crate_type in KNOWN_CRATE_TYPES.iter() { - process.arg("--crate-type").arg(crate_type.as_str()); - } - let supports_split_debuginfo = rustc - .cached_output( - process.clone().arg("-Csplit-debuginfo=packed"), - extra_fingerprint, - ) - .is_ok(); - - process.arg("--print=sysroot"); - process.arg("--print=cfg"); - - let (output, error) = rustc - .cached_output(&process, extra_fingerprint) - .with_context(|| "failed to run `rustc` to learn about target-specific information")?; + let mut turn = 0; + loop { + let extra_fingerprint = kind.fingerprint_hash(); + let mut process = rustc.workspace_process(); + process + .arg("-") + .arg("--crate-name") + .arg("___") + .arg("--print=file-names") + .args(&rustflags) + .env_remove("RUSTC_LOG"); + + if let CompileKind::Target(target) = kind { + process.arg("--target").arg(target.rustc_target()); + } - let mut lines = output.lines(); - let mut map = HashMap::new(); - for crate_type in KNOWN_CRATE_TYPES { - let out = parse_crate_type(crate_type, &process, &output, &error, &mut lines)?; - map.insert(crate_type.clone(), out); - } + let crate_type_process = process.clone(); + const KNOWN_CRATE_TYPES: &[CrateType] = &[ + CrateType::Bin, + CrateType::Rlib, + CrateType::Dylib, + CrateType::Cdylib, + CrateType::Staticlib, + CrateType::ProcMacro, + ]; + for crate_type in KNOWN_CRATE_TYPES.iter() { + process.arg("--crate-type").arg(crate_type.as_str()); + } + let supports_split_debuginfo = rustc + .cached_output( + process.clone().arg("-Csplit-debuginfo=packed"), + extra_fingerprint, + ) + .is_ok(); + + process.arg("--print=sysroot"); + process.arg("--print=cfg"); + + let (output, error) = rustc + .cached_output(&process, extra_fingerprint) + .with_context(|| { + "failed to run `rustc` to learn about target-specific information" + })?; + + let mut lines = output.lines(); + let mut map = HashMap::new(); + for crate_type in KNOWN_CRATE_TYPES { + let out = parse_crate_type(crate_type, &process, &output, &error, &mut lines)?; + map.insert(crate_type.clone(), out); + } - let line = match lines.next() { - Some(line) => line, - None => anyhow::bail!( - "output of --print=sysroot missing when learning about \ + let line = match lines.next() { + Some(line) => line, + None => anyhow::bail!( + "output of --print=sysroot missing when learning about \ target-specific information from rustc\n{}", - output_err_info(&process, &output, &error) - ), - }; - let sysroot = PathBuf::from(line); - let sysroot_host_libdir = if cfg!(windows) { - sysroot.join("bin") - } else { - sysroot.join("lib") - }; - let mut sysroot_target_libdir = sysroot.clone(); - sysroot_target_libdir.push("lib"); - sysroot_target_libdir.push("rustlib"); - sysroot_target_libdir.push(match &kind { - CompileKind::Host => rustc.host.as_str(), - CompileKind::Target(target) => target.short_name(), - }); - sysroot_target_libdir.push("lib"); - - let cfg = lines - .map(|line| Ok(Cfg::from_str(line)?)) - .filter(TargetInfo::not_user_specific_cfg) - .collect::>>() - .with_context(|| { - format!( - "failed to parse the cfg from `rustc --print=cfg`, got:\n{}", - output - ) - })?; - - Ok(TargetInfo { - crate_type_process, - crate_types: RefCell::new(map), - sysroot, - sysroot_host_libdir, - sysroot_target_libdir, + output_err_info(&process, &output, &error) + ), + }; + let sysroot = PathBuf::from(line); + let sysroot_host_libdir = if cfg!(windows) { + sysroot.join("bin") + } else { + sysroot.join("lib") + }; + let mut sysroot_target_libdir = sysroot.clone(); + sysroot_target_libdir.push("lib"); + sysroot_target_libdir.push("rustlib"); + sysroot_target_libdir.push(match &kind { + CompileKind::Host => rustc.host.as_str(), + CompileKind::Target(target) => target.short_name(), + }); + sysroot_target_libdir.push("lib"); + + let cfg = lines + .map(|line| Ok(Cfg::from_str(line)?)) + .filter(TargetInfo::not_user_specific_cfg) + .collect::>>() + .with_context(|| { + format!( + "failed to parse the cfg from `rustc --print=cfg`, got:\n{}", + output + ) + })?; + // recalculate `rustflags` from above now that we have `cfg` // information - rustflags: env_args( + let new_flags = env_args( config, requested_kinds, &rustc.host, Some(&cfg), kind, Flags::Rust, - )?, - rustdocflags: env_args( - config, - requested_kinds, - &rustc.host, - Some(&cfg), - kind, - Flags::Rustdoc, - )?, - cfg, - supports_split_debuginfo, - }) + )?; + + // Tricky: `RUSTFLAGS` defines the set of active `cfg` flags, active + // `cfg` flags define which `.cargo/config` sections apply, and they + // in turn can affect `RUSTFLAGS`! This is a bona fide mutual + // dependency, and it can even diverge (see `cfg_paradox` test). + // + // So what we do here is running at most *two* iterations of + // fixed-point iteration, which should be enough to cover + // practically useful cases, and warn if that's not enough for + // convergence. + let reached_fixed_point = new_flags == rustflags; + if !reached_fixed_point && turn == 0 { + turn += 1; + rustflags = new_flags; + continue; + } + if !reached_fixed_point { + config.shell().warn("non-trivial mutual dependency between target-specific configuration and RUSTFLAGS")?; + } + + return Ok(TargetInfo { + crate_type_process, + crate_types: RefCell::new(map), + sysroot, + sysroot_host_libdir, + sysroot_target_libdir, + rustflags, + rustdocflags: env_args( + config, + requested_kinds, + &rustc.host, + Some(&cfg), + kind, + Flags::Rustdoc, + )?, + cfg, + supports_split_debuginfo, + }); + } } fn not_user_specific_cfg(cfg: &CargoResult) -> bool { diff --git a/tests/testsuite/build_script_env.rs b/tests/testsuite/build_script_env.rs index febae9c0cde..119e4210a2d 100644 --- a/tests/testsuite/build_script_env.rs +++ b/tests/testsuite/build_script_env.rs @@ -208,3 +208,34 @@ fn build_script_sees_cfg_target_feature() { .run(); } } + +/// In this test, the cfg is self-contradictory. There's no *right* answer as to +/// what the value of `RUSTFLAGS` should be in this case. We chose to give a +/// warning. However, no matter what we do, it's important that build scripts +/// and rustc see a consistent picture +#[cargo_test] +fn cfg_paradox() { + let build_rs = r#" + fn main() { + let cfg = std::env::var("CARGO_CFG_BERTRAND").is_ok(); + eprintln!("cfg!(bertrand)={cfg}"); + } + "#; + + let config = r#" + [target.'cfg(not(bertrand))'] + rustflags = ["--cfg=bertrand"] + "#; + + let p = project() + .file(".cargo/config.toml", config) + .file("src/lib.rs", r#""#) + .file("build.rs", build_rs) + .build(); + + p.cargo("build -vv") + .with_stderr_contains("[WARNING] non-trivial mutual dependency between target-specific configuration and RUSTFLAGS") + .with_stderr_contains("[foo 0.0.1] cfg!(bertrand)=true") + .with_stderr_contains("[..]--cfg=bertrand[..]") + .run(); +}