Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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![]
Expand Down
11 changes: 5 additions & 6 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)?
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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));
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/cargo/core/compiler/context/compilation_files.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down
5 changes: 2 additions & 3 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}

Expand All @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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,
};
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`?"
)?;
Expand Down
7 changes: 4 additions & 3 deletions src/cargo/ops/cargo_doc.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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())?;
}
}

Expand All @@ -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)) => {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/common_for_install_and_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
36 changes: 22 additions & 14 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ pub fn fix_exec_rustc(config: &Config, lock_addr: &str) -> CargoResult<()> {
let args = FixArgs::get()?;
trace!("cargo-fix as rustc got file {:?}", args.file);

let workspace_rustc = std::env::var("RUSTC_WORKSPACE_WRAPPER")
let workspace_rustc = config
.get_env("RUSTC_WORKSPACE_WRAPPER")
.map(PathBuf::from)
.ok();
let mut rustc = ProcessBuilder::new(&args.rustc).wrapped(workspace_rustc.as_ref());
Expand Down Expand Up @@ -388,7 +389,7 @@ pub fn fix_exec_rustc(config: &Config, lock_addr: &str) -> CargoResult<()> {
file: path.clone(),
fixes: file.fixes_applied,
}
.post()?;
.post(config)?;
}
}

Expand All @@ -403,7 +404,7 @@ pub fn fix_exec_rustc(config: &Config, lock_addr: &str) -> CargoResult<()> {
// user's code with our changes. Back out everything and fall through
// below to recompile again.
if !output.status.success() {
if env::var_os(BROKEN_CODE_ENV).is_none() {
if config.get_env_os(BROKEN_CODE_ENV).is_none() {
for (path, file) in fixes.files.iter() {
debug!("reverting {:?} due to errors", path);
paths::write(path, &file.original_code)?;
Expand All @@ -420,7 +421,7 @@ pub fn fix_exec_rustc(config: &Config, lock_addr: &str) -> CargoResult<()> {
}
krate
};
log_failed_fix(krate, &output.stderr, output.status)?;
log_failed_fix(config, krate, &output.stderr, output.status)?;
}
}

Expand Down Expand Up @@ -510,7 +511,8 @@ fn rustfix_crate(
// definitely can't make progress, so bail out.
let mut fixes = FixedCrate::default();
let mut last_fix_counts = HashMap::new();
let iterations = env::var("CARGO_FIX_MAX_RETRIES")
let iterations = config
.get_env("CARGO_FIX_MAX_RETRIES")
.ok()
.and_then(|n| n.parse().ok())
.unwrap_or(4);
Expand Down Expand Up @@ -547,7 +549,7 @@ fn rustfix_crate(
file: path.clone(),
message: error,
}
.post()?;
.post(config)?;
}
}

Expand Down Expand Up @@ -576,7 +578,7 @@ fn rustfix_and_fix(
// worse by applying fixes where a bug could cause *more* broken code.
// Instead, punt upwards which will reexec rustc over the original code,
// displaying pretty versions of the diagnostics we just read out.
if !output.status.success() && env::var_os(BROKEN_CODE_ENV).is_none() {
if !output.status.success() && config.get_env_os(BROKEN_CODE_ENV).is_none() {
debug!(
"rustfixing `{:?}` failed, rustc exited with {:?}",
filename,
Expand All @@ -585,7 +587,8 @@ fn rustfix_and_fix(
return Ok(());
}

let fix_mode = env::var_os("__CARGO_FIX_YOLO")
let fix_mode = config
.get_env_os("__CARGO_FIX_YOLO")
.map(|_| rustfix::Filter::Everything)
.unwrap_or(rustfix::Filter::MachineApplicableOnly);

Expand Down Expand Up @@ -710,7 +713,12 @@ fn exit_with(status: ExitStatus) -> ! {
process::exit(status.code().unwrap_or(3));
}

fn log_failed_fix(krate: Option<String>, stderr: &[u8], status: ExitStatus) -> CargoResult<()> {
fn log_failed_fix(
config: &Config,
krate: Option<String>,
stderr: &[u8],
status: ExitStatus,
) -> CargoResult<()> {
let stderr = str::from_utf8(stderr).context("failed to parse rustc stderr as utf-8")?;

let diagnostics = stderr
Expand Down Expand Up @@ -745,7 +753,7 @@ fn log_failed_fix(krate: Option<String>, stderr: &[u8], status: ExitStatus) -> C
errors,
abnormal_exit,
}
.post()?;
.post(config)?;

Ok(())
}
Expand Down Expand Up @@ -895,7 +903,7 @@ impl FixArgs {
return Message::Fixing {
file: self.file.display().to_string(),
}
.post()
.post(config)
.and(Ok(true));
}
};
Expand All @@ -922,7 +930,7 @@ impl FixArgs {
message,
edition: to_edition.previous().unwrap(),
}
.post()
.post(config)
.and(Ok(false)); // Do not run rustfix for this the edition.
}
let from_edition = self.enabled_edition.unwrap_or(Edition::Edition2015);
Expand All @@ -937,14 +945,14 @@ impl FixArgs {
message,
edition: to_edition,
}
.post()
.post(config)
} else {
Message::Migrating {
file: self.file.display().to_string(),
from_edition,
to_edition,
}
.post()
.post(config)
}
.and(Ok(true))
}
Expand Down
Loading