Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: env table config can't trigger rebuild with rerun-if-env-changed. #14756

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
148 changes: 148 additions & 0 deletions tests/testsuite/build_script_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,3 +383,151 @@ 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#"
[COMPILING] foo v0.0.1 ([ROOT]/foo)
[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#"
[COMPILING] foo v0.0.1 ([ROOT]/foo)
[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#"
[COMPILING] foo v0.0.1 ([ROOT]/foo)
[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();
}