Skip to content

Commit 7b18c85

Browse files
committed
Auto merge of #11863 - weihanglo:rust-1.69.0, r=ehuss
[beta-1.69] backport #11824 Beta backports: - #11824 In order to make CI pass, the following PRs are also cherry-picked: - —
2 parents 4a4d795 + a02ca0d commit 7b18c85

File tree

5 files changed

+257
-48
lines changed

5 files changed

+257
-48
lines changed

src/cargo/util/config/de.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ impl<'config> ValueDeserializer<'config> {
424424
let definition = {
425425
let env = de.key.as_env_key();
426426
let env_def = Definition::Environment(env.to_string());
427-
match (de.config.env_has_key(env), de.config.get_cv(&de.key)?) {
427+
match (de.config.env.contains_key(env), de.config.get_cv(&de.key)?) {
428428
(true, Some(cv)) => {
429429
// Both, pick highest priority.
430430
if env_def.is_higher_priority(cv.definition()) {

src/cargo/util/config/environment.rs

+184
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
//! Encapsulates snapshotting of environment variables.
2+
3+
use std::collections::HashMap;
4+
use std::ffi::{OsStr, OsString};
5+
6+
use crate::util::errors::CargoResult;
7+
use anyhow::{anyhow, bail};
8+
9+
/// Generate `case_insensitive_env` and `normalized_env` from the `env`.
10+
fn make_case_insensitive_and_normalized_env(
11+
env: &HashMap<OsString, OsString>,
12+
) -> (HashMap<String, String>, HashMap<String, String>) {
13+
let case_insensitive_env: HashMap<_, _> = env
14+
.keys()
15+
.filter_map(|k| k.to_str())
16+
.map(|k| (k.to_uppercase(), k.to_owned()))
17+
.collect();
18+
let normalized_env = env
19+
.iter()
20+
// Only keep entries where both the key and value are valid UTF-8,
21+
// since the config env vars only support UTF-8 keys and values.
22+
// Otherwise, the normalized map warning could incorrectly warn about entries that can't be
23+
// read by the config system.
24+
// Please see the docs for `Env` for more context.
25+
.filter_map(|(k, v)| Some((k.to_str()?, v.to_str()?)))
26+
.map(|(k, _)| (k.to_uppercase().replace("-", "_"), k.to_owned()))
27+
.collect();
28+
(case_insensitive_env, normalized_env)
29+
}
30+
31+
/// A snapshot of the environment variables available to [`super::Config`].
32+
///
33+
/// Currently, the [`Config`](super::Config) supports lookup of environment variables
34+
/// through two different means:
35+
///
36+
/// - [`Config::get_env`](super::Config::get_env)
37+
/// and [`Config::get_env_os`](super::Config::get_env_os)
38+
/// for process environment variables (similar to [`std::env::var`] and [`std::env::var_os`]),
39+
/// - Typed Config Value API via [`Config::get`](super::Config::get).
40+
/// This is only available for `CARGO_` prefixed environment keys.
41+
///
42+
/// This type contains the env var snapshot and helper methods for both APIs.
43+
#[derive(Debug)]
44+
pub struct Env {
45+
/// A snapshot of the process's environment variables.
46+
env: HashMap<OsString, OsString>,
47+
/// Used in the typed Config value API for warning messages when config keys are
48+
/// given in the wrong format.
49+
///
50+
/// Maps from "normalized" (upper case and with "-" replaced by "_") env keys
51+
/// to the actual keys in the environment.
52+
/// The normalized format is the one expected by Cargo.
53+
///
54+
/// This only holds env keys that are valid UTF-8, since [`super::ConfigKey`] only supports UTF-8 keys.
55+
/// In addition, this only holds env keys whose value in the environment is also valid UTF-8,
56+
/// since the typed Config value API only supports UTF-8 values.
57+
normalized_env: HashMap<String, String>,
58+
/// Used to implement `get_env` and `get_env_os` on Windows, where env keys are case-insensitive.
59+
///
60+
/// Maps from uppercased env keys to the actual key in the environment.
61+
/// For example, this might hold a pair `("PATH", "Path")`.
62+
/// Currently only supports UTF-8 keys and values.
63+
case_insensitive_env: HashMap<String, String>,
64+
}
65+
66+
impl Env {
67+
/// Create a new `Env` from process's environment variables.
68+
pub fn new() -> Self {
69+
let env: HashMap<_, _> = std::env::vars_os().collect();
70+
let (case_insensitive_env, normalized_env) = make_case_insensitive_and_normalized_env(&env);
71+
Self {
72+
env,
73+
case_insensitive_env,
74+
normalized_env,
75+
}
76+
}
77+
78+
/// Set the env directly from a `HashMap`.
79+
/// This should be used for debugging purposes only.
80+
pub(super) fn from_map(env: HashMap<String, String>) -> Self {
81+
let env = env.into_iter().map(|(k, v)| (k.into(), v.into())).collect();
82+
let (case_insensitive_env, normalized_env) = make_case_insensitive_and_normalized_env(&env);
83+
Self {
84+
env,
85+
case_insensitive_env,
86+
normalized_env,
87+
}
88+
}
89+
90+
/// Returns all environment variables as an iterator,
91+
/// keeping only entries where both the key and value are valid UTF-8.
92+
pub fn iter_str(&self) -> impl Iterator<Item = (&str, &str)> {
93+
self.env
94+
.iter()
95+
.filter_map(|(k, v)| Some((k.to_str()?, v.to_str()?)))
96+
}
97+
98+
/// Returns all environment variable keys, filtering out keys that are not valid UTF-8.
99+
pub fn keys_str(&self) -> impl Iterator<Item = &str> {
100+
self.env.keys().filter_map(|k| k.to_str())
101+
}
102+
103+
/// Get the value of environment variable `key` through the `Config` snapshot.
104+
///
105+
/// This can be used similarly to `std::env::var_os`.
106+
/// On Windows, we check for case mismatch since environment keys are case-insensitive.
107+
pub fn get_env_os(&self, key: impl AsRef<OsStr>) -> Option<OsString> {
108+
match self.env.get(key.as_ref()) {
109+
Some(s) => Some(s.clone()),
110+
None => {
111+
if cfg!(windows) {
112+
self.get_env_case_insensitive(key).cloned()
113+
} else {
114+
None
115+
}
116+
}
117+
}
118+
}
119+
120+
/// Get the value of environment variable `key` through the `self.env` snapshot.
121+
///
122+
/// This can be used similarly to `std::env::var`.
123+
/// On Windows, we check for case mismatch since environment keys are case-insensitive.
124+
pub fn get_env(&self, key: impl AsRef<OsStr>) -> CargoResult<String> {
125+
let key = key.as_ref();
126+
let s = self
127+
.get_env_os(key)
128+
.ok_or_else(|| anyhow!("{key:?} could not be found in the environment snapshot"))?;
129+
130+
match s.to_str() {
131+
Some(s) => Ok(s.to_owned()),
132+
None => bail!("environment variable value is not valid unicode: {s:?}"),
133+
}
134+
}
135+
136+
/// Performs a case-insensitive lookup of `key` in the environment.
137+
///
138+
/// This is relevant on Windows, where environment variables are case-insensitive.
139+
/// Note that this only works on keys that are valid UTF-8 and it uses Unicode uppercase,
140+
/// which may differ from the OS's notion of uppercase.
141+
fn get_env_case_insensitive(&self, key: impl AsRef<OsStr>) -> Option<&OsString> {
142+
let upper_case_key = key.as_ref().to_str()?.to_uppercase();
143+
let env_key: &OsStr = self.case_insensitive_env.get(&upper_case_key)?.as_ref();
144+
self.env.get(env_key)
145+
}
146+
147+
/// Get the value of environment variable `key` as a `&str`.
148+
/// Returns `None` if `key` is not in `self.env` or if the value is not valid UTF-8.
149+
///
150+
/// This is intended for use in private methods of `Config`,
151+
/// and does not check for env key case mismatch.
152+
///
153+
/// This is case-sensitive on Windows (even though environment keys on Windows are usually
154+
/// case-insensitive) due to an unintended regression in 1.28 (via #5552).
155+
/// This should only affect keys used for cargo's config-system env variables (`CARGO_`
156+
/// prefixed ones), which are currently all uppercase.
157+
/// We may want to consider rectifying it if users report issues.
158+
/// One thing that adds a wrinkle here is the unstable advanced-env option that *requires*
159+
/// case-sensitive keys.
160+
///
161+
/// Do not use this for any other purposes.
162+
/// Use [`Env::get_env_os`] or [`Env::get_env`] instead, which properly handle case
163+
/// insensitivity on Windows.
164+
pub(super) fn get_str(&self, key: impl AsRef<OsStr>) -> Option<&str> {
165+
self.env.get(key.as_ref()).and_then(|s| s.to_str())
166+
}
167+
168+
/// Check if the environment contains `key`.
169+
///
170+
/// This is intended for use in private methods of `Config`,
171+
/// and does not check for env key case mismatch.
172+
/// See the docstring of [`Env::get_str`] for more context.
173+
pub(super) fn contains_key(&self, key: impl AsRef<OsStr>) -> bool {
174+
self.env.contains_key(key.as_ref())
175+
}
176+
177+
/// Looks up a normalized `key` in the `normalized_env`.
178+
/// Returns the corresponding (non-normalized) env key if it exists, else `None`.
179+
///
180+
/// This is used by [`super::Config::check_environment_key_case_mismatch`].
181+
pub(super) fn get_normalized(&self, key: &str) -> Option<&str> {
182+
self.normalized_env.get(key).map(|s| s.as_ref())
183+
}
184+
}

src/cargo/util/config/mod.rs

+21-47
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ pub use path::{ConfigRelativePath, PathAndArgs};
100100
mod target;
101101
pub use target::{TargetCfgConfig, TargetConfig};
102102

103+
mod environment;
104+
use environment::Env;
105+
103106
// Helper macro for creating typed access methods.
104107
macro_rules! get_value_typed {
105108
($name:ident, $ty:ty, $variant:ident, $expected:expr) => {
@@ -199,10 +202,8 @@ pub struct Config {
199202
creation_time: Instant,
200203
/// Target Directory via resolved Cli parameter
201204
target_dir: Option<Filesystem>,
202-
/// Environment variables, separated to assist testing.
203-
env: HashMap<OsString, OsString>,
204-
/// Environment variables, converted to uppercase to check for case mismatch
205-
upper_case_env: HashMap<String, String>,
205+
/// Environment variable snapshot.
206+
env: Env,
206207
/// Tracks which sources have been updated to avoid multiple updates.
207208
updated_sources: LazyCell<RefCell<HashSet<SourceId>>>,
208209
/// Cache of credentials from configuration or credential providers.
@@ -260,16 +261,10 @@ impl Config {
260261
}
261262
});
262263

263-
let env: HashMap<_, _> = env::vars_os().collect();
264-
265-
let upper_case_env = env
266-
.iter()
267-
.filter_map(|(k, _)| k.to_str()) // Only keep valid UTF-8
268-
.map(|k| (k.to_uppercase().replace("-", "_"), k.to_owned()))
269-
.collect();
264+
let env = Env::new();
270265

271-
let cache_key: &OsStr = "CARGO_CACHE_RUSTC_INFO".as_ref();
272-
let cache_rustc_info = match env.get(cache_key) {
266+
let cache_key = "CARGO_CACHE_RUSTC_INFO";
267+
let cache_rustc_info = match env.get_env_os(cache_key) {
273268
Some(cache) => cache != "0",
274269
_ => true,
275270
};
@@ -303,7 +298,6 @@ impl Config {
303298
creation_time: Instant::now(),
304299
target_dir: None,
305300
env,
306-
upper_case_env,
307301
updated_sources: LazyCell::new(),
308302
credential_cache: LazyCell::new(),
309303
package_cache_lock: RefCell::new(None),
@@ -658,7 +652,7 @@ impl Config {
658652
// Root table can't have env value.
659653
return Ok(cv);
660654
}
661-
let env = self.get_env_str(key.as_env_key());
655+
let env = self.env.get_str(key.as_env_key());
662656
let env_def = Definition::Environment(key.as_env_key().to_string());
663657
let use_env = match (&cv, env) {
664658
// Lists are always merged.
@@ -729,28 +723,26 @@ impl Config {
729723

730724
/// Helper primarily for testing.
731725
pub fn set_env(&mut self, env: HashMap<String, String>) {
732-
self.env = env.into_iter().map(|(k, v)| (k.into(), v.into())).collect();
726+
self.env = Env::from_map(env);
733727
}
734728

735-
/// Returns all environment variables as an iterator, filtering out entries
736-
/// that are not valid UTF-8.
729+
/// Returns all environment variables as an iterator,
730+
/// keeping only entries where both the key and value are valid UTF-8.
737731
pub(crate) fn env(&self) -> impl Iterator<Item = (&str, &str)> {
738-
self.env
739-
.iter()
740-
.filter_map(|(k, v)| Some((k.to_str()?, v.to_str()?)))
732+
self.env.iter_str()
741733
}
742734

743-
/// Returns all environment variable keys, filtering out entries that are not valid UTF-8.
735+
/// Returns all environment variable keys, filtering out keys that are not valid UTF-8.
744736
fn env_keys(&self) -> impl Iterator<Item = &str> {
745-
self.env.iter().filter_map(|(k, _)| k.to_str())
737+
self.env.keys_str()
746738
}
747739

748740
fn get_config_env<T>(&self, key: &ConfigKey) -> Result<OptValue<T>, ConfigError>
749741
where
750742
T: FromStr,
751743
<T as FromStr>::Err: fmt::Display,
752744
{
753-
match self.get_env_str(key.as_env_key()) {
745+
match self.env.get_str(key.as_env_key()) {
754746
Some(value) => {
755747
let definition = Definition::Environment(key.as_env_key().to_string());
756748
Ok(Some(Value {
@@ -771,39 +763,21 @@ impl Config {
771763
///
772764
/// This can be used similarly to `std::env::var`.
773765
pub fn get_env(&self, key: impl AsRef<OsStr>) -> CargoResult<String> {
774-
let key = key.as_ref();
775-
let s = match self.env.get(key) {
776-
Some(s) => s,
777-
None => bail!("{key:?} could not be found in the environment snapshot",),
778-
};
779-
match s.to_str() {
780-
Some(s) => Ok(s.to_owned()),
781-
None => bail!("environment variable value is not valid unicode: {s:?}"),
782-
}
766+
self.env.get_env(key)
783767
}
784768

785769
/// Get the value of environment variable `key` through the `Config` snapshot.
786770
///
787771
/// This can be used similarly to `std::env::var_os`.
788772
pub fn get_env_os(&self, key: impl AsRef<OsStr>) -> Option<OsString> {
789-
self.env.get(key.as_ref()).cloned()
790-
}
791-
792-
/// Get the value of environment variable `key`.
793-
/// Returns `None` if `key` is not in `self.env` or if the value is not valid UTF-8.
794-
fn get_env_str(&self, key: impl AsRef<OsStr>) -> Option<&str> {
795-
self.env.get(key.as_ref()).and_then(|s| s.to_str())
796-
}
797-
798-
fn env_has_key(&self, key: impl AsRef<OsStr>) -> bool {
799-
self.env.contains_key(key.as_ref())
773+
self.env.get_env_os(key)
800774
}
801775

802776
/// Check if the [`Config`] contains a given [`ConfigKey`].
803777
///
804778
/// See `ConfigMapAccess` for a description of `env_prefix_ok`.
805779
fn has_key(&self, key: &ConfigKey, env_prefix_ok: bool) -> CargoResult<bool> {
806-
if self.env_has_key(key.as_env_key()) {
780+
if self.env.contains_key(key.as_env_key()) {
807781
return Ok(true);
808782
}
809783
if env_prefix_ok {
@@ -821,7 +795,7 @@ impl Config {
821795
}
822796

823797
fn check_environment_key_case_mismatch(&self, key: &ConfigKey) {
824-
if let Some(env_key) = self.upper_case_env.get(key.as_env_key()) {
798+
if let Some(env_key) = self.env.get_normalized(key.as_env_key()) {
825799
let _ = self.shell().warn(format!(
826800
"Environment variables are expected to use uppercase letters and underscores, \
827801
the variable `{}` will be ignored and have no effect",
@@ -920,7 +894,7 @@ impl Config {
920894
key: &ConfigKey,
921895
output: &mut Vec<(String, Definition)>,
922896
) -> CargoResult<()> {
923-
let env_val = match self.get_env_str(key.as_env_key()) {
897+
let env_val = match self.env.get_str(key.as_env_key()) {
924898
Some(v) => v,
925899
None => {
926900
self.check_environment_key_case_mismatch(key);

tests/testsuite/cargo_command.rs

+26
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,32 @@ fn list_command_looks_at_path() {
104104
);
105105
}
106106

107+
#[cfg(windows)]
108+
#[cargo_test]
109+
fn list_command_looks_at_path_case_mismatch() {
110+
let proj = project()
111+
.executable(Path::new("path-test").join("cargo-1"), "")
112+
.build();
113+
114+
let mut path = path();
115+
path.push(proj.root().join("path-test"));
116+
let path = env::join_paths(path.iter()).unwrap();
117+
118+
// See issue #11814: Environment variable names are case-insensitive on Windows.
119+
// We need to check that having "Path" instead of "PATH" is okay.
120+
let output = cargo_process("-v --list")
121+
.env("Path", &path)
122+
.env_remove("PATH")
123+
.exec_with_output()
124+
.unwrap();
125+
let output = str::from_utf8(&output.stdout).unwrap();
126+
assert!(
127+
output.contains("\n 1 "),
128+
"missing 1: {}",
129+
output
130+
);
131+
}
132+
107133
#[cargo_test]
108134
fn list_command_handles_known_external_commands() {
109135
let p = project()

0 commit comments

Comments
 (0)