diff --git a/src/cargo/util/config/environment.rs b/src/cargo/util/config/environment.rs index a5617f315f9..bb049546a61 100644 --- a/src/cargo/util/config/environment.rs +++ b/src/cargo/util/config/environment.rs @@ -17,25 +17,49 @@ fn make_case_insensitive_and_normalized_env( .collect(); let normalized_env = env .iter() - // Only keep entries where both the key and value are valid UTF-8 + // Only keep entries where both the key and value are valid UTF-8, + // since the config env vars only support UTF-8 keys and values. + // Otherwise, the normalized map warning could incorrectly warn about entries that can't be + // read by the config system. + // Please see the docs for `Env` for more context. .filter_map(|(k, v)| Some((k.to_str()?, v.to_str()?))) .map(|(k, _)| (k.to_uppercase().replace("-", "_"), k.to_owned())) .collect(); (case_insensitive_env, normalized_env) } +/// A snapshot of the environment variables available to [`super::Config`]. +/// +/// Currently, the [`Config`](super::Config) supports lookup of environment variables +/// through two different means: +/// +/// - [`Config::get_env`](super::Config::get_env) +/// and [`Config::get_env_os`](super::Config::get_env_os) +/// for process environment variables (similar to [`std::env::var`] and [`std::env::var_os`]), +/// - Typed Config Value API via [`Config::get`](super::Config::get). +/// This is only available for `CARGO_` prefixed environment keys. +/// +/// This type contains the env var snapshot and helper methods for both APIs. #[derive(Debug)] pub struct Env { /// A snapshot of the process's environment variables. env: HashMap, - /// A map from normalized (upper case and with "-" replaced by "_") env keys to the actual key - /// in the environment. - /// The "normalized" key is the format expected by Cargo. - /// This is used to warn users when env keys are not provided in this format. + /// Used in the typed Config value API for warning messages when config keys are + /// given in the wrong format. + /// + /// Maps from "normalized" (upper case and with "-" replaced by "_") env keys + /// to the actual keys in the environment. + /// The normalized format is the one expected by Cargo. + /// + /// This only holds env keys that are valid UTF-8, since [`super::ConfigKey`] only supports UTF-8 keys. + /// In addition, this only holds env keys whose value in the environment is also valid UTF-8, + /// since the typed Config value API only supports UTF-8 values. normalized_env: HashMap, - /// A map from uppercased env keys to the actual key in the environment. - /// This is relevant on Windows, where env keys are case-insensitive. + /// Used to implement `get_env` and `get_env_os` on Windows, where env keys are case-insensitive. + /// + /// Maps from uppercased env keys to the actual key in the environment. /// For example, this might hold a pair `("PATH", "Path")`. + /// Currently only supports UTF-8 keys and values. case_insensitive_env: HashMap, } @@ -125,6 +149,18 @@ impl Env { /// /// This is intended for use in private methods of `Config`, /// and does not check for env key case mismatch. + /// + /// This is case-sensitive on Windows (even though environment keys on Windows are usually + /// case-insensitive) due to an unintended regression in 1.28 (via #5552). + /// This should only affect keys used for cargo's config-system env variables (`CARGO_` + /// prefixed ones), which are currently all uppercase. + /// We may want to consider rectifying it if users report issues. + /// One thing that adds a wrinkle here is the unstable advanced-env option that *requires* + /// case-sensitive keys. + /// + /// Do not use this for any other purposes. + /// Use [`Env::get_env_os`] or [`Env::get_env`] instead, which properly handle case + /// insensitivity on Windows. pub(super) fn get_str(&self, key: impl AsRef) -> Option<&str> { self.env.get(key.as_ref()).and_then(|s| s.to_str()) } @@ -133,6 +169,7 @@ impl Env { /// /// This is intended for use in private methods of `Config`, /// and does not check for env key case mismatch. + /// See the docstring of [`Env::get_str`] for more context. pub(super) fn contains_key(&self, key: impl AsRef) -> bool { self.env.contains_key(key.as_ref()) } @@ -140,7 +177,7 @@ impl Env { /// Looks up a normalized `key` in the `normalized_env`. /// Returns the corresponding (non-normalized) env key if it exists, else `None`. /// - /// This is used by `Config::check_environment_key_mismatch`. + /// This is used by [`super::Config::check_environment_key_case_mismatch`]. pub(super) fn get_normalized(&self, key: &str) -> Option<&str> { self.normalized_env.get(key).map(|s| s.as_ref()) }