fix(env): dont track envs set by cargo in dep-info file #14811
Conversation
rustbot
added
A-build-execution
| #[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" | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
@ehuss I'm curious about your thoughts on this
### What does this PR try to resolve? This was found when doing #14811. Other counterparts display environment variables, but rustdoc does not. This patch makes rustdoc follow suit. ### How should we test and review this PR? Run `cargo doc -vv`
| @@ -529,3 +529,98 @@ fn target_linker(bcx: &BuildContext<'_, '_>, kind: CompileKind) -> CargoResult<O | |||
| } | |||
| Ok(matching_linker.map(|(_k, linker)| linker.val.clone().resolve_program(bcx.gctx))) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
In order to better discover these variables, whether to put this part of the content in a separate file management is better?
There was a problem hiding this comment.
A simple search for ENV_VARS_SET_FOR_CRATES could lead the way, though no objection to this proposal, just lacking of a better module name (naming is hard 😆).
|
☔ The latest upstream changes (presumably 69e5959) made this pull request unmergeable. Please resolve the merge conflicts. |
This is a side effect when fixing 13280 via PR 14701.
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.
What does this PR try to resolve?
Don't track environment variables set by Cargo1 in Cargo's 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.
Fixes #14798
How should we test and review this PR?
A new test is added to show no rebuild triggered.
Try adding a random new env here and see if a debug build fails.