From 475f6dcc8c36daf098ef8faf431f39dcfd6032ce Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Wed, 30 Oct 2024 11:15:47 +0800 Subject: [PATCH 1/2] test: add test for `rerun-if-env-changed` custom build script. --- tests/testsuite/build_script_env.rs | 145 ++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/tests/testsuite/build_script_env.rs b/tests/testsuite/build_script_env.rs index 2c6b984b070..d7babd5baa1 100644 --- a/tests/testsuite/build_script_env.rs +++ b/tests/testsuite/build_script_env.rs @@ -383,3 +383,148 @@ fn rustc_cfg_with_and_without_value() { ); check.run(); } + +#[cargo_test] +fn env_config_rerun_if_changed() { + let p = project() + .file("src/main.rs", "fn main() {}") + .file( + "build.rs", + r#" + fn main() { + println!("cargo::rerun-if-env-changed=FOO"); + } + "#, + ) + .file( + ".cargo/config.toml", + r#" + [env] + FOO = "foo" + "#, + ) + .build(); + + p.cargo("check") + .with_stderr_data(str![[r#" +[COMPILING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); + + p.change_file( + ".cargo/config.toml", + r#" + [env] + FOO = "bar" + "#, + ); + p.cargo("check") + .with_stderr_data(str![[r#" +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); + // This identical cargo invocation is to ensure no rebuild happen. + p.cargo("check") + .with_stderr_data(str![[r#" +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} + +#[cfg(windows)] +#[cargo_test] +fn insensitive_env_config_rerun_if_changed() { + let p = project() + .file("src/main.rs", "fn main() {}") + .file( + "build.rs", + r#" + fn main() { + println!("cargo::rerun-if-env-changed=FOO"); + } + "#, + ) + .file( + ".cargo/config.toml", + r#" + [env] + Foo = "foo" + "#, + ) + .build(); + + p.cargo("check") + .with_stderr_data(str![[r#" +[COMPILING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); + p.change_file( + ".cargo/config.toml", + r#" + [env] + Foo = "bar" + "#, + ); + p.cargo("check") + .with_stderr_data(str![[r#" +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); + // This identical cargo invocation is to ensure no rebuild happen. + p.cargo("check") + .with_stderr_data(str![[r#" +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} + +#[cargo_test] +fn env_config_newly_added_rerun() { + let p = project() + .file("src/main.rs", "fn main() {}") + .file( + "build.rs", + r#" + fn main() { + println!("cargo::rerun-if-env-changed=FOO"); + } + "#, + ) + .build(); + + p.cargo("check") + .with_stderr_data(str![[r#" +[COMPILING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); + p.change_file( + ".cargo/config.toml", + r#" + [env] + FOO = "foo" + "#, + ); + p.cargo("check") + .with_stderr_data(str![[r#" +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); + // This identical cargo invocation is to ensure no rebuild happen. + p.cargo("check") + .with_stderr_data(str![[r#" +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + +"#]]) + .run(); +} From eeaf9bb981c7d8307736d3306203ca09c0709e9b Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Wed, 30 Oct 2024 21:10:53 +0800 Subject: [PATCH 2/2] fix: envs in config can trigger rebuild by custom build script with `rerun-if-env-changed`. --- src/cargo/core/compiler/fingerprint/mod.rs | 61 ++++++++++++++++------ tests/testsuite/build_script_env.rs | 3 ++ 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs index 64a50edb69d..9c144f990fa 100644 --- a/src/cargo/core/compiler/fingerprint/mod.rs +++ b/src/cargo/core/compiler/fingerprint/mod.rs @@ -362,8 +362,10 @@ mod dep_info; mod dirty_reason; +use std::cell::OnceCell; use std::collections::hash_map::{Entry, HashMap}; use std::env; +use std::ffi::OsString; use std::fs; use std::fs::File; use std::hash::{self, Hash, Hasher}; @@ -499,7 +501,7 @@ pub fn prepare_target( // thunk we can invoke on a foreign thread to calculate this. let build_script_outputs = Arc::clone(&build_runner.build_script_outputs); let metadata = build_runner.get_run_build_script_metadata(unit); - let (gen_local, _overridden) = build_script_local_fingerprints(build_runner, unit); + let (gen_local, _overridden) = build_script_local_fingerprints(build_runner, unit)?; let output_path = build_runner.build_explicit_deps[unit] .build_script_output .clone(); @@ -796,14 +798,36 @@ pub enum StaleItem { impl LocalFingerprint { /// Read the environment variable of the given env `key`, and creates a new /// [`LocalFingerprint::RerunIfEnvChanged`] for it. - /// - // TODO: This is allowed at this moment. Should figure out if it makes - // sense if permitting to read env from the config system. #[allow(clippy::disallowed_methods)] - fn from_env>(key: K) -> LocalFingerprint { + fn from_env>( + key: K, + env_config: &Arc>, + env_config_insensitive: &Arc>>, + ) -> LocalFingerprint { let key = key.as_ref(); let var = key.to_owned(); - let val = env::var(key).ok(); + let val = if let Some(val) = match env_config.get(key) { + Some(value) => value.to_str().map(|s| s.to_owned()), + None => { + if cfg!(windows) { + env_config_insensitive + .get_or_init(|| { + env_config + .iter() + .map(|(k, v)| (k.to_uppercase().clone(), v.clone())) + .collect() + }) + .get(&key.to_uppercase()) + .and_then(|s| s.to_str().map(|s| s.to_owned())) + } else { + None + } + } + } { + Some(val) + } else { + env::var(key).ok() + }; LocalFingerprint::RerunIfEnvChanged { var, val } } @@ -1573,7 +1597,7 @@ fn calculate_run_custom_build( // the build script this means we'll be watching files and env vars. // Otherwise if we haven't previously executed it we'll just start watching // the whole crate. - let (gen_local, overridden) = build_script_local_fingerprints(build_runner, unit); + let (gen_local, overridden) = build_script_local_fingerprints(build_runner, unit)?; let deps = &build_runner.build_explicit_deps[unit]; let local = (gen_local)( deps, @@ -1667,7 +1691,7 @@ See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rerun-if-change fn build_script_local_fingerprints( build_runner: &mut BuildRunner<'_, '_>, unit: &Unit, -) -> ( +) -> CargoResult<( Box< dyn FnOnce( &BuildDeps, @@ -1676,20 +1700,20 @@ fn build_script_local_fingerprints( + Send, >, bool, -) { +)> { assert!(unit.mode.is_run_custom_build()); // First up, if this build script is entirely overridden, then we just // return the hash of what we overrode it with. This is the easy case! if let Some(fingerprint) = build_script_override_fingerprint(build_runner, unit) { debug!("override local fingerprints deps {}", unit.pkg); - return ( + return Ok(( Box::new( move |_: &BuildDeps, _: Option<&dyn Fn() -> CargoResult>| { Ok(Some(vec![fingerprint])) }, ), true, // this is an overridden build script - ); + )); } // ... Otherwise this is a "real" build script and we need to return a real @@ -1701,6 +1725,7 @@ fn build_script_local_fingerprints( // obvious. let pkg_root = unit.pkg.root().to_path_buf(); let target_dir = target_root(build_runner); + let env_config = Arc::clone(build_runner.bcx.gctx.env_config()?); let calculate = move |deps: &BuildDeps, pkg_fingerprint: Option<&dyn Fn() -> CargoResult>| { if deps.rerun_if_changed.is_empty() && deps.rerun_if_env_changed.is_empty() { @@ -1730,11 +1755,16 @@ fn build_script_local_fingerprints( // Ok so now we're in "new mode" where we can have files listed as // dependencies as well as env vars listed as dependencies. Process // them all here. - Ok(Some(local_fingerprints_deps(deps, &target_dir, &pkg_root))) + Ok(Some(local_fingerprints_deps( + deps, + &target_dir, + &pkg_root, + &env_config, + ))) }; // Note that `false` == "not overridden" - (Box::new(calculate), false) + Ok((Box::new(calculate), false)) } /// Create a [`LocalFingerprint`] for an overridden build script. @@ -1765,6 +1795,7 @@ fn local_fingerprints_deps( deps: &BuildDeps, target_root: &Path, pkg_root: &Path, + env_config: &Arc>, ) -> Vec { debug!("new local fingerprints deps {:?}", pkg_root); let mut local = Vec::new(); @@ -1785,11 +1816,11 @@ fn local_fingerprints_deps( .collect(); local.push(LocalFingerprint::RerunIfChanged { output, paths }); } - + let env_config_insensitive = Arc::new(OnceCell::new()); local.extend( deps.rerun_if_env_changed .iter() - .map(LocalFingerprint::from_env), + .map(|s| LocalFingerprint::from_env(s, env_config, &env_config_insensitive)), ); local diff --git a/tests/testsuite/build_script_env.rs b/tests/testsuite/build_script_env.rs index d7babd5baa1..d8fee64abcc 100644 --- a/tests/testsuite/build_script_env.rs +++ b/tests/testsuite/build_script_env.rs @@ -422,6 +422,7 @@ fn env_config_rerun_if_changed() { ); p.cargo("check") .with_stderr_data(str![[r#" +[COMPILING] foo v0.0.1 ([ROOT]/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) @@ -473,6 +474,7 @@ fn insensitive_env_config_rerun_if_changed() { ); p.cargo("check") .with_stderr_data(str![[r#" +[COMPILING] foo v0.0.1 ([ROOT]/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) @@ -516,6 +518,7 @@ fn env_config_newly_added_rerun() { ); p.cargo("check") .with_stderr_data(str![[r#" +[COMPILING] foo v0.0.1 ([ROOT]/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]])