Auto merge of #11727 - kylematsuda:config-env, r=ehuss

Read environment variables through `Config` instead of `std::env::var(_os)`

### What does this PR try to resolve?

Adds two new methods `get_env` and `get_env_os` for reading environment variables from the `Config` and uses them to replace the many of the calls to `std::env::var` and `var_os`  (see the discussion in #11588).

Closes #11588

Author: bors

src/bin/cargo/main.rs

@@ -235,7 +235,7 @@ fn is_executable<P: AsRef<Path>>(path: P) -> bool {
 }
 
 fn search_directories(config: &Config) -> Vec<PathBuf> {
-    let mut path_dirs = if let Some(val) = env::var_os("PATH") {
+    let mut path_dirs = if let Some(val) = config.get_env_os("PATH") {
         env::split_paths(&val).collect()
     } else {
         vec![]

src/cargo/core/compiler/build_context/target_info.rs

@@ -20,7 +20,6 @@ use cargo_util::{paths, ProcessBuilder};
 use serde::{Deserialize, Serialize};
 use std::cell::RefCell;
 use std::collections::hash_map::{Entry, HashMap};
-use std::env;
 use std::path::{Path, PathBuf};
 use std::str::{self, FromStr};
 
@@ -731,7 +730,7 @@ fn extra_args(
     // NOTE: It is impossible to have a [host] section and reach this logic with kind.is_host(),
     // since [host] implies `target-applies-to-host = false`, which always early-returns above.
 
-    if let Some(rustflags) = rustflags_from_env(flags) {
+    if let Some(rustflags) = rustflags_from_env(config, flags) {
         Ok(rustflags)
     } else if let Some(rustflags) =
         rustflags_from_target(config, host_triple, target_cfg, kind, flags)?
@@ -746,18 +745,18 @@ fn extra_args(
 
 /// Gets compiler flags from environment variables.
 /// See [`extra_args`] for more.
-fn rustflags_from_env(flags: Flags) -> Option<Vec<String>> {
+fn rustflags_from_env(config: &Config, flags: Flags) -> Option<Vec<String>> {
     // First try CARGO_ENCODED_RUSTFLAGS from the environment.
     // Prefer this over RUSTFLAGS since it's less prone to encoding errors.
-    if let Ok(a) = env::var(format!("CARGO_ENCODED_{}", flags.as_env())) {
+    if let Ok(a) = config.get_env(format!("CARGO_ENCODED_{}", flags.as_env())) {
         if a.is_empty() {
             return Some(Vec::new());
         }
         return Some(a.split('\x1f').map(str::to_string).collect());
     }
 
     // Then try RUSTFLAGS from the environment
-    if let Ok(a) = env::var(flags.as_env()) {
+    if let Ok(a) = config.get_env(flags.as_env()) {
         let args = a
             .split(' ')
             .map(str::trim)
@@ -855,7 +854,7 @@ pub struct RustcTargetData<'cfg> {
     pub rustc: Rustc,
 
     /// Config
-    config: &'cfg Config,
+    pub config: &'cfg Config,
     requested_kinds: Vec<CompileKind>,
 
     /// Build information for the "host", which is information about when

src/cargo/core/compiler/compilation.rs

@@ -1,7 +1,6 @@
 //! Type definitions for the result of a compilation.
 
 use std::collections::{BTreeSet, HashMap};
-use std::env;
 use std::ffi::{OsStr, OsString};
 use std::path::PathBuf;
 
@@ -295,7 +294,7 @@ impl<'cfg> Compilation<'cfg> {
             // These are the defaults when DYLD_FALLBACK_LIBRARY_PATH isn't
             // set or set to an empty string. Since Cargo is explicitly setting
             // the value, make sure the defaults still work.
-            if let Some(home) = env::var_os("HOME") {
+            if let Some(home) = self.config.get_env_os("HOME") {
                 search_path.push(PathBuf::from(home).join("lib"));
             }
             search_path.push(PathBuf::from("/usr/local/lib"));
@@ -362,7 +361,7 @@ impl<'cfg> Compilation<'cfg> {
                 continue;
             }
 
-            if value.is_force() || env::var_os(key).is_none() {
+            if value.is_force() || self.config.get_env_os(key).is_none() {
                 cmd.env(key, value.resolve(self.config));
             }
         }

src/cargo/core/compiler/context/compilation_files.rs

@@ -1,7 +1,6 @@
 //! See [`CompilationFiles`].
 
 use std::collections::HashMap;
-use std::env;
 use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::path::{Path, PathBuf};
@@ -628,7 +627,7 @@ fn compute_metadata(
 
     // Seed the contents of `__CARGO_DEFAULT_LIB_METADATA` to the hasher if present.
     // This should be the release channel, to get a different hash for each channel.
-    if let Ok(ref channel) = env::var("__CARGO_DEFAULT_LIB_METADATA") {
+    if let Ok(ref channel) = cx.bcx.config.get_env("__CARGO_DEFAULT_LIB_METADATA") {
         channel.hash(&mut hasher);
     }
 
@@ -717,7 +716,7 @@ fn should_use_metadata(bcx: &BuildContext<'_, '_>, unit: &Unit) -> bool {
         || (unit.target.is_executable() && short_name == "wasm32-unknown-emscripten")
         || (unit.target.is_executable() && short_name.contains("msvc")))
         && unit.pkg.package_id().source_id().is_path()
-        && env::var("__CARGO_DEFAULT_LIB_METADATA").is_err()
+        && bcx.config.get_env("__CARGO_DEFAULT_LIB_METADATA").is_err()
     {
         return false;
     }

src/cargo/core/compiler/standard_lib.rs

@@ -11,7 +11,6 @@ use crate::ops::{self, Packages};
 use crate::util::errors::CargoResult;
 use crate::Config;
 use std::collections::{HashMap, HashSet};
-use std::env;
 use std::path::PathBuf;
 
 use super::BuildConfig;
@@ -222,7 +221,7 @@ pub fn generate_std_roots(
 }
 
 fn detect_sysroot_src_path(target_data: &RustcTargetData<'_>) -> CargoResult<PathBuf> {
-    if let Some(s) = env::var_os("__CARGO_TESTS_ONLY_SRC_ROOT") {
+    if let Some(s) = target_data.config.get_env_os("__CARGO_TESTS_ONLY_SRC_ROOT") {
         return Ok(s.into());
     }
 
@@ -241,7 +240,7 @@ fn detect_sysroot_src_path(target_data: &RustcTargetData<'_>) -> CargoResult<Pat
              library, try:\n        rustup component add rust-src",
             lock
         );
-        match env::var("RUSTUP_TOOLCHAIN") {
+        match target_data.config.get_env("RUSTUP_TOOLCHAIN") {
             Ok(rustup_toolchain) => {
                 anyhow::bail!("{} --toolchain {}", msg, rustup_toolchain);
             }

src/cargo/core/profiles.rs

@@ -31,7 +31,7 @@ use crate::util::{closest_msg, config, CargoResult, Config};
 use anyhow::{bail, Context as _};
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::hash::Hash;
-use std::{cmp, env, fmt, hash};
+use std::{cmp, fmt, hash};
 
 /// Collection of all profiles.
 ///
@@ -62,7 +62,7 @@ pub struct Profiles {
 impl Profiles {
     pub fn new(ws: &Workspace<'_>, requested_profile: InternedString) -> CargoResult<Profiles> {
         let config = ws.config();
-        let incremental = match env::var_os("CARGO_INCREMENTAL") {
+        let incremental = match config.get_env_os("CARGO_INCREMENTAL") {
             Some(v) => Some(v == "1"),
             None => config.build_config()?.incremental,
         };

src/cargo/ops/cargo_compile/mod.rs

@@ -215,14 +215,14 @@ pub fn create_bcx<'a, 'cfg>(
         | CompileMode::Check { .. }
         | CompileMode::Bench
         | CompileMode::RunCustomBuild => {
-            if std::env::var("RUST_FLAGS").is_ok() {
+            if ws.config().get_env("RUST_FLAGS").is_ok() {
                 config.shell().warn(
                     "Cargo does not read `RUST_FLAGS` environment variable. Did you mean `RUSTFLAGS`?",
                 )?;
             }
         }
         CompileMode::Doc { .. } | CompileMode::Doctest | CompileMode::Docscrape => {
-            if std::env::var("RUSTDOC_FLAGS").is_ok() {
+            if ws.config().get_env("RUSTDOC_FLAGS").is_ok() {
                 config.shell().warn(
                     "Cargo does not read `RUSTDOC_FLAGS` environment variable. Did you mean `RUSTDOCFLAGS`?"
                 )?;

src/cargo/ops/cargo_config.rs

@@ -93,7 +93,7 @@ fn maybe_env<'config>(
     config: &'config Config,
     key: &ConfigKey,
     cv: &CV,
-) -> Option<Vec<(&'config String, &'config String)>> {
+) -> Option<Vec<(&'config str, &'config str)>> {
     // Only fetching a table is unable to load env values. Leaf entries should
     // work properly.
     match cv {
@@ -102,7 +102,6 @@ fn maybe_env<'config>(
     }
     let mut env: Vec<_> = config
         .env()
-        .iter()
         .filter(|(env_key, _val)| env_key.starts_with(&format!("{}_", key.as_env_key())))
         .collect();
     env.sort_by_key(|x| x.0);
@@ -162,7 +161,7 @@ fn print_toml(config: &Config, opts: &GetOptions<'_>, key: &ConfigKey, cv: &CV)
     }
 }
 
-fn print_toml_env(config: &Config, env: &[(&String, &String)]) {
+fn print_toml_env(config: &Config, env: &[(&str, &str)]) {
     drop_println!(
         config,
         "# The following environment variables may affect the loaded values."
@@ -173,7 +172,7 @@ fn print_toml_env(config: &Config, env: &[(&String, &String)]) {
     }
 }
 
-fn print_json_env(config: &Config, env: &[(&String, &String)]) {
+fn print_json_env(config: &Config, env: &[(&str, &str)]) {
     drop_eprintln!(
         config,
         "note: The following environment variables may affect the loaded values."
@@ -287,7 +286,6 @@ fn print_toml_unmerged(config: &Config, opts: &GetOptions<'_>, key: &ConfigKey)
     // special, and will just naturally get loaded as part of the config.
     let mut env: Vec<_> = config
         .env()
-        .iter()
         .filter(|(env_key, _val)| env_key.starts_with(key.as_env_key()))
         .collect();
     if !env.is_empty() {

src/cargo/ops/cargo_doc.rs

@@ -1,6 +1,6 @@
 use crate::core::{Shell, Workspace};
 use crate::ops;
-use crate::util::config::PathAndArgs;
+use crate::util::config::{Config, PathAndArgs};
 use crate::util::CargoResult;
 use std::path::Path;
 use std::path::PathBuf;
@@ -37,7 +37,7 @@ pub fn doc(ws: &Workspace<'_>, options: &DocOptions) -> CargoResult<()> {
 
             let mut shell = ws.config().shell();
             shell.status("Opening", path.display())?;
-            open_docs(&path, &mut shell, config_browser)?;
+            open_docs(&path, &mut shell, config_browser, ws.config())?;
         }
     }
 
@@ -48,9 +48,10 @@ fn open_docs(
     path: &Path,
     shell: &mut Shell,
     config_browser: Option<(PathBuf, Vec<String>)>,
+    config: &Config,
 ) -> CargoResult<()> {
     let browser =
-        config_browser.or_else(|| Some((PathBuf::from(std::env::var_os("BROWSER")?), Vec::new())));
+        config_browser.or_else(|| Some((PathBuf::from(config.get_env_os("BROWSER")?), Vec::new())));
 
     match browser {
         Some((browser, initial_args)) => {

src/cargo/ops/cargo_install.rs

@@ -704,7 +704,7 @@ pub fn install(
     if installed_anything {
         // Print a warning that if this directory isn't in PATH that they won't be
         // able to run these commands.
-        let path = env::var_os("PATH").unwrap_or_default();
+        let path = config.get_env_os("PATH").unwrap_or_default();
         let dst_in_path = env::split_paths(&path).any(|path| path == dst);
 
         if !dst_in_path {

src/cargo/ops/cargo_new.rs

@@ -463,7 +463,7 @@ pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> {
 
 pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<NewProjectKind> {
     // This is here just as a random location to exercise the internal error handling.
-    if std::env::var_os("__CARGO_TEST_INTERNAL_ERROR").is_some() {
+    if config.get_env_os("__CARGO_TEST_INTERNAL_ERROR").is_some() {
         return Err(crate::util::internal("internal error test"));
     }
 

src/cargo/ops/common_for_install_and_uninstall.rs

@@ -506,7 +506,7 @@ pub fn resolve_root(flag: Option<&str>, config: &Config) -> CargoResult<Filesyst
     let config_root = config.get_path("install.root")?;
     Ok(flag
         .map(PathBuf::from)
-        .or_else(|| env::var_os("CARGO_INSTALL_ROOT").map(PathBuf::from))
+        .or_else(|| config.get_env_os("CARGO_INSTALL_ROOT").map(PathBuf::from))
         .or_else(move || config_root.map(|v| v.val))
         .map(Filesystem::new)
         .unwrap_or_else(|| config.home().clone()))