Skip to content

Commit

Permalink
fix(env): dont track envs set by cargo in dep-info file
Browse files Browse the repository at this point in the history
This patch is quite hacky since the "set-by-Cargo" env vars are
hardcoded. However, not all environment variables set by Cargo are
statically known. To prevent the actual environment variables applied to
rustc invocation get out of sync from what we hard-code, a debug time
assertion is put to help discover missing environment variables earlier.
  • Loading branch information
weihanglo committed Dec 11, 2024
1 parent 08b7746 commit 3129515
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 3 deletions.
2 changes: 1 addition & 1 deletion crates/cargo-util/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub fn join_paths<T: AsRef<OsStr>>(paths: &[T], env: &str) -> Result<OsString> {

/// Returns the name of the environment variable used for searching for
/// dynamic libraries.
pub fn dylib_path_envvar() -> &'static str {
pub const fn dylib_path_envvar() -> &'static str {
if cfg!(windows) {
"PATH"
} else if cfg!(target_os = "macos") {
Expand Down
95 changes: 95 additions & 0 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,3 +527,98 @@ fn target_linker(bcx: &BuildContext<'_, '_>, kind: CompileKind) -> CargoResult<O
}
Ok(matching_linker.map(|(_k, linker)| linker.val.clone().resolve_program(bcx.gctx)))
}

/// This tracks environment variables Cargo sets to rustc when building a crate.
///
/// This only inclues variables with statically known keys.
/// For environment variable keys that vary like `CARG_BIN_EXE_<name>` or
/// `-Zbindeps` related env vars, we compare only their prefixes to determine
/// if they are internal.
/// See [`is_env_set_by_cargo`] and
/// <https://doc.rust-lang.org/nightly/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates>.
const ENV_VARS_SET_FOR_CRATES: [&str; 23] = [
crate::CARGO_ENV,
"CARGO_MANIFEST_DIR",
"CARGO_MANIFEST_PATH",
"CARGO_PKG_VERSION",
"CARGO_PKG_VERSION_MAJOR",
"CARGO_PKG_VERSION_MINOR",
"CARGO_PKG_VERSION_PATCH",
"CARGO_PKG_VERSION_PRE",
"CARGO_PKG_AUTHORS",
"CARGO_PKG_NAME",
"CARGO_PKG_DESCRIPTION",
"CARGO_PKG_HOMEPAGE",
"CARGO_PKG_REPOSITORY",
"CARGO_PKG_LICENSE",
"CARGO_PKG_LICENSE_FILE",
"CARGO_PKG_RUST_VERSION",
"CARGO_PKG_README",
"CARGO_CRATE_NAME",
"CARGO_BIN_NAME",
"OUT_DIR",
"CARGO_PRIMARY_PACKAGE",
"CARGO_TARGET_TMPDIR",
paths::dylib_path_envvar(),
];
/// Asserts if the given env vars are controlled by Cargo.
///
/// This is only for reminding Cargo developer to keep newly added environment
/// variables in sync with [`ENV_VARS_SET_FOR_CRATES`].
#[cfg(debug_assertions)]
pub(crate) fn assert_only_envs_set_by_cargo<'a>(
keys: impl Iterator<Item = impl AsRef<str>>,
env_config: &HashMap<String, OsString>,
) {
for key in keys {
let key = key.as_ref();
// When running Cargo's test suite,
// we're fine if it is from the `[env]` table
if env_config.contains_key(key) {
continue;
}
assert!(
is_env_set_by_cargo(key),
"`{key}` is not tracked as an environment variable set by Cargo\n\
Add it to `ENV_VARS_SET_FOR_CRATES` if you intend to introduce a new one"
);
}
}

/// True if the given env var is controlled or set by Cargo.
/// See [`ENV_VARS_SET_FOR_CRATES`].
pub(crate) fn is_env_set_by_cargo(key: &str) -> bool {
ENV_VARS_SET_FOR_CRATES.contains(&key)
|| key.starts_with("CARGO_BIN_EXE_")
|| key.starts_with("__CARGO") // internal/test-only envs
|| key == "RUSTC_BOOTSTRAP" // for -Zbuild-std
|| is_artifact_dep_env_vars(key)
}

/// Whether an env var is set because of `-Zbindeps`.
fn is_artifact_dep_env_vars(key: &str) -> bool {
let Some(key) = key.strip_prefix("CARGO_") else {
return false;
};
let Some(key) = key
.strip_prefix("BIN_")
.or_else(|| key.strip_prefix("CDYLIB_"))
.or_else(|| key.strip_prefix("STATICLIB_"))
else {
return false;
};
key.starts_with("FILE_") || key.starts_with("DIR_")
}

#[cfg(test)]
mod tests {
use std::collections::HashSet;

use super::ENV_VARS_SET_FOR_CRATES;

#[test]
fn ensure_env_vars_set_for_crates_unique() {
let set: HashSet<&str> = HashSet::from_iter(ENV_VARS_SET_FOR_CRATES);
assert_eq!(ENV_VARS_SET_FOR_CRATES.len(), set.len());
}
}
9 changes: 8 additions & 1 deletion src/cargo/core/compiler/fingerprint/dep_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use cargo_util::paths;
use cargo_util::ProcessBuilder;
use cargo_util::Sha256;

use crate::core::compiler::is_env_set_by_cargo;
use crate::CargoResult;
use crate::CARGO_ENV;

Expand Down Expand Up @@ -334,7 +335,13 @@ pub fn translate_dep_info(
//
// For cargo#13280, We trace env vars that are defined in the `[env]` config table.
on_disk_info.env.retain(|(key, _)| {
env_config.contains_key(key) || !rustc_cmd.get_envs().contains_key(key) || key == CARGO_ENV
if env_config.contains_key(key) && !is_env_set_by_cargo(key) {
return true;
}
if !rustc_cmd.get_envs().contains_key(key) {
return true;
}
key == CARGO_ENV
});

let serialize_path = |file| {
Expand Down
5 changes: 5 additions & 0 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ pub use self::build_context::{
};
use self::build_plan::BuildPlan;
pub use self::build_runner::{BuildRunner, Metadata, UnitHash};
pub(crate) use self::compilation::is_env_set_by_cargo;
pub use self::compilation::{Compilation, Doctest, UnitOutput};
pub use self::compile_kind::{CompileKind, CompileTarget};
pub use self::crate_type::CrateType;
Expand Down Expand Up @@ -331,6 +332,10 @@ fn rustc(
output_options.show_diagnostics = false;
}
let env_config = Arc::clone(build_runner.bcx.gctx.env_config()?);

#[cfg(debug_assertions)]
compilation::assert_only_envs_set_by_cargo(rustc.get_envs().keys(), &env_config);

return Ok(Work::new(move |state| {
// Artifacts are in a different location than typical units,
// hence we must assure the crate- and target-dependent
Expand Down
1 change: 0 additions & 1 deletion tests/testsuite/cargo_env_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,6 @@ foo
"#]])
.with_stderr_data(str![[r#"
[COMPILING] foo v0.5.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[RUNNING] `target/debug/foo[EXE]`
Expand Down

0 comments on commit 3129515

Please sign in to comment.