Skip to content

Commit

Permalink
clippy: allow usages of std::env::var family by exceptions
Browse files Browse the repository at this point in the history
See comments above for each usage about why it is allowed.
  • Loading branch information
weihanglo committed Mar 17, 2023
1 parent 9e3bb85 commit f78d081
Show file tree
Hide file tree
Showing 12 changed files with 93 additions and 15 deletions.
10 changes: 6 additions & 4 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ use std::process::Command;
fn main() {
commit_info();
compress_man();
println!(
"cargo:rustc-env=RUST_HOST_TARGET={}",
std::env::var("TARGET").unwrap()
);
// ALLOWED: Accessing environment during build time shouldn't be prohibited.
#[allow(clippy::disallowed_methods)]
let target = std::env::var("TARGET").unwrap();
println!("cargo:rustc-env=RUST_HOST_TARGET={target}");
}

fn compress_man() {
// ALLOWED: Accessing environment during build time shouldn't be prohibited.
#[allow(clippy::disallowed_methods)]
let out_path = Path::new(&std::env::var("OUT_DIR").unwrap()).join("man.tgz");
let dst = fs::File::create(out_path).unwrap();
let encoder = GzBuilder::new()
Expand Down
3 changes: 3 additions & 0 deletions src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,9 @@ impl GlobalArgs {
}

pub fn cli() -> Command {
// ALLOWED: `RUSTUP_HOME` should only be read from process env, otherwise
// other tools may point to executables from incompatible distributions.
#[allow(clippy::disallowed_methods)]
let is_rustup = std::env::var_os("RUSTUP_HOME").is_some();
let usage = if is_rustup {
"cargo [+toolchain] [OPTIONS] [COMMAND]"
Expand Down
8 changes: 8 additions & 0 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
// If we're opting into backtraces, mention that build dependencies' backtraces can
// be improved by requesting debuginfo to be built, if we're not building with
// debuginfo already.
//
// ALLOWED: Other tools like `rustc` might read it directly
// through `std::env`. We should make their behavior consistent.
#[allow(clippy::disallowed_methods)]
if let Ok(show_backtraces) = std::env::var("RUST_BACKTRACE") {
if !built_with_debuginfo && show_backtraces != "0" {
build_error_context.push_str(&format!(
Expand Down Expand Up @@ -727,6 +731,10 @@ impl BuildOutput {
None => return false,
Some(n) => n,
};
// ALLOWED: the process of rustc boostrapping reads this through
// `std::env`. We should make the behavior consistent. Also, we
// don't advertise this for bypassing nightly.
#[allow(clippy::disallowed_methods)]
std::env::var("RUSTC_BOOTSTRAP")
.map_or(false, |var| var.split(',').any(|s| s == name))
};
Expand Down
18 changes: 14 additions & 4 deletions src/cargo/core/compiler/fingerprint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,19 @@ pub enum StaleItem {
}

impl LocalFingerprint {
/// Read the environment variable of the given env `key`, and creates a new
/// [`LocalFingerprint::RerunIfEnvChanged`] for it.
///
// TODO: This is allowed at this moment. Should figure out if it makes
// sense if permitting to read env from the config system.
#[allow(clippy::disallowed_methods)]
fn from_env<K: AsRef<str>>(key: K) -> LocalFingerprint {
let key = key.as_ref();
let var = key.to_owned();
let val = env::var(key).ok();
LocalFingerprint::RerunIfEnvChanged { var, val }
}

/// Checks dynamically at runtime if this `LocalFingerprint` has a stale
/// item inside of it.
///
Expand Down Expand Up @@ -1661,10 +1674,7 @@ fn local_fingerprints_deps(
local.extend(
deps.rerun_if_env_changed
.iter()
.map(|var| LocalFingerprint::RerunIfEnvChanged {
var: var.clone(),
val: env::var(var).ok(),
}),
.map(LocalFingerprint::from_env),
);

local
Expand Down
20 changes: 16 additions & 4 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -946,10 +946,7 @@ impl CliUnstable {
self.add(flag, &mut warnings)?;
}

if self.gitoxide.is_none()
&& std::env::var_os("__CARGO_USE_GITOXIDE_INSTEAD_OF_GIT2")
.map_or(false, |value| value == "1")
{
if self.gitoxide.is_none() && cargo_use_gitoxide_instead_of_git2() {
self.gitoxide = GitoxideFeatures::safe().into();
}
Ok(warnings)
Expand Down Expand Up @@ -1171,9 +1168,15 @@ impl CliUnstable {

/// Returns the current release channel ("stable", "beta", "nightly", "dev").
pub fn channel() -> String {
// ALLOWED: For testing cargo itself only.
#[allow(clippy::disallowed_methods)]
if let Ok(override_channel) = env::var("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS") {
return override_channel;
}
// ALLOWED: the process of rustc boostrapping reads this through
// `std::env`. We should make the behavior consistent. Also, we
// don't advertise this for bypassing nightly.
#[allow(clippy::disallowed_methods)]
if let Ok(staging) = env::var("RUSTC_BOOTSTRAP") {
if staging == "1" {
return "dev".to_string();
Expand All @@ -1183,3 +1186,12 @@ pub fn channel() -> String {
.release_channel
.unwrap_or_else(|| String::from("dev"))
}

/// Only for testing and developing. See ["Running with gitoxide as default git backend in tests"][1].
///
/// [1]: https://doc.crates.io/contrib/tests/running.html#running-with-gitoxide-as-default-git-backend-in-tests
// ALLOWED: For testing cargo itself only.
#[allow(clippy::disallowed_methods)]
fn cargo_use_gitoxide_instead_of_git2() -> bool {
std::env::var_os("__CARGO_USE_GITOXIDE_INSTEAD_OF_GIT2").map_or(false, |value| value == "1")
}
6 changes: 6 additions & 0 deletions src/cargo/core/resolver/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ pub struct ResolverProgress {
time_to_print: Duration,
printed: bool,
deps_time: Duration,
/// Provides an escape hatch for machine with slow CPU for debugging and
/// testing Cargo itself.
/// See [rust-lang/cargo#6596](https://github.com/rust-lang/cargo/pull/6596) for more.
#[cfg(debug_assertions)]
slow_cpu_multiplier: u64,
}
Expand All @@ -31,6 +34,9 @@ impl ResolverProgress {
// Architectures that do not have a modern processor, hardware emulation, etc.
// In the test code we have `slow_cpu_multiplier`, but that is not accessible here.
#[cfg(debug_assertions)]
// ALLOWED: For testing cargo itself only. However, it was communicated as an public
// interface to other developers, so keep it as-is, shouldn't add `__CARGO` prefix.
#[allow(clippy::disallowed_methods)]
slow_cpu_multiplier: std::env::var("CARGO_TEST_SLOW_CPU_MULTIPLIER")
.ok()
.and_then(|m| m.parse().ok())
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/core/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ impl TtyWidth {
/// Returns the width of the terminal to use for diagnostics (which is
/// relayed to rustc via `--diagnostic-width`).
pub fn diagnostic_terminal_width(&self) -> Option<usize> {
// ALLOWED: For testing cargo itself only.
#[allow(clippy::disallowed_methods)]
if let Ok(width) = std::env::var("__CARGO_TEST_TTY_WIDTH_DO_NOT_USE_THIS") {
return Some(width.parse().unwrap());
}
Expand Down
11 changes: 8 additions & 3 deletions src/cargo/core/source/source_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,9 +428,7 @@ impl SourceId {
_ => return false,
}
let url = self.inner.url.as_str();
url == CRATES_IO_INDEX
|| url == CRATES_IO_HTTP_INDEX
|| std::env::var("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS").as_deref() == Ok(url)
url == CRATES_IO_INDEX || url == CRATES_IO_HTTP_INDEX || is_overridden_crates_io_url(url)
}

/// Hashes `self`.
Expand Down Expand Up @@ -884,3 +882,10 @@ mod tests {
assert_eq!(source_id, deserialized);
}
}

/// For testing Cargo itself only.
/// Check if `url` equals to the overridden crates.io URL.
#[allow(clippy::disallowed_methods)]
fn is_overridden_crates_io_url(url: &str) -> bool {
std::env::var("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS").map_or(false, |v| v == url)
}
19 changes: 19 additions & 0 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,19 @@ use crate::util::Config;
use crate::util::{existing_vcs_repo, LockServer, LockServerClient};
use crate::{drop_eprint, drop_eprintln};

/// **Internal only.**
/// Indicates Cargo is in fix-proxy-mode if presents.
/// The value of it is the socket address of the [`LockServer`] being used.
/// See the [module-level documentation](mod@super::fix) for more.
const FIX_ENV: &str = "__CARGO_FIX_PLZ";
/// **Internal only.**
/// For passing [`FixOptions::broken_code`] through to cargo running in proxy mode.
const BROKEN_CODE_ENV: &str = "__CARGO_FIX_BROKEN_CODE";
/// **Internal only.**
/// For passing [`FixOptions::edition`] through to cargo running in proxy mode.
const EDITION_ENV: &str = "__CARGO_FIX_EDITION";
/// **Internal only.**
/// For passing [`FixOptions::idioms`] through to cargo running in proxy mode.
const IDIOMS_ENV: &str = "__CARGO_FIX_IDIOMS";

pub struct FixOptions {
Expand Down Expand Up @@ -339,6 +349,9 @@ to prevent this issue from happening.
/// Returns `None` if `fix` is not being run (not in proxy mode). Returns
/// `Some(...)` if in `fix` proxy mode
pub fn fix_get_proxy_lock_addr() -> Option<String> {
// ALLOWED: For the internal mechanism of `cargo fix` only.
// Shouldn't be set directly by anyone.
#[allow(clippy::disallowed_methods)]
env::var(FIX_ENV).ok()
}

Expand Down Expand Up @@ -847,8 +860,14 @@ impl FixArgs {
}

let file = file.ok_or_else(|| anyhow::anyhow!("could not find .rs file in rustc args"))?;
// ALLOWED: For the internal mechanism of `cargo fix` only.
// Shouldn't be set directly by anyone.
#[allow(clippy::disallowed_methods)]
let idioms = env::var(IDIOMS_ENV).is_ok();

// ALLOWED: For the internal mechanism of `cargo fix` only.
// Shouldn't be set directly by anyone.
#[allow(clippy::disallowed_methods)]
let prepare_for_edition = env::var(EDITION_ENV).ok().map(|_| {
enabled_edition
.unwrap_or(Edition::Edition2015)
Expand Down
5 changes: 5 additions & 0 deletions src/cargo/util/config/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ pub struct Env {
impl Env {
/// Create a new `Env` from process's environment variables.
pub fn new() -> Self {
// ALLOWED: This is the only permissible usage of `std::env::vars{_os}`
// within cargo. If you do need access to individual variables without
// interacting with `Config` system, please use `std::env::var{_os}`
// and justify the validity of the usage.
#[allow(clippy::disallowed_methods)]
let env: HashMap<_, _> = std::env::vars_os().collect();
let (case_insensitive_env, normalized_env) = make_case_insensitive_and_normalized_env(&env);
Self {
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/util/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ mod imp {
// when-cargo-is-killed-subprocesses-are-also-killed, but that requires
// one cargo spawned to become its own session leader, so we do that
// here.
//
// ALLOWED: For testing cargo itself only.
#[allow(clippy::disallowed_methods)]
if env::var("__CARGO_TEST_SETSID_PLEASE_DONT_USE_ELSEWHERE").is_ok() {
libc::setsid();
}
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/util/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ pub struct Profiler {
}

fn enabled_level() -> Option<usize> {
// `CARGO_PROFILE` is for profiling Cargo itself,
// not intended to be used beyond Cargo contributors.
#[allow(clippy::disallowed_methods)]
env::var("CARGO_PROFILE").ok().and_then(|s| s.parse().ok())
}

Expand Down

0 comments on commit f78d081

Please sign in to comment.