Skip to content

Commit

Permalink
fix: envs in config can trigger rebuild by custom build script with `…
Browse files Browse the repository at this point in the history
…rerun-if-env-changed`.
  • Loading branch information
linyihai committed Nov 5, 2024
1 parent 475f6dc commit eeaf9bb
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 15 deletions.
61 changes: 46 additions & 15 deletions src/cargo/core/compiler/fingerprint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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<K: AsRef<str>>(key: K) -> LocalFingerprint {
fn from_env<K: AsRef<str>>(
key: K,
env_config: &Arc<HashMap<String, OsString>>,
env_config_insensitive: &Arc<OnceCell<HashMap<String, OsString>>>,
) -> 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 }
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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<String>>| {
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
Expand All @@ -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<String>>| {
if deps.rerun_if_changed.is_empty() && deps.rerun_if_env_changed.is_empty() {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -1765,6 +1795,7 @@ fn local_fingerprints_deps(
deps: &BuildDeps,
target_root: &Path,
pkg_root: &Path,
env_config: &Arc<HashMap<String, OsString>>,
) -> Vec<LocalFingerprint> {
debug!("new local fingerprints deps {:?}", pkg_root);
let mut local = Vec::new();
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions tests/testsuite/build_script_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
"#]])
Expand Down Expand Up @@ -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
"#]])
Expand Down Expand Up @@ -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
"#]])
Expand Down

0 comments on commit eeaf9bb

Please sign in to comment.