Skip to content

Commit d853d5d

Browse files
committed
fix(env): dont track envs set by cargo in dep-info file
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.
1 parent 99a8417 commit d853d5d

File tree

4 files changed

+106
-5
lines changed

4 files changed

+106
-5
lines changed

src/cargo/core/compiler/compilation.rs

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
//! Type definitions for the result of a compilation.
22
3-
use std::collections::{BTreeSet, HashMap};
3+
use std::collections::BTreeSet;
4+
use std::collections::HashMap;
5+
use std::collections::HashSet;
46
use std::ffi::{OsStr, OsString};
57
use std::path::PathBuf;
8+
use std::sync::LazyLock;
69

710
use cargo_platform::CfgExpr;
811
use cargo_util::{paths, ProcessBuilder};
@@ -529,3 +532,88 @@ fn target_linker(bcx: &BuildContext<'_, '_>, kind: CompileKind) -> CargoResult<O
529532
}
530533
Ok(matching_linker.map(|(_k, linker)| linker.val.clone().resolve_program(bcx.gctx)))
531534
}
535+
536+
/// This tracks environment variables Cargo sets to rustc when building a crate.
537+
///
538+
/// This only inclues variables with statically known keys.
539+
/// For environment variable keys that vary like `CARG_BIN_EXE_<name>` or
540+
/// `-Zbindeps` related env vars, we compare only their prefixes to determine
541+
/// if they are internal.
542+
/// See [`is_env_set_by_cargo`] and
543+
/// <https://doc.rust-lang.org/nightly/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates>.
544+
static ENV_VARS_SET_FOR_CRATES: LazyLock<HashSet<&'static str>> = LazyLock::new(|| {
545+
HashSet::from_iter([
546+
crate::CARGO_ENV,
547+
"CARGO_MANIFEST_DIR",
548+
"CARGO_MANIFEST_PATH",
549+
"CARGO_PKG_VERSION",
550+
"CARGO_PKG_VERSION_MAJOR",
551+
"CARGO_PKG_VERSION_MINOR",
552+
"CARGO_PKG_VERSION_PATCH",
553+
"CARGO_PKG_VERSION_PRE",
554+
"CARGO_PKG_AUTHORS",
555+
"CARGO_PKG_NAME",
556+
"CARGO_PKG_DESCRIPTION",
557+
"CARGO_PKG_HOMEPAGE",
558+
"CARGO_PKG_REPOSITORY",
559+
"CARGO_PKG_LICENSE",
560+
"CARGO_PKG_LICENSE_FILE",
561+
"CARGO_PKG_RUST_VERSION",
562+
"CARGO_PKG_README",
563+
"CARGO_CRATE_NAME",
564+
"CARGO_BIN_NAME",
565+
"OUT_DIR",
566+
"CARGO_PRIMARY_PACKAGE",
567+
"CARGO_TARGET_TMPDIR",
568+
paths::dylib_path_envvar(),
569+
])
570+
});
571+
572+
/// Asserts if the given env vars are controlled by Cargo.
573+
///
574+
/// This is only for reminding Cargo developer to keep newly added environment
575+
/// variables in sync with [`ENV_VARS_SET_FOR_CRATES`].
576+
#[cfg(debug_assertions)]
577+
pub(crate) fn assert_only_envs_set_by_cargo<'a>(
578+
keys: impl Iterator<Item = impl AsRef<str>>,
579+
env_config: &HashMap<String, OsString>,
580+
) {
581+
for key in keys {
582+
let key = key.as_ref();
583+
// When running Cargo's test suite,
584+
// we're fine if it is from the `[env]` table
585+
if env_config.contains_key(key) {
586+
continue;
587+
}
588+
assert!(
589+
is_env_set_by_cargo(key),
590+
"`{key}` is not tracked as an environment variable set by Cargo\n\
591+
Add it to `ENV_VARS_SET_FOR_CRATES` if you intend to introduce a new one"
592+
);
593+
}
594+
}
595+
596+
/// True if the given env var is controlled or set by Cargo.
597+
/// See [`ENV_VARS_SET_FOR_CRATES`].
598+
pub(crate) fn is_env_set_by_cargo(key: &str) -> bool {
599+
ENV_VARS_SET_FOR_CRATES.contains(key)
600+
|| key.starts_with("CARGO_BIN_EXE_")
601+
|| key.starts_with("__CARGO") // internal/test-only envs
602+
|| key == "RUSTC_BOOTSTRAP" // for -Zbuild-std
603+
|| is_artifact_dep_env_vars(key)
604+
}
605+
606+
/// Whether an env var is set because of `-Zbindeps`.
607+
fn is_artifact_dep_env_vars(key: &str) -> bool {
608+
let Some(key) = key.strip_prefix("CARGO_") else {
609+
return false;
610+
};
611+
let Some(key) = key
612+
.strip_prefix("BIN_")
613+
.or_else(|| key.strip_prefix("CDYLIB_"))
614+
.or_else(|| key.strip_prefix("STATICLIB_"))
615+
else {
616+
return false;
617+
};
618+
key.starts_with("FILE_") || key.starts_with("DIR_")
619+
}

src/cargo/core/compiler/fingerprint/dep_info.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use cargo_util::paths;
1919
use cargo_util::ProcessBuilder;
2020
use cargo_util::Sha256;
2121

22+
use crate::core::compiler::is_env_set_by_cargo;
2223
use crate::CargoResult;
2324
use crate::CARGO_ENV;
2425

@@ -334,7 +335,13 @@ pub fn translate_dep_info(
334335
//
335336
// For cargo#13280, We trace env vars that are defined in the `[env]` config table.
336337
on_disk_info.env.retain(|(key, _)| {
337-
env_config.contains_key(key) || !rustc_cmd.get_envs().contains_key(key) || key == CARGO_ENV
338+
if key == CARGO_ENV {
339+
return true;
340+
}
341+
if !rustc_cmd.get_envs().contains_key(key) {
342+
return true;
343+
}
344+
env_config.contains_key(key) && !is_env_set_by_cargo(key)
338345
});
339346

340347
let serialize_path = |file| {

src/cargo/core/compiler/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ pub use self::build_context::{
7474
};
7575
use self::build_plan::BuildPlan;
7676
pub use self::build_runner::{BuildRunner, Metadata};
77+
pub(crate) use self::compilation::is_env_set_by_cargo;
7778
pub use self::compilation::{Compilation, Doctest, UnitOutput};
7879
pub use self::compile_kind::{CompileKind, CompileTarget};
7980
pub use self::crate_type::CrateType;
@@ -334,6 +335,10 @@ fn rustc(
334335
output_options.show_diagnostics = false;
335336
}
336337
let env_config = Arc::clone(build_runner.bcx.gctx.env_config()?);
338+
339+
#[cfg(debug_assertions)]
340+
compilation::assert_only_envs_set_by_cargo(rustc.get_envs().keys(), &env_config);
341+
337342
return Ok(Work::new(move |state| {
338343
// Artifacts are in a different location than typical units,
339344
// hence we must assure the crate- and target-dependent

tests/testsuite/cargo_env_config.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,10 @@ fn override_env_set_by_cargo() {
460460
.build();
461461

462462
let args = [
463-
"--config", "env.CARGO_MANIFEST_DIR='Sauron'",
464-
"--config", "env.CARGO_PKG_NAME='Saruman'",
463+
"--config",
464+
"env.CARGO_MANIFEST_DIR='Sauron'",
465+
"--config",
466+
"env.CARGO_PKG_NAME='Saruman'",
465467
];
466468

467469
p.cargo("run")
@@ -488,7 +490,6 @@ foo
488490
489491
"#]])
490492
.with_stderr_data(str![[r#"
491-
[COMPILING] foo v0.5.0 ([ROOT]/foo)
492493
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
493494
[RUNNING] `target/debug/foo[EXE]`
494495

0 commit comments

Comments
 (0)