Skip to content

Commit

Permalink
Auto merge of #9169 - Liberatys:emit-warning-on-env-variable-case-mis…
Browse files Browse the repository at this point in the history
…match, r=ehuss

Emit warning on env variable case mismatch

When running a command like `cargo --target TRIPPLE` cargo expects to find the environment variable CARGO_TARGET_[TRIPPLE]_* with uppercase and underscores. This check emits a warning if the checked environment variable has a mismatching case and/or contains dashes rather than underscores. The warning contains the given env variable as well as an explanation for the cause of the warning.

The check is skipped on windows as environment variables are treated as case insensitive on the platform.

Fixes #8285
  • Loading branch information
bors committed Feb 14, 2021
2 parents 8fa0827 + 0564466 commit af564b2
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 2 deletions.
40 changes: 38 additions & 2 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ pub struct Config {
target_dir: Option<Filesystem>,
/// Environment variables, separated to assist testing.
env: HashMap<String, String>,
/// Environment variables, converted to uppercase to check for case mismatch
upper_case_env: HashMap<String, String>,
/// Tracks which sources have been updated to avoid multiple updates.
updated_sources: LazyCell<RefCell<HashSet<SourceId>>>,
/// Lock, if held, of the global package cache along with the number of
Expand Down Expand Up @@ -211,6 +213,15 @@ impl Config {
})
.collect();

let upper_case_env = if cfg!(windows) {
HashMap::new()
} else {
env.clone()
.into_iter()
.map(|(k, _)| (k.to_uppercase().replace("-", "_"), k))
.collect()
};

let cache_rustc_info = match env.get("CARGO_CACHE_RUSTC_INFO") {
Some(cache) => cache != "0",
_ => true,
Expand Down Expand Up @@ -244,6 +255,7 @@ impl Config {
creation_time: Instant::now(),
target_dir: None,
env,
upper_case_env,
updated_sources: LazyCell::new(),
package_cache_lock: RefCell::new(None),
http_config: LazyCell::new(),
Expand Down Expand Up @@ -525,7 +537,10 @@ impl Config {
definition,
}))
}
None => Ok(None),
None => {
self.check_environment_key_case_mismatch(key);
Ok(None)
}
}
}

Expand All @@ -545,9 +560,27 @@ impl Config {
return true;
}
}
self.check_environment_key_case_mismatch(key);

false
}

fn check_environment_key_case_mismatch(&self, key: &ConfigKey) {
if cfg!(windows) {
// In the case of windows the check for case mismatch in keys can be skipped
// as windows already converts its environment keys into the desired format.
return;
}

if let Some(env_key) = self.upper_case_env.get(key.as_env_key()) {
let _ = self.shell().warn(format!(
"Environment variables are expected to use uppercase letters and underscores, \
the variable `{}` will be ignored and have no effect",
env_key
));
}
}

/// Get a string config value.
///
/// See `get` for more details.
Expand Down Expand Up @@ -640,7 +673,10 @@ impl Config {
) -> CargoResult<()> {
let env_val = match self.env.get(key.as_env_key()) {
Some(v) => v,
None => return Ok(()),
None => {
self.check_environment_key_case_mismatch(key);
return Ok(());
}
};

let def = Definition::Environment(key.as_env_key().to_string());
Expand Down
25 changes: 25 additions & 0 deletions tests/testsuite/tool_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,31 @@ fn custom_linker_env() {
.run();
}

#[cargo_test]
fn target_in_environment_contains_lower_case() {
let p = project().file("src/main.rs", "fn main() {}").build();

let target_keys = [
"CARGO_TARGET_X86_64_UNKNOWN_LINUX_musl_LINKER",
"CARGO_TARGET_x86_64_unknown_linux_musl_LINKER",
];

for target_key in &target_keys {
let mut execs = p.cargo("build -v --target x86_64-unknown-linux-musl");
execs.env(target_key, "nonexistent-linker").with_status(101);
if cfg!(windows) {
execs.with_stderr_does_not_contain("warning:[..]");
} else {
execs.with_stderr_contains(format!(
"warning: Environment variables are expected to use uppercase letters and underscores, \
the variable `{}` will be ignored and have no effect",
target_key
));
}
execs.run();
}
}

#[cargo_test]
fn cfg_ignored_fields() {
// Test for some ignored fields in [target.'cfg()'] tables.
Expand Down

0 comments on commit af564b2

Please sign in to comment.