From c52845251290885fb22d43b3e296d87367ca7091 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 9 Apr 2024 13:47:37 +0200 Subject: [PATCH 1/9] Simplify process access to argument iterator --- src/cli/common.rs | 1 - src/cli/proxy_mode.rs | 5 ++- src/cli/rustup_mode.rs | 4 +- src/currentprocess.rs | 20 ++++++++-- src/currentprocess/argsource.rs | 66 --------------------------------- 5 files changed, 21 insertions(+), 75 deletions(-) delete mode 100644 src/currentprocess/argsource.rs diff --git a/src/cli/common.rs b/src/cli/common.rs index be0c6237ae..a0b74bf9ce 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -15,7 +15,6 @@ use once_cell::sync::Lazy; use super::self_update; use crate::cli::download_tracker::DownloadTracker; use crate::currentprocess::{ - argsource::ArgSource, filesource::{StdinSource, StdoutSource}, process, terminalsource, varsource::VarSource, diff --git a/src/cli/proxy_mode.rs b/src/cli/proxy_mode.rs index 331d73737c..3746a3722d 100644 --- a/src/cli/proxy_mode.rs +++ b/src/cli/proxy_mode.rs @@ -7,7 +7,7 @@ use crate::{ cli::{common::set_globals, job, self_update}, command::run_command_for_dir, config::Cfg, - currentprocess::{argsource::ArgSource, process}, + currentprocess::process, toolchain::names::{LocalToolchainName, ResolvableLocalToolchainName}, utils::utils, }; @@ -18,7 +18,8 @@ pub async fn main(arg0: &str) -> Result { let _setup = job::setup(); - let mut args = process().args_os().skip(1); + let process = process(); + let mut args = process.args_os().skip(1); // Check for a + toolchain specifier let arg1 = args.next(); diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index b24c39dd50..a02e4f89b3 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -24,7 +24,6 @@ use crate::{ command, config::{new_toolchain_with_reason, ActiveReason, Cfg}, currentprocess::{ - argsource::ArgSource, filesource::{StderrSource, StdoutSource}, process, terminalsource::{self, ColorableTerminal}, @@ -1513,7 +1512,8 @@ async fn man( fn set_auto_self_update(cfg: &mut Cfg, auto_self_update_mode: &str) -> Result { if self_update::NEVER_SELF_UPDATE { - let mut args = process().args_os(); + let process = process(); + let mut args = process.args_os(); let arg0 = args.next().map(PathBuf::from); let arg0 = arg0 .as_ref() diff --git a/src/currentprocess.rs b/src/currentprocess.rs index 4523e4e6f8..323fa5e3b3 100644 --- a/src/currentprocess.rs +++ b/src/currentprocess.rs @@ -20,14 +20,12 @@ use home::env as home; #[cfg(feature = "test")] use rand::{thread_rng, Rng}; -pub mod argsource; pub mod cwdsource; pub mod filesource; mod homethunk; pub mod terminalsource; pub mod varsource; -use argsource::*; use cwdsource::*; use filesource::*; use varsource::*; @@ -69,7 +67,6 @@ use varsource::*; #[enum_dispatch] pub trait CurrentProcess: home::Env - + ArgSource + CurrentDirSource + VarSource + StdoutSource @@ -84,7 +81,6 @@ pub trait CurrentProcess: #[derive(Clone, Debug)] #[enum_dispatch( CurrentProcess, - ArgSource, CurrentDirSource, VarSource, StdoutSource, @@ -111,6 +107,22 @@ impl Process { .and_then(std::ffi::OsStr::to_str) .map(String::from) } + + pub(crate) fn args(&self) -> Box + '_> { + match self { + Process::OSProcess(_) => Box::new(env::args()), + #[cfg(feature = "test")] + Process::TestProcess(p) => Box::new(p.args.iter().cloned()), + } + } + + pub(crate) fn args_os(&self) -> Box + '_> { + match self { + Process::OSProcess(_) => Box::new(env::args_os()), + #[cfg(feature = "test")] + Process::TestProcess(p) => Box::new(p.args.iter().map(OsString::from)), + } + } } /// Obtain the current instance of CurrentProcess diff --git a/src/currentprocess/argsource.rs b/src/currentprocess/argsource.rs deleted file mode 100644 index b3eecf7755..0000000000 --- a/src/currentprocess/argsource.rs +++ /dev/null @@ -1,66 +0,0 @@ -/// Abstracts over reading the current process environment variables as a -/// zero-cost abstraction to support threaded in-process testing. -use std::env; -use std::ffi::OsString; -use std::marker::PhantomData; - -use enum_dispatch::enum_dispatch; - -#[enum_dispatch] -pub trait ArgSource { - fn args(&self) -> Box>; - fn args_os(&self) -> Box>; -} - -/// Implements ArgSource with `std::env::args` -impl ArgSource for super::OSProcess { - fn args(&self) -> Box> { - Box::new(env::args()) - } - fn args_os(&self) -> Box> { - Box::new(env::args_os()) - } -} - -/// Helper for ArgSource over `Vec` -pub(crate) struct VecArgs { - v: Vec, - i: usize, - _marker: PhantomData, -} - -impl From<&Vec> for VecArgs { - fn from(source: &Vec) -> Self { - let v = source.clone(); - VecArgs { - v, - i: 0, - _marker: PhantomData, - } - } -} - -impl> Iterator for VecArgs { - type Item = T; - fn next(&mut self) -> Option { - if self.i == self.v.len() { - return None; - } - let i = self.i; - self.i += 1; - Some(T::from(self.v[i].clone())) - } - fn size_hint(&self) -> (usize, Option) { - (self.v.len(), Some(self.v.len())) - } -} - -#[cfg(feature = "test")] -impl ArgSource for super::TestProcess { - fn args(&self) -> Box> { - Box::new(VecArgs::::from(&self.args)) - } - fn args_os(&self) -> Box> { - Box::new(VecArgs::::from(&self.args)) - } -} From 5230106604862349688334d1d0f9ed8359968bc5 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 9 Apr 2024 13:53:29 +0200 Subject: [PATCH 2/9] Simplify process access to stdout --- src/cli/common.rs | 4 +--- src/cli/download_tracker.rs | 2 +- src/cli/rustup_mode.rs | 2 +- src/cli/self_update.rs | 2 +- src/cli/self_update/windows.rs | 2 +- src/cli/setup_mode.rs | 2 +- src/currentprocess.rs | 10 ++++++++-- src/currentprocess/filesource.rs | 22 +--------------------- 8 files changed, 15 insertions(+), 31 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index a0b74bf9ce..ca4bb084d4 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -15,9 +15,7 @@ use once_cell::sync::Lazy; use super::self_update; use crate::cli::download_tracker::DownloadTracker; use crate::currentprocess::{ - filesource::{StdinSource, StdoutSource}, - process, terminalsource, - varsource::VarSource, + filesource::StdinSource, process, terminalsource, varsource::VarSource, }; use crate::dist::dist::{TargetTriple, ToolchainDesc}; use crate::dist::manifest::ComponentStatus; diff --git a/src/cli/download_tracker.rs b/src/cli/download_tracker.rs index dbc10c9593..8937e2a6a2 100644 --- a/src/cli/download_tracker.rs +++ b/src/cli/download_tracker.rs @@ -4,7 +4,7 @@ use std::io::Write; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; -use crate::currentprocess::{filesource::StdoutSource, process, terminalsource}; +use crate::currentprocess::{process, terminalsource}; use crate::dist::Notification as In; use crate::notifications::Notification; use crate::utils::units::{Size, Unit, UnitMode}; diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index a02e4f89b3..618316d8df 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -24,7 +24,7 @@ use crate::{ command, config::{new_toolchain_with_reason, ActiveReason, Cfg}, currentprocess::{ - filesource::{StderrSource, StdoutSource}, + filesource::StderrSource, process, terminalsource::{self, ColorableTerminal}, }, diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 5ae5559e70..bd231fe4c5 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -66,7 +66,7 @@ use crate::{ markdown::md, }, config::Cfg, - currentprocess::{filesource::StdoutSource, process, varsource::VarSource}, + currentprocess::{process, varsource::VarSource}, dist::dist::{self, PartialToolchainDesc, Profile, TargetTriple, ToolchainDesc}, install::UpdateStatus, toolchain::{ diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 2102afd418..8025f9497e 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -12,7 +12,7 @@ use super::super::errors::*; use super::common; use super::{install_bins, InstallOpts}; use crate::cli::download_tracker::DownloadTracker; -use crate::currentprocess::{filesource::StdoutSource, process, varsource::VarSource}; +use crate::currentprocess::{process, varsource::VarSource}; use crate::dist::dist::TargetTriple; use crate::utils::utils; use crate::utils::Notification; diff --git a/src/cli/setup_mode.rs b/src/cli/setup_mode.rs index d68c39a1e8..5dd32b49be 100644 --- a/src/cli/setup_mode.rs +++ b/src/cli/setup_mode.rs @@ -6,7 +6,7 @@ use crate::{ common, self_update::{self, InstallOpts}, }, - currentprocess::{filesource::StdoutSource, process}, + currentprocess::process, dist::dist::Profile, toolchain::names::MaybeOfficialToolchainName, utils::utils, diff --git a/src/currentprocess.rs b/src/currentprocess.rs index 323fa5e3b3..6493828f81 100644 --- a/src/currentprocess.rs +++ b/src/currentprocess.rs @@ -69,7 +69,6 @@ pub trait CurrentProcess: home::Env + CurrentDirSource + VarSource - + StdoutSource + StderrSource + StdinSource + ProcessSource @@ -83,7 +82,6 @@ pub trait CurrentProcess: CurrentProcess, CurrentDirSource, VarSource, - StdoutSource, StderrSource, StdinSource, ProcessSource @@ -123,6 +121,14 @@ impl Process { Process::TestProcess(p) => Box::new(p.args.iter().map(OsString::from)), } } + + pub(crate) fn stdout(&self) -> Box { + match self { + Process::OSProcess(_) => Box::new(io::stdout()), + #[cfg(feature = "test")] + Process::TestProcess(p) => Box::new(filesource::TestWriter(p.stdout.clone())), + } + } } /// Obtain the current instance of CurrentProcess diff --git a/src/currentprocess/filesource.rs b/src/currentprocess/filesource.rs index c5be4b180e..1415f2dc6b 100644 --- a/src/currentprocess/filesource.rs +++ b/src/currentprocess/filesource.rs @@ -60,14 +60,6 @@ pub trait Writer: Write + Send + Sync { fn terminal(&self) -> ColorableTerminal; } -// -------------- stdout ------------------------------- - -/// Stand-in for [`std::io::stdout`]. -#[enum_dispatch] -pub trait StdoutSource { - fn stdout(&self) -> Box; -} - // -------------- stderr ------------------------------- /// Stand-in for std::io::stderr. @@ -98,12 +90,6 @@ impl Writer for io::Stdout { } } -impl StdoutSource for super::OSProcess { - fn stdout(&self) -> Box { - Box::new(io::stdout()) - } -} - impl WriterLock for io::StderrLock<'_> {} impl Writer for io::Stderr { @@ -208,7 +194,7 @@ mod test_support { /// A thread-safe test file handle that pretends to be e.g. stdout. #[derive(Clone, Default)] - pub(in super::super) struct TestWriter(TestWriterInner); + pub(in super::super) struct TestWriter(pub(in super::super) TestWriterInner); impl TestWriter { pub(in super::super) fn lock(&self) -> TestWriterLock<'_> { @@ -244,12 +230,6 @@ mod test_support { } } - impl StdoutSource for TestProcess { - fn stdout(&self) -> Box { - Box::new(TestWriter(self.stdout.clone())) - } - } - impl StderrSource for TestProcess { fn stderr(&self) -> Box { Box::new(TestWriter(self.stderr.clone())) From a8c07c23fd84633127a1c1633722c5ea184d15be Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 9 Apr 2024 13:55:35 +0200 Subject: [PATCH 3/9] Simplify process access to stderr --- src/cli/log.rs | 4 +--- src/cli/rustup_mode.rs | 1 - src/currentprocess.rs | 17 +++++++++-------- src/currentprocess/filesource.rs | 20 -------------------- src/dist/component/package.rs | 2 +- 5 files changed, 11 insertions(+), 33 deletions(-) diff --git a/src/cli/log.rs b/src/cli/log.rs index 6741101696..5d18bc2f82 100644 --- a/src/cli/log.rs +++ b/src/cli/log.rs @@ -1,9 +1,7 @@ use std::fmt; use std::io::Write; -use crate::currentprocess::{ - filesource::StderrSource, process, terminalsource, varsource::VarSource, -}; +use crate::currentprocess::{process, terminalsource, varsource::VarSource}; macro_rules! warn { ( $ ( $ arg : tt ) * ) => ( $crate::cli::log::warn_fmt ( format_args ! ( $ ( $ arg ) * ) ) ) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 618316d8df..10f7cf8903 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -24,7 +24,6 @@ use crate::{ command, config::{new_toolchain_with_reason, ActiveReason, Cfg}, currentprocess::{ - filesource::StderrSource, process, terminalsource::{self, ColorableTerminal}, }, diff --git a/src/currentprocess.rs b/src/currentprocess.rs index 6493828f81..1381ee649b 100644 --- a/src/currentprocess.rs +++ b/src/currentprocess.rs @@ -66,13 +66,7 @@ use varsource::*; /// and even there we should be doing debouncing and managing update rates). #[enum_dispatch] pub trait CurrentProcess: - home::Env - + CurrentDirSource - + VarSource - + StderrSource - + StdinSource - + ProcessSource - + Debug + home::Env + CurrentDirSource + VarSource + StdinSource + ProcessSource + Debug { } @@ -82,7 +76,6 @@ pub trait CurrentProcess: CurrentProcess, CurrentDirSource, VarSource, - StderrSource, StdinSource, ProcessSource )] @@ -129,6 +122,14 @@ impl Process { Process::TestProcess(p) => Box::new(filesource::TestWriter(p.stdout.clone())), } } + + pub(crate) fn stderr(&self) -> Box { + match self { + Process::OSProcess(_) => Box::new(io::stderr()), + #[cfg(feature = "test")] + Process::TestProcess(p) => Box::new(filesource::TestWriter(p.stderr.clone())), + } + } } /// Obtain the current instance of CurrentProcess diff --git a/src/currentprocess/filesource.rs b/src/currentprocess/filesource.rs index 1415f2dc6b..1049fc4bae 100644 --- a/src/currentprocess/filesource.rs +++ b/src/currentprocess/filesource.rs @@ -60,14 +60,6 @@ pub trait Writer: Write + Send + Sync { fn terminal(&self) -> ColorableTerminal; } -// -------------- stderr ------------------------------- - -/// Stand-in for std::io::stderr. -#[enum_dispatch] -pub trait StderrSource { - fn stderr(&self) -> Box; -} - // ----------------- OS support for writers ----------------- impl WriterLock for io::StdoutLock<'_> {} @@ -110,12 +102,6 @@ impl Writer for io::Stderr { } } -impl StderrSource for super::OSProcess { - fn stderr(&self) -> Box { - Box::new(io::stderr()) - } -} - #[cfg(feature = "test")] pub(crate) use self::test_support::*; @@ -229,10 +215,4 @@ mod test_support { Ok(()) } } - - impl StderrSource for TestProcess { - fn stderr(&self) -> Box { - Box::new(TestWriter(self.stderr.clone())) - } - } } diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index b472588d60..125086c611 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -11,7 +11,7 @@ use std::path::{Path, PathBuf}; use anyhow::{anyhow, bail, Context, Result}; use tar::EntryType; -use crate::currentprocess::{filesource::StderrSource, process, varsource::VarSource}; +use crate::currentprocess::{process, varsource::VarSource}; use crate::diskio::{get_executor, CompletedIo, Executor, FileBuffer, Item, Kind, IO_CHUNK_SIZE}; use crate::dist::component::components::*; use crate::dist::component::transaction::*; From f3853f3fa293e77ba4533cc06f136cdf14f0cfe4 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 9 Apr 2024 13:58:46 +0200 Subject: [PATCH 4/9] Simplify process access to stdin --- src/cli/common.rs | 4 +--- src/currentprocess.rs | 28 +++++++++++++--------------- src/currentprocess/filesource.rs | 24 ++---------------------- 3 files changed, 16 insertions(+), 40 deletions(-) diff --git a/src/cli/common.rs b/src/cli/common.rs index ca4bb084d4..59613bca1e 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -14,9 +14,7 @@ use once_cell::sync::Lazy; use super::self_update; use crate::cli::download_tracker::DownloadTracker; -use crate::currentprocess::{ - filesource::StdinSource, process, terminalsource, varsource::VarSource, -}; +use crate::currentprocess::{process, terminalsource, varsource::VarSource}; use crate::dist::dist::{TargetTriple, ToolchainDesc}; use crate::dist::manifest::ComponentStatus; use crate::install::UpdateStatus; diff --git a/src/currentprocess.rs b/src/currentprocess.rs index 1381ee649b..f99028d061 100644 --- a/src/currentprocess.rs +++ b/src/currentprocess.rs @@ -27,7 +27,6 @@ pub mod terminalsource; pub mod varsource; use cwdsource::*; -use filesource::*; use varsource::*; /// An abstraction for the current process. @@ -65,20 +64,11 @@ use varsource::*; /// methods are in performance critical loops (except perhaps progress bars - /// and even there we should be doing debouncing and managing update rates). #[enum_dispatch] -pub trait CurrentProcess: - home::Env + CurrentDirSource + VarSource + StdinSource + ProcessSource + Debug -{ -} +pub trait CurrentProcess: home::Env + CurrentDirSource + VarSource + ProcessSource + Debug {} /// Allows concrete types for the currentprocess abstraction. #[derive(Clone, Debug)] -#[enum_dispatch( - CurrentProcess, - CurrentDirSource, - VarSource, - StdinSource, - ProcessSource -)] +#[enum_dispatch(CurrentProcess, CurrentDirSource, VarSource, ProcessSource)] pub enum Process { OSProcess(OSProcess), #[cfg(feature = "test")] @@ -115,6 +105,14 @@ impl Process { } } + pub(crate) fn stdin(&self) -> Box { + match self { + Process::OSProcess(_) => Box::new(io::stdin()), + #[cfg(feature = "test")] + Process::TestProcess(p) => Box::new(filesource::TestStdin(p.stdin.clone())), + } + } + pub(crate) fn stdout(&self) -> Box { match self { Process::OSProcess(_) => Box::new(io::stdout()), @@ -296,9 +294,9 @@ pub struct TestProcess { pub args: Vec, pub vars: HashMap, pub id: u64, - pub stdin: TestStdinInner, - pub stdout: TestWriterInner, - pub stderr: TestWriterInner, + pub stdin: filesource::TestStdinInner, + pub stdout: filesource::TestWriterInner, + pub stderr: filesource::TestWriterInner, } #[cfg(feature = "test")] diff --git a/src/currentprocess/filesource.rs b/src/currentprocess/filesource.rs index 1049fc4bae..bf46a54f4e 100644 --- a/src/currentprocess/filesource.rs +++ b/src/currentprocess/filesource.rs @@ -1,7 +1,5 @@ use std::io::{self, BufRead, Read, Result, Write}; -use enum_dispatch::enum_dispatch; - use super::terminalsource::{ColorableTerminal, StreamSelector}; use crate::currentprocess::process; @@ -14,12 +12,6 @@ pub trait Stdin { /// Stand-in for std::io::StdinLock pub trait StdinLock: Read + BufRead {} -/// Stand-in for std::io::stdin -#[enum_dispatch] -pub trait StdinSource { - fn stdin(&self) -> Box; -} - // ----------------- OS support for stdin ----------------- impl StdinLock for io::StdinLock<'_> {} @@ -34,12 +26,6 @@ impl Stdin for io::Stdin { } } -impl StdinSource for super::OSProcess { - fn stdin(&self) -> Box { - Box::new(io::stdin()) - } -} - // -------------- stdout ------------------------------- /// This is a stand-in for [`std::io::StdoutLock`] and [`std::io::StderrLock`]. @@ -112,7 +98,7 @@ mod test_support { sync::{Arc, Mutex, MutexGuard}, }; - use super::{super::TestProcess, *}; + use super::*; // ----------------------- test support for stdin ------------------ @@ -139,7 +125,7 @@ mod test_support { pub(crate) type TestStdinInner = Arc>>; - struct TestStdin(TestStdinInner); + pub struct TestStdin(pub(in super::super) TestStdinInner); impl Stdin for TestStdin { fn lock(&self) -> Box { @@ -152,12 +138,6 @@ mod test_support { } } - impl StdinSource for TestProcess { - fn stdin(&self) -> Box { - Box::new(TestStdin(self.stdin.clone())) - } - } - // ----------------------- test support for writers ------------------ pub(in super::super) struct TestWriterLock<'a> { From 77acc983f5f493c48ff8886ee6c6c1a859142728 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 9 Apr 2024 14:05:09 +0200 Subject: [PATCH 5/9] Simplify process access to pid --- src/currentprocess.rs | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/src/currentprocess.rs b/src/currentprocess.rs index f99028d061..0a414ccca5 100644 --- a/src/currentprocess.rs +++ b/src/currentprocess.rs @@ -64,11 +64,11 @@ use varsource::*; /// methods are in performance critical loops (except perhaps progress bars - /// and even there we should be doing debouncing and managing update rates). #[enum_dispatch] -pub trait CurrentProcess: home::Env + CurrentDirSource + VarSource + ProcessSource + Debug {} +pub trait CurrentProcess: home::Env + CurrentDirSource + VarSource + Debug {} /// Allows concrete types for the currentprocess abstraction. #[derive(Clone, Debug)] -#[enum_dispatch(CurrentProcess, CurrentDirSource, VarSource, ProcessSource)] +#[enum_dispatch(CurrentProcess, CurrentDirSource, VarSource)] pub enum Process { OSProcess(OSProcess), #[cfg(feature = "test")] @@ -128,6 +128,15 @@ impl Process { Process::TestProcess(p) => Box::new(filesource::TestWriter(p.stderr.clone())), } } + + #[cfg(test)] + fn id(&self) -> u64 { + match self { + Process::OSProcess(_) => std::process::id() as u64, + #[cfg(feature = "test")] + Process::TestProcess(p) => p.id, + } + } } /// Obtain the current instance of CurrentProcess @@ -247,16 +256,6 @@ thread_local! { pub(crate) static PROCESS: RefCell> = const { RefCell::new(None) }; } -// PID related things -#[enum_dispatch] -pub trait ProcessSource { - /// Returns a unique id for the process. - /// - /// Real process ids are <= u32::MAX. - /// Test process ids are > u32::MAX - fn id(&self) -> u64; -} - // ----------- real process ----------------- #[derive(Clone, Debug)] @@ -280,12 +279,6 @@ impl Default for OSProcess { } } -impl ProcessSource for OSProcess { - fn id(&self) -> u64 { - std::process::id() as u64 - } -} - // ------------ test process ---------------- #[cfg(feature = "test")] #[derive(Clone, Debug, Default)] @@ -342,13 +335,6 @@ impl TestProcess { } } -#[cfg(feature = "test")] -impl ProcessSource for TestProcess { - fn id(&self) -> u64 { - self.id - } -} - #[cfg(test)] mod tests { use std::collections::HashMap; @@ -356,7 +342,7 @@ mod tests { use rustup_macros::unit_test as test; - use super::{process, with, ProcessSource, TestProcess}; + use super::{process, with, TestProcess}; #[test] fn test_instance() { @@ -367,7 +353,7 @@ mod tests { "", ); with(proc.clone().into(), || { - assert_eq!(proc.id(), process().id(), "{:?} != {:?}", proc, process()) + assert_eq!(proc.id, process().id(), "{:?} != {:?}", proc, process()) }); } } From 3901c3216e96c8affe72530cad435f607619ed7e Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 9 Apr 2024 14:24:53 +0200 Subject: [PATCH 6/9] Remove unnecessary trait bound for home::Env --- src/currentprocess.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/currentprocess.rs b/src/currentprocess.rs index 0a414ccca5..aa7801aa37 100644 --- a/src/currentprocess.rs +++ b/src/currentprocess.rs @@ -16,7 +16,6 @@ use std::{ }; use enum_dispatch::enum_dispatch; -use home::env as home; #[cfg(feature = "test")] use rand::{thread_rng, Rng}; @@ -64,7 +63,7 @@ use varsource::*; /// methods are in performance critical loops (except perhaps progress bars - /// and even there we should be doing debouncing and managing update rates). #[enum_dispatch] -pub trait CurrentProcess: home::Env + CurrentDirSource + VarSource + Debug {} +pub trait CurrentProcess: CurrentDirSource + VarSource + Debug {} /// Allows concrete types for the currentprocess abstraction. #[derive(Clone, Debug)] From 1885938c22cb8af2d4e01a5e8f5ccd12469645e4 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 9 Apr 2024 14:34:07 +0200 Subject: [PATCH 7/9] Simplify process access to environment variables --- src/bin/rustup-init.rs | 2 +- src/cli/common.rs | 2 +- src/cli/log.rs | 2 +- src/cli/rustup_mode.rs | 2 -- src/cli/self_update.rs | 2 +- src/cli/self_update/shell.rs | 2 +- src/cli/self_update/unix.rs | 2 +- src/cli/self_update/windows.rs | 2 +- src/config.rs | 2 +- src/currentprocess.rs | 25 +++++++++++++--- src/currentprocess/homethunk.rs | 44 ++++++---------------------- src/currentprocess/terminalsource.rs | 2 +- src/currentprocess/varsource.rs | 40 ------------------------- src/diskio/mod.rs | 2 +- src/diskio/threaded.rs | 1 - src/dist/component/package.rs | 2 +- src/dist/dist.rs | 2 +- src/dist/manifestation.rs | 2 +- src/env_var.rs | 2 +- src/toolchain/toolchain.rs | 2 +- src/utils/raw.rs | 2 +- src/utils/utils.rs | 4 +-- 22 files changed, 47 insertions(+), 101 deletions(-) delete mode 100644 src/currentprocess/varsource.rs diff --git a/src/bin/rustup-init.rs b/src/bin/rustup-init.rs index b37a20125c..8e5ee411aa 100644 --- a/src/bin/rustup-init.rs +++ b/src/bin/rustup-init.rs @@ -27,7 +27,7 @@ use rustup::cli::rustup_mode; #[cfg(windows)] use rustup::cli::self_update; use rustup::cli::setup_mode; -use rustup::currentprocess::{process, varsource::VarSource, with_runtime, OSProcess}; +use rustup::currentprocess::{process, with_runtime, OSProcess}; use rustup::env_var::RUST_RECURSION_COUNT_MAX; use rustup::is_proxyable_tools; use rustup::utils::utils::{self, ExitCode}; diff --git a/src/cli/common.rs b/src/cli/common.rs index 59613bca1e..808e94daf2 100644 --- a/src/cli/common.rs +++ b/src/cli/common.rs @@ -14,7 +14,7 @@ use once_cell::sync::Lazy; use super::self_update; use crate::cli::download_tracker::DownloadTracker; -use crate::currentprocess::{process, terminalsource, varsource::VarSource}; +use crate::currentprocess::{process, terminalsource}; use crate::dist::dist::{TargetTriple, ToolchainDesc}; use crate::dist::manifest::ComponentStatus; use crate::install::UpdateStatus; diff --git a/src/cli/log.rs b/src/cli/log.rs index 5d18bc2f82..48f1c6d6a4 100644 --- a/src/cli/log.rs +++ b/src/cli/log.rs @@ -1,7 +1,7 @@ use std::fmt; use std::io::Write; -use crate::currentprocess::{process, terminalsource, varsource::VarSource}; +use crate::currentprocess::{process, terminalsource}; macro_rules! warn { ( $ ( $ arg : tt ) * ) => ( $crate::cli::log::warn_fmt ( format_args ! ( $ ( $ arg ) * ) ) ) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 10f7cf8903..6283a977ec 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1488,8 +1488,6 @@ async fn man( command: &str, toolchain: Option, ) -> Result { - use crate::currentprocess::varsource::VarSource; - let toolchain = Toolchain::from_partial(toolchain, cfg).await?; let mut path = toolchain.path().to_path_buf(); path.push("share"); diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index bd231fe4c5..cdcd6ac603 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -66,7 +66,7 @@ use crate::{ markdown::md, }, config::Cfg, - currentprocess::{process, varsource::VarSource}, + currentprocess::process, dist::dist::{self, PartialToolchainDesc, Profile, TargetTriple, ToolchainDesc}, install::UpdateStatus, toolchain::{ diff --git a/src/cli/self_update/shell.rs b/src/cli/self_update/shell.rs index 78ed7374ab..ca3a88488e 100644 --- a/src/cli/self_update/shell.rs +++ b/src/cli/self_update/shell.rs @@ -29,7 +29,7 @@ use std::path::PathBuf; use anyhow::{bail, Result}; use super::utils; -use crate::currentprocess::{process, varsource::VarSource}; +use crate::currentprocess::process; pub(crate) type Shell = Box; diff --git a/src/cli/self_update/unix.rs b/src/cli/self_update/unix.rs index c9fd59ad2a..21cda58e8b 100644 --- a/src/cli/self_update/unix.rs +++ b/src/cli/self_update/unix.rs @@ -5,7 +5,7 @@ use anyhow::{bail, Context, Result}; use super::install_bins; use super::shell; -use crate::currentprocess::{process, varsource::VarSource}; +use crate::currentprocess::process; use crate::utils::utils; use crate::utils::Notification; diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 8025f9497e..40c255ffe3 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -12,7 +12,7 @@ use super::super::errors::*; use super::common; use super::{install_bins, InstallOpts}; use crate::cli::download_tracker::DownloadTracker; -use crate::currentprocess::{process, varsource::VarSource}; +use crate::currentprocess::process; use crate::dist::dist::TargetTriple; use crate::utils::utils; use crate::utils::Notification; diff --git a/src/config.rs b/src/config.rs index 6cc28c1437..f4093063b2 100644 --- a/src/config.rs +++ b/src/config.rs @@ -13,7 +13,7 @@ use tokio_stream::StreamExt; use crate::{ cli::self_update::SelfUpdateMode, - currentprocess::{process, varsource::VarSource}, + currentprocess::process, dist::{ dist::{self, PartialToolchainDesc, Profile, ToolchainDesc}, download::DownloadCfg, diff --git a/src/currentprocess.rs b/src/currentprocess.rs index aa7801aa37..4afea95968 100644 --- a/src/currentprocess.rs +++ b/src/currentprocess.rs @@ -23,10 +23,8 @@ pub mod cwdsource; pub mod filesource; mod homethunk; pub mod terminalsource; -pub mod varsource; use cwdsource::*; -use varsource::*; /// An abstraction for the current process. /// @@ -63,11 +61,11 @@ use varsource::*; /// methods are in performance critical loops (except perhaps progress bars - /// and even there we should be doing debouncing and managing update rates). #[enum_dispatch] -pub trait CurrentProcess: CurrentDirSource + VarSource + Debug {} +pub trait CurrentProcess: CurrentDirSource + Debug {} /// Allows concrete types for the currentprocess abstraction. #[derive(Clone, Debug)] -#[enum_dispatch(CurrentProcess, CurrentDirSource, VarSource)] +#[enum_dispatch(CurrentProcess, CurrentDirSource)] pub enum Process { OSProcess(OSProcess), #[cfg(feature = "test")] @@ -88,6 +86,25 @@ impl Process { .map(String::from) } + pub fn var(&self, key: &str) -> Result { + match self { + Process::OSProcess(_) => env::var(key), + #[cfg(feature = "test")] + Process::TestProcess(p) => match p.vars.get(key) { + Some(val) => Ok(val.to_owned()), + None => Err(env::VarError::NotPresent), + }, + } + } + + pub(crate) fn var_os(&self, key: &str) -> Option { + match self { + Process::OSProcess(_) => env::var_os(key), + #[cfg(feature = "test")] + Process::TestProcess(p) => p.vars.get(key).map(OsString::from), + } + } + pub(crate) fn args(&self) -> Box + '_> { match self { Process::OSProcess(_) => Box::new(env::args()), diff --git a/src/currentprocess/homethunk.rs b/src/currentprocess/homethunk.rs index 065a28960e..758711a58b 100644 --- a/src/currentprocess/homethunk.rs +++ b/src/currentprocess/homethunk.rs @@ -5,56 +5,30 @@ use std::path::PathBuf; use home::env as home; -use super::OSProcess; -use super::Process; -#[cfg(feature = "test")] -use super::{CurrentDirSource, TestProcess, VarSource}; +use super::{CurrentDirSource, Process}; impl home::Env for Process { fn home_dir(&self) -> Option { match self { - Process::OSProcess(p) => p.home_dir(), + Process::OSProcess(_) => self.var("HOME").ok().map(|v| v.into()), #[cfg(feature = "test")] - Process::TestProcess(p) => p.home_dir(), + Process::TestProcess(_) => home::OS_ENV.home_dir(), } } + fn current_dir(&self) -> Result { match self { - Process::OSProcess(p) => home::Env::current_dir(p), + Process::OSProcess(_) => CurrentDirSource::current_dir(self), #[cfg(feature = "test")] - Process::TestProcess(p) => home::Env::current_dir(p), + Process::TestProcess(_) => home::OS_ENV.current_dir(), } } + fn var_os(&self, key: &str) -> Option { match self { - Process::OSProcess(p) => home::Env::var_os(p, key), + Process::OSProcess(_) => self.var_os(key), #[cfg(feature = "test")] - Process::TestProcess(p) => home::Env::var_os(p, key), + Process::TestProcess(_) => self.var_os(key), } } } - -#[cfg(feature = "test")] -impl home::Env for TestProcess { - fn home_dir(&self) -> Option { - self.var("HOME").ok().map(|v| v.into()) - } - fn current_dir(&self) -> Result { - CurrentDirSource::current_dir(self) - } - fn var_os(&self, key: &str) -> Option { - VarSource::var_os(self, key) - } -} - -impl home::Env for OSProcess { - fn home_dir(&self) -> Option { - home::OS_ENV.home_dir() - } - fn current_dir(&self) -> Result { - home::OS_ENV.current_dir() - } - fn var_os(&self, key: &str) -> Option { - home::OS_ENV.var_os(key) - } -} diff --git a/src/currentprocess/terminalsource.rs b/src/currentprocess/terminalsource.rs index f89612f8a4..a0092908fc 100644 --- a/src/currentprocess/terminalsource.rs +++ b/src/currentprocess/terminalsource.rs @@ -11,7 +11,7 @@ use termcolor::{ColorChoice, ColorSpec, StandardStream, StandardStreamLock, Writ #[cfg(feature = "test")] use super::filesource::{TestWriter, TestWriterLock}; -use super::{process, varsource::VarSource}; +use super::process; /// Select what stream to make a terminal on pub(super) enum StreamSelector { diff --git a/src/currentprocess/varsource.rs b/src/currentprocess/varsource.rs deleted file mode 100644 index 1c287cb17d..0000000000 --- a/src/currentprocess/varsource.rs +++ /dev/null @@ -1,40 +0,0 @@ -/// Abstracts over reading the current process environment variables as a -/// zero-cost abstraction to support threaded in-process testing. -use std::env; -use std::ffi::OsString; - -use enum_dispatch::enum_dispatch; - -#[enum_dispatch] -pub trait VarSource { - // In order to support dyn dispatch we use concrete types rather than the - // stdlib signature. - fn var(&self, key: &str) -> std::result::Result; - fn var_os(&self, key: &str) -> Option; -} - -/// Implements VarSource with `std::env::env` -impl VarSource for super::OSProcess { - fn var(&self, key: &str) -> std::result::Result { - env::var(key) - } - fn var_os(&self, key: &str) -> Option { - env::var_os(key) - } -} - -#[cfg(feature = "test")] -impl VarSource for super::TestProcess { - fn var(&self, key: &str) -> std::result::Result { - match self.var_os(key) { - None => Err(env::VarError::NotPresent), - // safe because we know this only has String in it. - Some(key) => Ok(key.into_string().unwrap()), - } - } - fn var_os(&self, key: &str) -> Option { - self.vars - .get(key) - .map(|s_ref| OsString::from(s_ref.clone())) - } -} diff --git a/src/diskio/mod.rs b/src/diskio/mod.rs index 66954c3f41..f59b7b0da1 100644 --- a/src/diskio/mod.rs +++ b/src/diskio/mod.rs @@ -66,7 +66,7 @@ use std::{fmt::Debug, fs::OpenOptions}; use anyhow::{Context, Result}; -use crate::currentprocess::{process, varsource::VarSource}; +use crate::currentprocess::process; use crate::utils::notifications::Notification; use threaded::PoolReference; diff --git a/src/diskio/threaded.rs b/src/diskio/threaded.rs index b99b7f5fe4..e2732da231 100644 --- a/src/diskio/threaded.rs +++ b/src/diskio/threaded.rs @@ -14,7 +14,6 @@ use enum_map::{enum_map, Enum, EnumMap}; use sharded_slab::pool::{OwnedRef, OwnedRefMut}; use super::{perform, CompletedIo, Executor, Item}; -use crate::currentprocess::varsource::VarSource; use crate::utils::notifications::Notification; use crate::utils::units::Unit; diff --git a/src/dist/component/package.rs b/src/dist/component/package.rs index 125086c611..40b4630baa 100644 --- a/src/dist/component/package.rs +++ b/src/dist/component/package.rs @@ -11,7 +11,7 @@ use std::path::{Path, PathBuf}; use anyhow::{anyhow, bail, Context, Result}; use tar::EntryType; -use crate::currentprocess::{process, varsource::VarSource}; +use crate::currentprocess::process; use crate::diskio::{get_executor, CompletedIo, Executor, FileBuffer, Item, Kind, IO_CHUNK_SIZE}; use crate::dist::component::components::*; use crate::dist::component::transaction::*; diff --git a/src/dist/dist.rs b/src/dist/dist.rs index 3c3359077d..e3e0711b95 100644 --- a/src/dist/dist.rs +++ b/src/dist/dist.rs @@ -14,7 +14,7 @@ use thiserror::Error as ThisError; pub(crate) use crate::dist::triple::*; use crate::{ - currentprocess::{process, varsource::VarSource}, + currentprocess::process, dist::{ download::DownloadCfg, manifest::{Component, Manifest as ManifestV2}, diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 02e5b0f872..d12c7a0b68 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -9,7 +9,7 @@ use std::path::Path; use anyhow::{anyhow, bail, Context, Result}; use tokio_retry::{strategy::FixedInterval, RetryIf}; -use crate::currentprocess::{process, varsource::VarSource}; +use crate::currentprocess::process; use crate::dist::component::{ Components, Package, TarGzPackage, TarXzPackage, TarZStdPackage, Transaction, }; diff --git a/src/env_var.rs b/src/env_var.rs index 1aabd74084..86d9035924 100644 --- a/src/env_var.rs +++ b/src/env_var.rs @@ -3,7 +3,7 @@ use std::env; use std::path::PathBuf; use std::process::Command; -use crate::currentprocess::{process, varsource::VarSource}; +use crate::currentprocess::process; pub const RUST_RECURSION_COUNT_MAX: u32 = 20; diff --git a/src/toolchain/toolchain.rs b/src/toolchain/toolchain.rs index 6458f6100f..16b7f7f4d4 100644 --- a/src/toolchain/toolchain.rs +++ b/src/toolchain/toolchain.rs @@ -15,7 +15,7 @@ use wait_timeout::ChildExt; use crate::{ config::Cfg, - currentprocess::{process, varsource::VarSource}, + currentprocess::process, dist::dist::PartialToolchainDesc, env_var, install, notifications::Notification, diff --git a/src/utils/raw.rs b/src/utils/raw.rs index 9b490c99c8..2820878997 100644 --- a/src/utils/raw.rs +++ b/src/utils/raw.rs @@ -8,7 +8,7 @@ use std::path::Path; use std::str; #[cfg(not(windows))] -use crate::currentprocess::{process, varsource::VarSource}; +use crate::currentprocess::process; pub(crate) fn ensure_dir_exists, F: FnOnce(&Path)>( path: P, diff --git a/src/utils/utils.rs b/src/utils/utils.rs index 6037442706..dabfeb30c0 100644 --- a/src/utils/utils.rs +++ b/src/utils/utils.rs @@ -11,9 +11,7 @@ use retry::{retry, OperationResult}; use sha2::Sha256; use url::Url; -use crate::currentprocess::{ - cwdsource::CurrentDirSource, home_process, process, varsource::VarSource, -}; +use crate::currentprocess::{cwdsource::CurrentDirSource, home_process, process}; use crate::errors::*; use crate::utils::notifications::Notification; use crate::utils::raw; From 7260097828387d5d4f4888c55d181b4c785a21ca Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 9 Apr 2024 14:36:59 +0200 Subject: [PATCH 8/9] Simplify process access to current_dir --- src/currentprocess.rs | 42 ++++++++++++++++++++++++++++----- src/currentprocess/cwdsource.rs | 26 -------------------- src/currentprocess/homethunk.rs | 34 -------------------------- src/utils/utils.rs | 2 +- 4 files changed, 37 insertions(+), 67 deletions(-) delete mode 100644 src/currentprocess/cwdsource.rs delete mode 100644 src/currentprocess/homethunk.rs diff --git a/src/currentprocess.rs b/src/currentprocess.rs index 4afea95968..6473aae165 100644 --- a/src/currentprocess.rs +++ b/src/currentprocess.rs @@ -19,13 +19,9 @@ use enum_dispatch::enum_dispatch; #[cfg(feature = "test")] use rand::{thread_rng, Rng}; -pub mod cwdsource; pub mod filesource; -mod homethunk; pub mod terminalsource; -use cwdsource::*; - /// An abstraction for the current process. /// /// This acts as a clonable proxy to the global state provided by some key OS @@ -61,11 +57,11 @@ use cwdsource::*; /// methods are in performance critical loops (except perhaps progress bars - /// and even there we should be doing debouncing and managing update rates). #[enum_dispatch] -pub trait CurrentProcess: CurrentDirSource + Debug {} +pub trait CurrentProcess: Debug {} /// Allows concrete types for the currentprocess abstraction. #[derive(Clone, Debug)] -#[enum_dispatch(CurrentProcess, CurrentDirSource)] +#[enum_dispatch(CurrentProcess)] pub enum Process { OSProcess(OSProcess), #[cfg(feature = "test")] @@ -145,6 +141,14 @@ impl Process { } } + pub(crate) fn current_dir(&self) -> io::Result { + match self { + Process::OSProcess(_) => env::current_dir(), + #[cfg(feature = "test")] + Process::TestProcess(p) => Ok(p.cwd.clone()), + } + } + #[cfg(test)] fn id(&self) -> u64 { match self { @@ -155,6 +159,32 @@ impl Process { } } +impl home::env::Env for Process { + fn home_dir(&self) -> Option { + match self { + Process::OSProcess(_) => self.var("HOME").ok().map(|v| v.into()), + #[cfg(feature = "test")] + Process::TestProcess(_) => home::env::OS_ENV.home_dir(), + } + } + + fn current_dir(&self) -> Result { + match self { + Process::OSProcess(_) => self.current_dir(), + #[cfg(feature = "test")] + Process::TestProcess(_) => home::env::OS_ENV.current_dir(), + } + } + + fn var_os(&self, key: &str) -> Option { + match self { + Process::OSProcess(_) => self.var_os(key), + #[cfg(feature = "test")] + Process::TestProcess(_) => self.var_os(key), + } + } +} + /// Obtain the current instance of CurrentProcess pub fn process() -> Process { home_process() diff --git a/src/currentprocess/cwdsource.rs b/src/currentprocess/cwdsource.rs deleted file mode 100644 index c44db686ca..0000000000 --- a/src/currentprocess/cwdsource.rs +++ /dev/null @@ -1,26 +0,0 @@ -/// Abstracts over reading the current process cwd as a zero-cost abstraction to -/// support threaded in-process testing. -use std::env; -use std::io; -use std::path::PathBuf; - -use enum_dispatch::enum_dispatch; - -#[enum_dispatch] -pub trait CurrentDirSource { - fn current_dir(&self) -> io::Result; -} - -/// Implements VarSource with `std::env::env` -impl CurrentDirSource for super::OSProcess { - fn current_dir(&self) -> io::Result { - env::current_dir() - } -} - -#[cfg(feature = "test")] -impl CurrentDirSource for super::TestProcess { - fn current_dir(&self) -> io::Result { - Ok(self.cwd.clone()) - } -} diff --git a/src/currentprocess/homethunk.rs b/src/currentprocess/homethunk.rs deleted file mode 100644 index 758711a58b..0000000000 --- a/src/currentprocess/homethunk.rs +++ /dev/null @@ -1,34 +0,0 @@ -/// Adapts currentprocess to the trait home::Env -use std::ffi::OsString; -use std::io; -use std::path::PathBuf; - -use home::env as home; - -use super::{CurrentDirSource, Process}; - -impl home::Env for Process { - fn home_dir(&self) -> Option { - match self { - Process::OSProcess(_) => self.var("HOME").ok().map(|v| v.into()), - #[cfg(feature = "test")] - Process::TestProcess(_) => home::OS_ENV.home_dir(), - } - } - - fn current_dir(&self) -> Result { - match self { - Process::OSProcess(_) => CurrentDirSource::current_dir(self), - #[cfg(feature = "test")] - Process::TestProcess(_) => home::OS_ENV.current_dir(), - } - } - - fn var_os(&self, key: &str) -> Option { - match self { - Process::OSProcess(_) => self.var_os(key), - #[cfg(feature = "test")] - Process::TestProcess(_) => self.var_os(key), - } - } -} diff --git a/src/utils/utils.rs b/src/utils/utils.rs index dabfeb30c0..c93ee899d2 100644 --- a/src/utils/utils.rs +++ b/src/utils/utils.rs @@ -11,7 +11,7 @@ use retry::{retry, OperationResult}; use sha2::Sha256; use url::Url; -use crate::currentprocess::{cwdsource::CurrentDirSource, home_process, process}; +use crate::currentprocess::{home_process, process}; use crate::errors::*; use crate::utils::notifications::Notification; use crate::utils::raw; From 3e387fa3e1996b3d2d30c806d2bbec29daeeb090 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 9 Apr 2024 14:52:06 +0200 Subject: [PATCH 9/9] Remove unnecessary trait abstraction --- Cargo.lock | 13 ----------- Cargo.toml | 2 -- src/bin/rustup-init.rs | 6 ++--- src/currentprocess.rs | 50 ++++++++++-------------------------------- 4 files changed, 14 insertions(+), 57 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8296509f94..ea6278e93b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -592,18 +592,6 @@ dependencies = [ "syn", ] -[[package]] -name = "enum_dispatch" -version = "0.3.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa18ce2bc66555b3218614519ac839ddb759a7d6720732f979ef8d13be147ecd" -dependencies = [ - "once_cell", - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "env_proxy" version = "0.4.1" @@ -1953,7 +1941,6 @@ dependencies = [ "download", "effective-limits", "enum-map", - "enum_dispatch", "flate2", "fs_at", "git-testament", diff --git a/Cargo.toml b/Cargo.toml index e18fde42f9..0c8f365198 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,7 +50,6 @@ clap_complete = "4" download = { path = "download", default-features = false } effective-limits = "0.5.5" enum-map = "2.5.0" -enum_dispatch.workspace = true flate2 = "1" fs_at.workspace = true git-testament = "0.2" @@ -134,7 +133,6 @@ members = ["download", "rustup-macros"] [workspace.dependencies] anyhow = "1.0.69" -enum_dispatch = "0.3.11" fs_at = "0.1.6" once_cell = "1.18.0" opentelemetry = "0.23" diff --git a/src/bin/rustup-init.rs b/src/bin/rustup-init.rs index 8e5ee411aa..7b13d00198 100644 --- a/src/bin/rustup-init.rs +++ b/src/bin/rustup-init.rs @@ -27,7 +27,7 @@ use rustup::cli::rustup_mode; #[cfg(windows)] use rustup::cli::self_update; use rustup::cli::setup_mode; -use rustup::currentprocess::{process, with_runtime, OSProcess}; +use rustup::currentprocess::{process, with_runtime, Process}; use rustup::env_var::RUST_RECURSION_COUNT_MAX; use rustup::is_proxyable_tools; use rustup::utils::utils::{self, ExitCode}; @@ -36,10 +36,10 @@ fn main() { #[cfg(windows)] pre_rustup_main_init(); - let process = OSProcess::default(); + let process = Process::os(); let mut builder = Builder::new_multi_thread(); builder.enable_all(); - with_runtime(process.into(), builder, { + with_runtime(process, builder, { async { match maybe_trace_rustup().await { Err(e) => { diff --git a/src/currentprocess.rs b/src/currentprocess.rs index 6473aae165..ba3d982e3d 100644 --- a/src/currentprocess.rs +++ b/src/currentprocess.rs @@ -15,53 +15,14 @@ use std::{ sync::{Arc, Mutex}, }; -use enum_dispatch::enum_dispatch; #[cfg(feature = "test")] use rand::{thread_rng, Rng}; pub mod filesource; pub mod terminalsource; -/// An abstraction for the current process. -/// -/// This acts as a clonable proxy to the global state provided by some key OS -/// interfaces - it is a zero cost abstraction. For the test variant it manages -/// a mutex and takes out locks to ensure consistency. -/// -/// This provides replacements env::arg*, env::var*, and the standard files -/// io::std* with traits that are customisable for tests. As a result any macros -/// or code that have non-pluggable usage of those are incompatible with -/// CurrentProcess and must not be used. That includes \[e\]println! as well as -/// third party crates. -/// -/// CurrentProcess is used via an instance in a thread local variable; when -/// making new threads, be sure to copy CurrentProcess::process() into the new -/// thread before calling any code that may need to use a CurrentProcess -/// function. -/// -/// Run some code using with: this will set the current instance, call your -/// function, then finally reset the instance at the end before returning. -/// -/// Testing level interoperation with external code that depends on environment -/// variables could be possible with a hypothetical `with_projected()` which -/// would be a zero-cost operation in real processes, but in test processes will -/// take a lock out to mutually exclude other code, then overwrite the current -/// value of std::env::vars, restoring it at the end. However, the only use for -/// that today is a test of cargo::home, which is now implemented in a separate -/// crate, so we've just deleted the test. -/// -/// A thread local is used to permit the instance to be available to the entire -/// rustup library without needing to explicitly wire this normally global state -/// everywhere; and a trait object with dyn dispatch is likewise used to avoid -/// needing to thread trait parameters across the entire code base: none of the -/// methods are in performance critical loops (except perhaps progress bars - -/// and even there we should be doing debouncing and managing update rates). -#[enum_dispatch] -pub trait CurrentProcess: Debug {} - /// Allows concrete types for the currentprocess abstraction. #[derive(Clone, Debug)] -#[enum_dispatch(CurrentProcess)] pub enum Process { OSProcess(OSProcess), #[cfg(feature = "test")] @@ -69,6 +30,10 @@ pub enum Process { } impl Process { + pub fn os() -> Self { + Self::OSProcess(OSProcess::new()) + } + pub fn name(&self) -> Option { let arg0 = match self.var("RUSTUP_FORCE_ARG0") { Ok(v) => Some(v), @@ -185,6 +150,13 @@ impl home::env::Env for Process { } } +#[cfg(feature = "test")] +impl From for Process { + fn from(p: TestProcess) -> Self { + Self::TestProcess(p) + } +} + /// Obtain the current instance of CurrentProcess pub fn process() -> Process { home_process()