From 0a3189419d9ac7d7f0b4802371eea1b51387f60a Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Sun, 14 Apr 2024 11:06:11 +0300 Subject: [PATCH 1/2] Refactor the API to use Arc and remove refcells --- examples/clone_and_publish.rs | 2 +- src/error.rs | 44 ++--- src/lib.rs | 291 ++++++++++++---------------------- tests/compile_time.rs | 10 +- tests/it/compile_failures.rs | 4 +- tests/it/env.rs | 12 +- tests/it/main.rs | 57 +++---- tests/it/tidy.rs | 4 +- 8 files changed, 166 insertions(+), 258 deletions(-) diff --git a/examples/clone_and_publish.rs b/examples/clone_and_publish.rs index e18d9e2..360529c 100644 --- a/examples/clone_and_publish.rs +++ b/examples/clone_and_publish.rs @@ -2,7 +2,7 @@ use xshell::{cmd, Shell}; fn main() -> anyhow::Result<()> { - let sh = Shell::new()?; + let mut sh = Shell::new()?; let user = "matklad"; let repo = "xshell"; diff --git a/src/error.rs b/src/error.rs index c35dcf4..b639837 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,6 +1,14 @@ -use std::{env, ffi::OsString, fmt, io, path::PathBuf, process::ExitStatus, string::FromUtf8Error}; - -use crate::{Cmd, CmdData}; +use std::{ + env, + ffi::OsString, + fmt, io, + path::{Path, PathBuf}, + process::ExitStatus, + string::FromUtf8Error, + sync::Arc, +}; + +use crate::Cmd; /// `Result` from std, with the error type defaulting to xshell's [`Error`]. pub type Result = std::result::Result; @@ -12,7 +20,7 @@ pub struct Error { /// Note: this is intentionally not public. enum ErrorKind { - CurrentDir { err: io::Error, path: Option }, + CurrentDir { err: io::Error, path: Option> }, Var { err: env::VarError, var: OsString }, ReadFile { err: io::Error, path: PathBuf }, ReadDir { err: io::Error, path: PathBuf }, @@ -21,10 +29,10 @@ enum ErrorKind { HardLink { err: io::Error, src: PathBuf, dst: PathBuf }, CreateDir { err: io::Error, path: PathBuf }, RemovePath { err: io::Error, path: PathBuf }, - CmdStatus { cmd: CmdData, status: ExitStatus }, - CmdIo { err: io::Error, cmd: CmdData }, - CmdUtf8 { err: FromUtf8Error, cmd: CmdData }, - CmdStdin { err: io::Error, cmd: CmdData }, + CmdStatus { cmd: Cmd, status: ExitStatus }, + CmdIo { err: io::Error, cmd: Cmd }, + CmdUtf8 { err: FromUtf8Error, cmd: Cmd }, + CmdStdin { err: io::Error, cmd: Cmd }, } impl From for Error { @@ -91,7 +99,7 @@ impl fmt::Display for Error { }, ErrorKind::CmdIo { err, cmd } => { if err.kind() == io::ErrorKind::NotFound { - let prog = cmd.prog.display(); + let prog = cmd.prog.as_path().display(); write!(f, "command not found: `{prog}`") } else { write!(f, "io error when running command `{cmd}`: {err}") @@ -117,7 +125,7 @@ impl std::error::Error for Error {} /// `pub(crate)` constructors, visible only in this crate. impl Error { - pub(crate) fn new_current_dir(err: io::Error, path: Option) -> Error { + pub(crate) fn new_current_dir(err: io::Error, path: Option>) -> Error { ErrorKind::CurrentDir { err, path }.into() } @@ -153,23 +161,23 @@ impl Error { ErrorKind::RemovePath { err, path }.into() } - pub(crate) fn new_cmd_status(cmd: &Cmd<'_>, status: ExitStatus) -> Error { - let cmd = cmd.data.clone(); + pub(crate) fn new_cmd_status(cmd: &Cmd, status: ExitStatus) -> Error { + let cmd = cmd.clone(); ErrorKind::CmdStatus { cmd, status }.into() } - pub(crate) fn new_cmd_io(cmd: &Cmd<'_>, err: io::Error) -> Error { - let cmd = cmd.data.clone(); + pub(crate) fn new_cmd_io(cmd: &Cmd, err: io::Error) -> Error { + let cmd = cmd.clone(); ErrorKind::CmdIo { err, cmd }.into() } - pub(crate) fn new_cmd_utf8(cmd: &Cmd<'_>, err: FromUtf8Error) -> Error { - let cmd = cmd.data.clone(); + pub(crate) fn new_cmd_utf8(cmd: &Cmd, err: FromUtf8Error) -> Error { + let cmd = cmd.clone(); ErrorKind::CmdUtf8 { err, cmd }.into() } - pub(crate) fn new_cmd_stdin(cmd: &Cmd<'_>, err: io::Error) -> Error { - let cmd = cmd.data.clone(); + pub(crate) fn new_cmd_stdin(cmd: &Cmd, err: io::Error) -> Error { + let cmd = cmd.clone(); ErrorKind::CmdStdin { err, cmd }.into() } } diff --git a/src/lib.rs b/src/lib.rs index 1ffa36d..2f8706e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -116,7 +116,7 @@ //! Next, `cd` into the folder you have just cloned: //! //! ```no_run -//! # use xshell::{Shell, cmd}; let sh = Shell::new().unwrap(); +//! # use xshell::{Shell, cmd}; let mut sh = Shell::new().unwrap(); //! # let repo = "xshell"; //! sh.change_dir(repo); //! ``` @@ -191,7 +191,7 @@ //! use xshell::{cmd, Shell}; //! //! fn main() -> anyhow::Result<()> { -//! let sh = Shell::new()?; +//! let mut sh = Shell::new()?; //! //! let user = "matklad"; //! let repo = "xshell"; @@ -279,7 +279,6 @@ mod error; use std::{ - cell::RefCell, collections::HashMap, env::{self, current_dir, VarError}, ffi::{OsStr, OsString}, @@ -288,7 +287,10 @@ use std::{ mem, path::{Path, PathBuf}, process::{Command, ExitStatus, Output, Stdio}, - sync::atomic::{AtomicUsize, Ordering}, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }, }; pub use crate::error::{Error, Result}; @@ -372,7 +374,7 @@ macro_rules! cmd { /// use xshell::{cmd, Shell}; /// /// let sh = Shell::new()?; -/// let _d = sh.push_dir("./target"); +/// let sh = sh.push_dir("./target"); /// let cwd = sh.current_dir(); /// cmd!(sh, "echo current dir is {cwd}").run()?; /// @@ -382,22 +384,20 @@ macro_rules! cmd { /// ``` #[derive(Debug, Clone)] pub struct Shell { - cwd: RefCell, - env: RefCell>, + cwd: Arc, + env: HashMap, Arc>, } impl std::panic::UnwindSafe for Shell {} impl std::panic::RefUnwindSafe for Shell {} - +/// You can use `Shell` in a tree manner by cloning the shell and modifying the `cwd`/`env` as needed. impl Shell { /// Creates a new [`Shell`]. /// /// Fails if [`std::env::current_dir`] returns an error. pub fn new() -> Result { let cwd = current_dir().map_err(|err| Error::new_current_dir(err, None))?; - let cwd = RefCell::new(cwd); - let env = RefCell::new(HashMap::new()); - Ok(Shell { cwd, env }) + Ok(Shell { cwd: cwd.into(), env: HashMap::new() }) } // region:env @@ -406,35 +406,31 @@ impl Shell { /// All relative paths are interpreted relative to this directory, rather /// than [`std::env::current_dir`]. #[doc(alias = "pwd")] - pub fn current_dir(&self) -> PathBuf { - self.cwd.borrow().clone() + pub fn current_dir(&self) -> &Path { + self.cwd.as_ref() } /// Changes the working directory for this [`Shell`]. /// /// Note that this doesn't affect [`std::env::current_dir`]. #[doc(alias = "pwd")] - pub fn change_dir>(&self, dir: P) { - self._change_dir(dir.as_ref()) + pub fn change_dir(&mut self, dir: impl AsRef) { + self._change_dir(dir.as_ref().as_ref()) } - fn _change_dir(&self, dir: &Path) { - let dir = self.path(dir); - *self.cwd.borrow_mut() = dir; + fn _change_dir(&mut self, dir: &OsStr) { + self.cwd = self.cwd.join(dir).into(); } - /// Temporary changes the working directory of this [`Shell`]. - /// - /// Returns a RAII guard which reverts the working directory to the old - /// value when dropped. + /// Returns a new [`Shell`] with the working directory set to `path`. /// /// Note that this doesn't affect [`std::env::current_dir`]. #[doc(alias = "pushd")] - pub fn push_dir>(&self, path: P) -> PushDir<'_> { + #[must_use] + pub fn push_dir(&self, path: impl AsRef) -> Self { self._push_dir(path.as_ref()) } - fn _push_dir(&self, path: &Path) -> PushDir<'_> { - let path = self.path(path); - PushDir::new(self, path) + fn _push_dir(&self, path: &Path) -> Self { + Self { cwd: self.cwd.join(path).into(), env: self.env.clone() } } /// Fetches the environmental variable `key` for this [`Shell`]. @@ -443,15 +439,18 @@ impl Shell { /// /// Environment of the [`Shell`] affects all commands spawned via this /// shell. - pub fn var>(&self, key: K) -> Result { + pub fn var(&self, key: impl AsRef) -> Result { self._var(key.as_ref()) } fn _var(&self, key: &OsStr) -> Result { match self._var_os(key) { - Some(it) => it.into_string().map_err(VarError::NotUnicode), + Some(it) => match it.to_str() { + Some(it) => Ok(it.to_string()), + None => Err(VarError::NotUnicode(key.into())), + }, None => Err(VarError::NotPresent), } - .map_err(|err| Error::new_var(err, key.to_os_string())) + .map_err(|err| Error::new_var(err, key.into())) } /// Fetches the environmental variable `key` for this [`Shell`] as @@ -459,36 +458,24 @@ impl Shell { /// /// Environment of the [`Shell`] affects all commands spawned via this /// shell. - pub fn var_os>(&self, key: K) -> Option { + pub fn var_os(&self, key: impl AsRef) -> Option> { self._var_os(key.as_ref()) } - fn _var_os(&self, key: &OsStr) -> Option { - self.env.borrow().get(key).cloned().or_else(|| env::var_os(key)) + fn _var_os(&self, key: &OsStr) -> Option> { + self.env.get(key).cloned().or_else(|| env::var_os(key).map(Into::into)) } /// Sets the value of `key` environment variable for this [`Shell`] to /// `val`. /// /// Note that this doesn't affect [`std::env::var`]. - pub fn set_var, V: AsRef>(&self, key: K, val: V) { + pub fn set_var(&mut self, key: impl AsRef, val: impl AsRef) { self._set_var(key.as_ref(), val.as_ref()) } - fn _set_var(&self, key: &OsStr, val: &OsStr) { - self.env.borrow_mut().insert(key.to_os_string(), val.to_os_string()); + fn _set_var(&mut self, key: &OsStr, val: &OsStr) { + self.env.insert(key.into(), val.into()); } - /// Temporary sets the value of `key` environment variable for this - /// [`Shell`] to `val`. - /// - /// Returns a RAII guard which restores the old environment when dropped. - /// - /// Note that this doesn't affect [`std::env::var`]. - pub fn push_env, V: AsRef>(&self, key: K, val: V) -> PushEnv<'_> { - self._push_env(key.as_ref(), val.as_ref()) - } - fn _push_env(&self, key: &OsStr, val: &OsStr) -> PushEnv<'_> { - PushEnv::new(self, key.to_os_string(), val.to_os_string()) - } // endregion:env // region:fs @@ -653,70 +640,13 @@ impl Shell { // endregion:fs /// Creates a new [`Cmd`] that executes the given `program`. - pub fn cmd>(&self, program: P) -> Cmd<'_> { + pub fn cmd(&self, program: impl AsRef) -> Cmd { // TODO: path lookup? Cmd::new(self, program.as_ref()) } fn path(&self, p: &Path) -> PathBuf { - let cd = self.cwd.borrow(); - cd.join(p) - } -} - -/// RAII guard returned from [`Shell::push_dir`]. -/// -/// Dropping `PushDir` restores the working directory of the [`Shell`] to the -/// old value. -#[derive(Debug)] -#[must_use] -pub struct PushDir<'a> { - old_cwd: PathBuf, - shell: &'a Shell, -} - -impl<'a> PushDir<'a> { - fn new(shell: &'a Shell, path: PathBuf) -> PushDir<'a> { - PushDir { old_cwd: mem::replace(&mut *shell.cwd.borrow_mut(), path), shell } - } -} - -impl Drop for PushDir<'_> { - fn drop(&mut self) { - mem::swap(&mut *self.shell.cwd.borrow_mut(), &mut self.old_cwd) - } -} - -/// RAII guard returned from [`Shell::push_env`]. -/// -/// Dropping `PushEnv` restores the old value of the environmental variable. -#[derive(Debug)] -#[must_use] -pub struct PushEnv<'a> { - key: OsString, - old_value: Option, - shell: &'a Shell, -} - -impl<'a> PushEnv<'a> { - fn new(shell: &'a Shell, key: OsString, val: OsString) -> PushEnv<'a> { - let old_value = shell.env.borrow_mut().insert(key.clone(), val); - PushEnv { shell, key, old_value } - } -} - -impl Drop for PushEnv<'_> { - fn drop(&mut self) { - let mut env = self.shell.env.borrow_mut(); - let key = mem::take(&mut self.key); - match self.old_value.take() { - Some(value) => { - env.insert(key, value); - } - None => { - env.remove(&key); - } - } + self.cwd.join(p) } } @@ -737,18 +667,13 @@ impl Drop for PushEnv<'_> { /// let cmd = cmd!(sh, "git switch {branch}").quiet().run()?; /// # Ok::<(), xshell::Error>(()) /// ``` -#[derive(Debug)] +#[derive(Debug, Clone)] #[must_use] -pub struct Cmd<'a> { - shell: &'a Shell, - data: CmdData, -} - -#[derive(Debug, Default, Clone)] -struct CmdData { +pub struct Cmd { + cwd: Arc, prog: PathBuf, args: Vec, - env_changes: Vec, + envs: HashMap, Arc>, ignore_status: bool, quiet: bool, secret: bool, @@ -757,30 +682,13 @@ struct CmdData { ignore_stderr: bool, } -// We just store a list of functions to call on the `Command` — the alternative -// would require mirroring the logic that `std::process::Command` (or rather -// `sys_common::CommandEnvs`) uses, which is moderately complex, involves -// special-casing `PATH`, and plausibly could change. -#[derive(Debug, Clone)] -enum EnvChange { - Set(OsString, OsString), - Remove(OsString), - Clear, -} - -impl fmt::Display for Cmd<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.data, f) - } -} - -impl fmt::Display for CmdData { +impl fmt::Display for Cmd { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if self.secret { return write!(f, ""); } - write!(f, "{}", self.prog.display())?; + write!(f, "{}", self.prog.as_path().display())?; for arg in &self.args { // TODO: this is potentially not copy-paste safe. let arg = arg.to_string_lossy(); @@ -794,31 +702,40 @@ impl fmt::Display for CmdData { } } -impl From> for Command { - fn from(cmd: Cmd<'_>) -> Command { +impl From for Command { + fn from(cmd: Cmd) -> Command { cmd.to_command() } } -impl<'a> Cmd<'a> { - fn new(shell: &'a Shell, prog: &Path) -> Cmd<'a> { - let mut data = CmdData::default(); - data.prog = prog.to_path_buf(); - Cmd { shell, data } +impl Cmd { + fn new(shell: &Shell, prog: impl AsRef) -> Self { + Cmd { + cwd: shell.cwd.clone(), + prog: prog.as_ref().into(), + envs: shell.env.clone(), + args: Vec::new(), + ignore_status: false, + quiet: false, + secret: false, + stdin_contents: None, + ignore_stdout: false, + ignore_stderr: false, + } } // region:builder /// Adds an argument to this commands. - pub fn arg>(mut self, arg: P) -> Cmd<'a> { + pub fn arg(mut self, arg: impl AsRef) -> Self { self._arg(arg.as_ref()); self } fn _arg(&mut self, arg: &OsStr) { - self.data.args.push(arg.to_owned()) + self.args.push(arg.to_owned()) } /// Adds all of the arguments to this command. - pub fn args(mut self, args: I) -> Cmd<'a> + pub fn args(mut self, args: I) -> Self where I: IntoIterator, I::Item: AsRef, @@ -828,128 +745,128 @@ impl<'a> Cmd<'a> { } #[doc(hidden)] - pub fn __extend_arg>(mut self, arg_fragment: P) -> Cmd<'a> { + pub fn __extend_arg(mut self, arg_fragment: impl AsRef) -> Self { self.___extend_arg(arg_fragment.as_ref()); self } fn ___extend_arg(&mut self, arg_fragment: &OsStr) { - match self.data.args.last_mut() { + match self.args.last_mut() { Some(last_arg) => last_arg.push(arg_fragment), None => { - let mut prog = mem::take(&mut self.data.prog).into_os_string(); - prog.push(arg_fragment); - self.data.prog = prog.into(); + let mut inner = mem::take(&mut self.prog).into_os_string(); + inner.push(arg_fragment); + self.prog = inner.into(); } } } /// Overrides the value of the environmental variable for this command. - pub fn env, V: AsRef>(mut self, key: K, val: V) -> Cmd<'a> { + pub fn env(mut self, key: impl AsRef, val: impl AsRef) -> Self { self._env_set(key.as_ref(), val.as_ref()); self } fn _env_set(&mut self, key: &OsStr, val: &OsStr) { - self.data.env_changes.push(EnvChange::Set(key.to_owned(), val.to_owned())); + self.envs.insert(key.into(), val.into()); } /// Overrides the values of specified environmental variables for this /// command. - pub fn envs(mut self, vars: I) -> Cmd<'a> + pub fn envs(mut self, vars: I) -> Self where I: IntoIterator, K: AsRef, V: AsRef, { - vars.into_iter().for_each(|(k, v)| self._env_set(k.as_ref(), v.as_ref())); + self.envs.extend(vars.into_iter().map(|(k, v)| (k.as_ref().into(), v.as_ref().into()))); self } /// Removes the environment variable from this command. - pub fn env_remove>(mut self, key: K) -> Cmd<'a> { + pub fn env_remove(mut self, key: impl AsRef) -> Self { self._env_remove(key.as_ref()); self } fn _env_remove(&mut self, key: &OsStr) { - self.data.env_changes.push(EnvChange::Remove(key.to_owned())); + self.envs.remove(key); } /// Removes all of the environment variables from this command. - pub fn env_clear(mut self) -> Cmd<'a> { - self.data.env_changes.push(EnvChange::Clear); + pub fn env_clear(mut self) -> Self { + self.envs.clear(); self } /// Don't return an error if command the command exits with non-zero status. /// /// By default, non-zero exit status is considered an error. - pub fn ignore_status(mut self) -> Cmd<'a> { + pub fn ignore_status(mut self) -> Self { self.set_ignore_status(true); self } /// Controls whether non-zero exit status is considered an error. pub fn set_ignore_status(&mut self, yes: bool) { - self.data.ignore_status = yes; + self.ignore_status = yes; } /// Don't echo the command itself to stderr. /// /// By default, the command itself will be printed to stderr when executed via [`Cmd::run`]. - pub fn quiet(mut self) -> Cmd<'a> { + pub fn quiet(mut self) -> Self { self.set_quiet(true); self } /// Controls whether the command itself is printed to stderr. pub fn set_quiet(&mut self, yes: bool) { - self.data.quiet = yes; + self.quiet = yes; } /// Marks the command as secret. /// /// If a command is secret, it echoes `` instead of the program and /// its arguments, even in error messages. - pub fn secret(mut self) -> Cmd<'a> { + pub fn secret(mut self) -> Self { self.set_secret(true); self } /// Controls whether the command is secret. pub fn set_secret(&mut self, yes: bool) { - self.data.secret = yes; + self.secret = yes; } /// Pass the given slice to the standard input of the spawned process. - pub fn stdin(mut self, stdin: impl AsRef<[u8]>) -> Cmd<'a> { + pub fn stdin(mut self, stdin: impl AsRef<[u8]>) -> Self { self._stdin(stdin.as_ref()); self } fn _stdin(&mut self, stdin: &[u8]) { - self.data.stdin_contents = Some(stdin.to_vec()); + self.stdin_contents = Some(stdin.to_vec()); } /// Ignores the standard output stream of the process. /// /// This is equivalent to redirecting stdout to `/dev/null`. By default, the /// stdout is inherited or captured. - pub fn ignore_stdout(mut self) -> Cmd<'a> { + pub fn ignore_stdout(mut self) -> Self { self.set_ignore_stdout(true); self } /// Controls whether the standard output is ignored. pub fn set_ignore_stdout(&mut self, yes: bool) { - self.data.ignore_stdout = yes; + self.ignore_stdout = yes; } /// Ignores the standard output stream of the process. /// /// This is equivalent redirecting stderr to `/dev/null`. By default, the /// stderr is inherited or captured. - pub fn ignore_stderr(mut self) -> Cmd<'a> { + pub fn ignore_stderr(mut self) -> Self { self.set_ignore_stderr(true); self } /// Controls whether the standard error is ignored. pub fn set_ignore_stderr(&mut self, yes: bool) { - self.data.ignore_stderr = yes; + self.ignore_stderr = yes; } // endregion:builder @@ -960,7 +877,7 @@ impl<'a> Cmd<'a> { /// are inherited, and non-zero return code is considered an error. These /// behaviors can be overridden by using various builder methods of the [`Cmd`]. pub fn run(&self) -> Result<()> { - if !self.data.quiet { + if !self.quiet { eprintln!("$ {}", self); } self.output_impl(false, false).map(|_| ()) @@ -1004,14 +921,14 @@ impl<'a> Cmd<'a> { let mut child = { let mut command = self.to_command(); - if !self.data.ignore_stdout { + if !self.ignore_stdout { command.stdout(if read_stdout { Stdio::piped() } else { Stdio::inherit() }); } - if !self.data.ignore_stderr { + if !self.ignore_stderr { command.stderr(if read_stderr { Stdio::piped() } else { Stdio::inherit() }); } - command.stdin(match &self.data.stdin_contents { + command.stdin(match &self.stdin_contents { Some(_) => Stdio::piped(), None => Stdio::null(), }); @@ -1021,9 +938,8 @@ impl<'a> Cmd<'a> { // directory does not exist. Return an appropriate error in such a // case. if matches!(err.kind(), io::ErrorKind::NotFound) { - let cwd = self.shell.cwd.borrow(); - if let Err(err) = cwd.metadata() { - return Error::new_current_dir(err, Some(cwd.clone())); + if let Err(err) = self.cwd.metadata() { + return Error::new_current_dir(err, Some(self.cwd.clone())); } } Error::new_cmd_io(self, err) @@ -1031,7 +947,7 @@ impl<'a> Cmd<'a> { }; let mut io_thread = None; - if let Some(stdin_contents) = self.data.stdin_contents.clone() { + if let Some(stdin_contents) = self.stdin_contents.clone() { let mut stdin = child.stdin.take().unwrap(); io_thread = Some(std::thread::spawn(move || { stdin.write_all(&stdin_contents)?; @@ -1049,26 +965,19 @@ impl<'a> Cmd<'a> { } fn to_command(&self) -> Command { - let mut res = Command::new(&self.data.prog); - res.current_dir(self.shell.current_dir()); - res.args(&self.data.args); + let mut res = Command::new(&self.prog); + res.current_dir(&self.cwd); + res.args(&self.args); - for (key, val) in &*self.shell.env.borrow() { + for (key, val) in &self.envs { res.env(key, val); } - for change in &self.data.env_changes { - match change { - EnvChange::Clear => res.env_clear(), - EnvChange::Remove(key) => res.env_remove(key), - EnvChange::Set(key, val) => res.env(key, val), - }; - } - if self.data.ignore_stdout { + if self.ignore_stdout { res.stdout(Stdio::null()); } - if self.data.ignore_stderr { + if self.ignore_stderr { res.stderr(Stdio::null()); } @@ -1076,7 +985,7 @@ impl<'a> Cmd<'a> { } fn check_status(&self, status: ExitStatus) -> Result<()> { - if status.success() || self.data.ignore_status { + if status.success() || self.ignore_status { return Ok(()); } Err(Error::new_cmd_status(self, status)) diff --git a/tests/compile_time.rs b/tests/compile_time.rs index ca35eef..17ec1b9 100644 --- a/tests/compile_time.rs +++ b/tests/compile_time.rs @@ -4,17 +4,17 @@ use xshell::{cmd, Shell}; #[test] fn fixed_cost_compile_times() { - let sh = Shell::new().unwrap(); + let mut sh = Shell::new().unwrap(); - let _p = sh.push_dir("tests/data"); - let baseline = compile_bench(&sh, "baseline"); + let _p = sh.change_dir("tests/data"); + let baseline = compile_bench(&mut sh, "baseline"); let _ducted = compile_bench(&sh, "ducted"); - let xshelled = compile_bench(&sh, "xshelled"); + let xshelled = compile_bench(&mut sh, "xshelled"); let ratio = (xshelled.as_millis() as f64) / (baseline.as_millis() as f64); assert!(1.0 < ratio && ratio < 10.0); fn compile_bench(sh: &Shell, name: &str) -> Duration { - let _p = sh.push_dir(name); + let sh = sh.push_dir(name); let cargo_build = cmd!(sh, "cargo build -q"); cargo_build.read().unwrap(); diff --git a/tests/it/compile_failures.rs b/tests/it/compile_failures.rs index 611af5a..1199a35 100644 --- a/tests/it/compile_failures.rs +++ b/tests/it/compile_failures.rs @@ -2,8 +2,8 @@ use xshell::{cmd, Shell}; #[track_caller] fn check(code: &str, err_msg: &str) { - let sh = Shell::new().unwrap(); - let xshell_dir = sh.current_dir(); + let mut sh = Shell::new().unwrap(); + let xshell_dir = sh.current_dir().to_owned(); let temp_dir = sh.create_temp_dir().unwrap(); sh.change_dir(temp_dir.path()); diff --git a/tests/it/env.rs b/tests/it/env.rs index 1933dfe..2c73333 100644 --- a/tests/it/env.rs +++ b/tests/it/env.rs @@ -6,7 +6,7 @@ use crate::setup; #[test] fn test_env() { - let sh = setup(); + let mut sh = setup(); let v1 = "xshell_test_123"; let v2 = "xshell_test_456"; @@ -34,8 +34,8 @@ fn test_env() { ); } - let _g1 = sh.push_env(v1, "foobar"); - let _g2 = sh.push_env(v2, "quark"); + let _g1 = sh.set_var(v1, "foobar"); + let _g2 = sh.set_var(v2, "quark"); assert_env(cmd!(sh, "xecho -$ {v1} {v2}"), &[(v1, Some("foobar")), (v2, Some("quark"))]); assert_env(cmd!(cloned_sh, "xecho -$ {v1} {v2}"), &[(v1, None), (v2, None)]); @@ -58,7 +58,7 @@ fn test_env() { #[test] fn test_env_clear() { - let sh = setup(); + let mut sh = setup(); let v1 = "xshell_test_123"; let v2 = "xshell_test_456"; @@ -79,8 +79,8 @@ fn test_env_clear() { &[(v1, Some("789")), (v2, None)], ); - let _g1 = sh.push_env(v1, "foobar"); - let _g2 = sh.push_env(v2, "quark"); + let _g1 = sh.set_var(v1, "foobar"); + let _g2 = sh.set_var(v2, "quark"); assert_env(cmd!(sh, "{xecho} -$ {v1} {v2}").env_clear(), &[(v1, None), (v2, None)]); assert_env( diff --git a/tests/it/main.rs b/tests/it/main.rs index 3214840..5b9f3d5 100644 --- a/tests/it/main.rs +++ b/tests/it/main.rs @@ -9,7 +9,7 @@ use xshell::{cmd, Shell}; fn setup() -> Shell { static ONCE: std::sync::Once = std::sync::Once::new(); - let sh = Shell::new().unwrap(); + let mut sh = Shell::new().unwrap(); let xecho_src = sh.current_dir().join("./tests/data/xecho.rs"); let target_dir = sh.current_dir().join("./target/"); @@ -87,7 +87,7 @@ fn program_concatenation() { let sh = setup(); let ho = "ho"; - let output = cmd!(sh, "xec{ho} hello").read().unwrap(); + let output = dbg!(cmd!(sh, "xec{ho} hello")).read().unwrap(); assert_eq!(output, "hello"); } @@ -245,11 +245,11 @@ fn test_push_dir() { let d1 = sh.current_dir(); { - let _p = sh.push_dir("xshell-macros"); + let sh = sh.push_dir("xshell-macros"); let d2 = sh.current_dir(); assert_eq!(d2, d1.join("xshell-macros")); { - let _p = sh.push_dir("src"); + let sh = sh.push_dir("src"); let d3 = sh.current_dir(); assert_eq!(d3, d1.join("xshell-macros/src")); } @@ -266,7 +266,7 @@ fn test_push_and_change_dir() { let d1 = sh.current_dir(); { - let _p = sh.push_dir("xshell-macros"); + let mut sh = sh.push_dir("xshell-macros"); let d2 = sh.current_dir(); assert_eq!(d2, d1.join("xshell-macros")); sh.change_dir("src"); @@ -283,26 +283,28 @@ fn push_dir_parent_dir() { let current = sh.current_dir(); let dirname = current.file_name().unwrap(); - let _d = sh.push_dir(".."); - let _d = sh.push_dir(dirname); + let sh = sh.push_dir(".."); + let sh = sh.push_dir(dirname); assert_eq!(sh.current_dir().canonicalize().unwrap(), current.canonicalize().unwrap()); } const VAR: &str = "SPICA"; #[test] -fn test_push_env() { +fn test_subshells_env() { let sh = setup(); let e1 = sh.var_os(VAR); { - let _e = sh.push_env(VAR, "1"); + let mut sh = sh.clone(); + sh.set_var(VAR, "1"); let e2 = sh.var_os(VAR); - assert_eq!(e2, Some("1".into())); + assert_eq!(e2.as_deref(), Some("1".as_ref())); { - let _e = sh.push_env(VAR, "2"); + let mut sh = sh.clone(); + let _e = sh.set_var(VAR, "2"); let e3 = sh.var_os(VAR); - assert_eq!(e3, Some("2".into())); + assert_eq!(e3.as_deref(), Some("2".as_ref())); } let e4 = sh.var_os(VAR); assert_eq!(e4, e2); @@ -311,30 +313,19 @@ fn test_push_env() { assert_eq!(e5, e1); } -#[test] -fn test_push_env_clone() { - let sh = setup(); - - assert!(sh.var_os(VAR).is_none()); - let guard = sh.push_env(VAR, "1"); - let cloned = sh.clone(); - drop(guard); - assert_eq!(sh.var_os(VAR), None); - assert_eq!(cloned.var_os(VAR), Some("1".into())); -} - #[test] fn test_push_env_and_set_var() { let sh = setup(); let e1 = sh.var_os(VAR); { - let _e = sh.push_env(VAR, "1"); + let mut sh = sh.clone(); + sh.set_var(VAR, "1"); let e2 = sh.var_os(VAR); - assert_eq!(e2, Some("1".into())); - let _e = sh.set_var(VAR, "2"); + assert_eq!(e2.as_deref(), Some("1".as_ref())); + sh.set_var(VAR, "2"); let e3 = sh.var_os(VAR); - assert_eq!(e3, Some("2".into())); + assert_eq!(e3.as_deref(), Some("2".as_ref())); } let e5 = sh.var_os(VAR); assert_eq!(e5, e1); @@ -401,7 +392,7 @@ fn test_copy_file() { #[test] fn test_exists() { - let sh = setup(); + let mut sh = setup(); let tmp = sh.create_temp_dir().unwrap(); let _d = sh.change_dir(tmp.path()); assert!(!sh.path_exists("foo.txt")); @@ -430,7 +421,7 @@ fn write_makes_directory() { #[test] fn test_remove_path() { - let sh = setup(); + let mut sh = setup(); let tempdir = sh.create_temp_dir().unwrap(); sh.change_dir(tempdir.path()); @@ -451,7 +442,7 @@ fn recovers_from_panics() { let orig = sh.current_dir(); std::panic::catch_unwind(|| { - let _p = sh.push_dir(&tempdir); + let sh = sh.push_dir(&tempdir); assert_eq!(sh.current_dir(), tempdir); std::panic::resume_unwind(Box::new(())); }) @@ -459,7 +450,7 @@ fn recovers_from_panics() { assert_eq!(sh.current_dir(), orig); { - let _p = sh.push_dir(&tempdir); + let sh = sh.push_dir(&tempdir); assert_eq!(sh.current_dir(), tempdir); } } @@ -475,7 +466,7 @@ fn string_escapes() { #[test] fn nonexistent_current_directory() { - let sh = setup(); + let mut sh = setup(); sh.change_dir("nonexistent"); let err = cmd!(sh, "ls").run().unwrap_err(); let message = err.to_string(); diff --git a/tests/it/tidy.rs b/tests/it/tidy.rs index 3d6f90d..b73d464 100644 --- a/tests/it/tidy.rs +++ b/tests/it/tidy.rs @@ -27,8 +27,8 @@ fn formatting() { #[test] fn current_version_in_changelog() { - let sh = Shell::new().unwrap(); - let _p = sh.push_dir(env!("CARGO_MANIFEST_DIR")); + let mut sh = Shell::new().unwrap(); + sh.change_dir(env!("CARGO_MANIFEST_DIR")); let changelog = sh.read_file("CHANGELOG.md").unwrap(); let current_version_header = format!("## {}", env!("CARGO_PKG_VERSION")); assert_eq!(changelog.lines().filter(|&line| line == current_version_header).count(), 1); From 1f775f4a60b112a866894a98e713c4042ab8e127 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Tue, 21 May 2024 22:26:47 +0300 Subject: [PATCH 2/2] Apply review comments --- README.md | 4 +- examples/ci.rs | 2 +- examples/clone_and_publish.rs | 4 +- src/lib.rs | 98 +++++++++++++++++++++-------------- tests/compile_time.rs | 4 +- tests/it/compile_failures.rs | 2 +- tests/it/env.rs | 8 +-- tests/it/main.rs | 54 +++++++++---------- tests/it/tidy.rs | 2 +- 9 files changed, 100 insertions(+), 78 deletions(-) diff --git a/README.md b/README.md index 8e11de1..aec78e6 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ fn main() -> anyhow::Result<()> { let user = "matklad"; let repo = "xshell"; cmd!(sh, "git clone https://github.com/{user}/{repo}.git").run()?; - sh.change_dir(repo); + sh.set_current_dir(repo); let test_args = ["-Zunstable-options", "--report-time"]; cmd!(sh, "cargo test -- {test_args...}").run()?; @@ -29,7 +29,7 @@ fn main() -> anyhow::Result<()> { cmd!(sh, "git tag {version}").run()?; - let dry_run = if sh.var("CI").is_ok() { None } else { Some("--dry-run") }; + let dry_run = if sh.env_var("CI").is_ok() { None } else { Some("--dry-run") }; cmd!(sh, "cargo publish {dry_run...}").run()?; Ok(()) diff --git a/examples/ci.rs b/examples/ci.rs index 8166447..41c9f6f 100644 --- a/examples/ci.rs +++ b/examples/ci.rs @@ -57,7 +57,7 @@ fn publish(sh: &Shell) -> Result<()> { if current_branch == "master" && !tag_exists { // Could also just use `CARGO_REGISTRY_TOKEN` environmental variable. - let token = sh.var("CRATES_IO_TOKEN").unwrap_or("DUMMY_TOKEN".to_string()); + let token = sh.env_var("CRATES_IO_TOKEN").unwrap_or("DUMMY_TOKEN".to_string()); cmd!(sh, "git tag v{version}").run()?; cmd!(sh, "cargo publish --token {token} --package xshell-macros").run()?; cmd!(sh, "cargo publish --token {token} --package xshell").run()?; diff --git a/examples/clone_and_publish.rs b/examples/clone_and_publish.rs index 360529c..7e971db 100644 --- a/examples/clone_and_publish.rs +++ b/examples/clone_and_publish.rs @@ -7,7 +7,7 @@ fn main() -> anyhow::Result<()> { let user = "matklad"; let repo = "xshell"; cmd!(sh, "git clone https://github.com/{user}/{repo}.git").run()?; - sh.change_dir(repo); + sh.set_current_dir(repo); let test_args = ["-Zunstable-options", "--report-time"]; cmd!(sh, "cargo test -- {test_args...}").run()?; @@ -21,7 +21,7 @@ fn main() -> anyhow::Result<()> { cmd!(sh, "git tag {version}").run()?; - let dry_run = if sh.var("CI").is_ok() { None } else { Some("--dry-run") }; + let dry_run = if sh.env_var("CI").is_ok() { None } else { Some("--dry-run") }; cmd!(sh, "cargo publish {dry_run...}").run()?; Ok(()) diff --git a/src/lib.rs b/src/lib.rs index 2f8706e..0975da8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -118,7 +118,7 @@ //! ```no_run //! # use xshell::{Shell, cmd}; let mut sh = Shell::new().unwrap(); //! # let repo = "xshell"; -//! sh.change_dir(repo); +//! sh.set_current_dir(repo); //! ``` //! //! Each instance of [`Shell`] has a current directory, which is independent of @@ -180,7 +180,7 @@ //! //! ```no_run //! # use xshell::{Shell, cmd}; let sh = Shell::new().unwrap(); -//! let dry_run = if sh.var("CI").is_ok() { None } else { Some("--dry-run") }; +//! let dry_run = if sh.env_var("CI").is_ok() { None } else { Some("--dry-run") }; //! cmd!(sh, "cargo publish {dry_run...}").run()?; //! # Ok::<(), xshell::Error>(()) //! ``` @@ -196,7 +196,7 @@ //! let user = "matklad"; //! let repo = "xshell"; //! cmd!(sh, "git clone https://github.com/{user}/{repo}.git").run()?; -//! sh.change_dir(repo); +//! sh.set_current_dir(repo); //! //! let test_args = ["-Zunstable-options", "--report-time"]; //! cmd!(sh, "cargo test -- {test_args...}").run()?; @@ -210,7 +210,7 @@ //! //! cmd!(sh, "git tag {version}").run()?; //! -//! let dry_run = if sh.var("CI").is_ok() { None } else { Some("--dry-run") }; +//! let dry_run = if sh.env_var("CI").is_ok() { None } else { Some("--dry-run") }; //! cmd!(sh, "cargo publish {dry_run...}").run()?; //! //! Ok(()) @@ -374,7 +374,7 @@ macro_rules! cmd { /// use xshell::{cmd, Shell}; /// /// let sh = Shell::new()?; -/// let sh = sh.push_dir("./target"); +/// let sh = sh.with_current_dir("./target"); /// let cwd = sh.current_dir(); /// cmd!(sh, "echo current dir is {cwd}").run()?; /// @@ -385,7 +385,7 @@ macro_rules! cmd { #[derive(Debug, Clone)] pub struct Shell { cwd: Arc, - env: HashMap, Arc>, + env: Arc, Arc>>, } impl std::panic::UnwindSafe for Shell {} @@ -397,7 +397,7 @@ impl Shell { /// Fails if [`std::env::current_dir`] returns an error. pub fn new() -> Result { let cwd = current_dir().map_err(|err| Error::new_current_dir(err, None))?; - Ok(Shell { cwd: cwd.into(), env: HashMap::new() }) + Ok(Shell { cwd: cwd.into(), env: Default::default() }) } // region:env @@ -413,11 +413,11 @@ impl Shell { /// Changes the working directory for this [`Shell`]. /// /// Note that this doesn't affect [`std::env::current_dir`]. - #[doc(alias = "pwd")] - pub fn change_dir(&mut self, dir: impl AsRef) { - self._change_dir(dir.as_ref().as_ref()) + #[doc(alias = "cd")] + pub fn set_current_dir(&mut self, dir: impl AsRef) { + self._set_current_dir(dir.as_ref().as_ref()) } - fn _change_dir(&mut self, dir: &OsStr) { + fn _set_current_dir(&mut self, dir: &OsStr) { self.cwd = self.cwd.join(dir).into(); } @@ -426,10 +426,10 @@ impl Shell { /// Note that this doesn't affect [`std::env::current_dir`]. #[doc(alias = "pushd")] #[must_use] - pub fn push_dir(&self, path: impl AsRef) -> Self { - self._push_dir(path.as_ref()) + pub fn with_current_dir(&self, path: impl AsRef) -> Self { + self._with_current_dir(path.as_ref()) } - fn _push_dir(&self, path: &Path) -> Self { + fn _with_current_dir(&self, path: &Path) -> Self { Self { cwd: self.cwd.join(path).into(), env: self.env.clone() } } @@ -439,11 +439,11 @@ impl Shell { /// /// Environment of the [`Shell`] affects all commands spawned via this /// shell. - pub fn var(&self, key: impl AsRef) -> Result { - self._var(key.as_ref()) + pub fn env_var(&self, key: impl AsRef) -> Result { + self._env_var(key.as_ref()) } - fn _var(&self, key: &OsStr) -> Result { - match self._var_os(key) { + fn _env_var(&self, key: &OsStr) -> Result { + match self._env_var_os(key) { Some(it) => match it.to_str() { Some(it) => Ok(it.to_string()), None => Err(VarError::NotUnicode(key.into())), @@ -458,22 +458,45 @@ impl Shell { /// /// Environment of the [`Shell`] affects all commands spawned via this /// shell. - pub fn var_os(&self, key: impl AsRef) -> Option> { - self._var_os(key.as_ref()) + pub fn env_var_os(&self, key: impl AsRef) -> Option> { + self._env_var_os(key.as_ref()) } - fn _var_os(&self, key: &OsStr) -> Option> { + fn _env_var_os(&self, key: &OsStr) -> Option> { self.env.get(key).cloned().or_else(|| env::var_os(key).map(Into::into)) } + /// Fetches the whole environment as a `(Key, Value)` iterator for this [`Shell`]. + /// + /// Returns an error if any of the variables are not utf8. + /// + /// Environment of the [`Shell`] affects all commands spawned via this + /// shell. + pub fn env_vars(&self) -> Result> { + if let Some((key, _)) = + self.env_vars_os().find(|(a, b)| a.to_str().or(b.to_str()).is_none()) + { + return Err(Error::new_var(VarError::NotUnicode(key.into()), key.into())); + } + Ok(self.env_vars_os().map(|(k, v)| (k.to_str().unwrap(), v.to_str().unwrap()))) + } + + /// Fetches the whole environment as a `(Key, Value)` iterator for this [`Shell`]. + /// + /// Environment of the [`Shell`] affects all commands spawned via this + /// shell. + pub fn env_vars_os(&self) -> impl Iterator { + self.env.iter().map(|(k, v)| (k.as_ref(), v.as_ref())) + } + /// Sets the value of `key` environment variable for this [`Shell`] to /// `val`. /// /// Note that this doesn't affect [`std::env::var`]. - pub fn set_var(&mut self, key: impl AsRef, val: impl AsRef) { - self._set_var(key.as_ref(), val.as_ref()) + pub fn set_env_var(&mut self, key: impl AsRef, val: impl AsRef) { + self._set_env_var(key.as_ref(), val.as_ref()) } - fn _set_var(&mut self, key: &OsStr, val: &OsStr) { - self.env.insert(key.into(), val.into()); + fn _set_env_var(&mut self, key: &OsStr, val: &OsStr) { + Arc::make_mut(&mut self.env).insert(key.into(), val.into()); } // endregion:env @@ -670,10 +693,9 @@ impl Shell { #[derive(Debug, Clone)] #[must_use] pub struct Cmd { - cwd: Arc, + sh: Shell, prog: PathBuf, args: Vec, - envs: HashMap, Arc>, ignore_status: bool, quiet: bool, secret: bool, @@ -709,11 +731,10 @@ impl From for Command { } impl Cmd { - fn new(shell: &Shell, prog: impl AsRef) -> Self { + fn new(sh: &Shell, prog: impl AsRef) -> Self { Cmd { - cwd: shell.cwd.clone(), + sh: sh.clone(), prog: prog.as_ref().into(), - envs: shell.env.clone(), args: Vec::new(), ignore_status: false, quiet: false, @@ -767,7 +788,7 @@ impl Cmd { } fn _env_set(&mut self, key: &OsStr, val: &OsStr) { - self.envs.insert(key.into(), val.into()); + Arc::make_mut(&mut self.sh.env).insert(key.into(), val.into()); } /// Overrides the values of specified environmental variables for this @@ -778,7 +799,8 @@ impl Cmd { K: AsRef, V: AsRef, { - self.envs.extend(vars.into_iter().map(|(k, v)| (k.as_ref().into(), v.as_ref().into()))); + Arc::make_mut(&mut self.sh.env) + .extend(vars.into_iter().map(|(k, v)| (k.as_ref().into(), v.as_ref().into()))); self } @@ -788,12 +810,12 @@ impl Cmd { self } fn _env_remove(&mut self, key: &OsStr) { - self.envs.remove(key); + Arc::make_mut(&mut self.sh.env).remove(key); } /// Removes all of the environment variables from this command. pub fn env_clear(mut self) -> Self { - self.envs.clear(); + Arc::make_mut(&mut self.sh.env).clear(); self } @@ -938,8 +960,8 @@ impl Cmd { // directory does not exist. Return an appropriate error in such a // case. if matches!(err.kind(), io::ErrorKind::NotFound) { - if let Err(err) = self.cwd.metadata() { - return Error::new_current_dir(err, Some(self.cwd.clone())); + if let Err(err) = self.sh.cwd.metadata() { + return Error::new_current_dir(err, Some(self.sh.cwd.clone())); } } Error::new_cmd_io(self, err) @@ -966,10 +988,10 @@ impl Cmd { fn to_command(&self) -> Command { let mut res = Command::new(&self.prog); - res.current_dir(&self.cwd); + res.current_dir(&self.sh.cwd); res.args(&self.args); - for (key, val) in &self.envs { + for (key, val) in &*self.sh.env { res.env(key, val); } diff --git a/tests/compile_time.rs b/tests/compile_time.rs index 17ec1b9..a90eefc 100644 --- a/tests/compile_time.rs +++ b/tests/compile_time.rs @@ -6,7 +6,7 @@ use xshell::{cmd, Shell}; fn fixed_cost_compile_times() { let mut sh = Shell::new().unwrap(); - let _p = sh.change_dir("tests/data"); + let _p = sh.set_current_dir("tests/data"); let baseline = compile_bench(&mut sh, "baseline"); let _ducted = compile_bench(&sh, "ducted"); let xshelled = compile_bench(&mut sh, "xshelled"); @@ -14,7 +14,7 @@ fn fixed_cost_compile_times() { assert!(1.0 < ratio && ratio < 10.0); fn compile_bench(sh: &Shell, name: &str) -> Duration { - let sh = sh.push_dir(name); + let sh = sh.with_current_dir(name); let cargo_build = cmd!(sh, "cargo build -q"); cargo_build.read().unwrap(); diff --git a/tests/it/compile_failures.rs b/tests/it/compile_failures.rs index 1199a35..fd9c8f7 100644 --- a/tests/it/compile_failures.rs +++ b/tests/it/compile_failures.rs @@ -5,7 +5,7 @@ fn check(code: &str, err_msg: &str) { let mut sh = Shell::new().unwrap(); let xshell_dir = sh.current_dir().to_owned(); let temp_dir = sh.create_temp_dir().unwrap(); - sh.change_dir(temp_dir.path()); + sh.set_current_dir(temp_dir.path()); let manifest = format!( r#" diff --git a/tests/it/env.rs b/tests/it/env.rs index 2c73333..9d5d2bb 100644 --- a/tests/it/env.rs +++ b/tests/it/env.rs @@ -34,8 +34,8 @@ fn test_env() { ); } - let _g1 = sh.set_var(v1, "foobar"); - let _g2 = sh.set_var(v2, "quark"); + let _g1 = sh.set_env_var(v1, "foobar"); + let _g2 = sh.set_env_var(v2, "quark"); assert_env(cmd!(sh, "xecho -$ {v1} {v2}"), &[(v1, Some("foobar")), (v2, Some("quark"))]); assert_env(cmd!(cloned_sh, "xecho -$ {v1} {v2}"), &[(v1, None), (v2, None)]); @@ -79,8 +79,8 @@ fn test_env_clear() { &[(v1, Some("789")), (v2, None)], ); - let _g1 = sh.set_var(v1, "foobar"); - let _g2 = sh.set_var(v2, "quark"); + let _g1 = sh.set_env_var(v1, "foobar"); + let _g2 = sh.set_env_var(v2, "quark"); assert_env(cmd!(sh, "{xecho} -$ {v1} {v2}").env_clear(), &[(v1, None), (v2, None)]); assert_env( diff --git a/tests/it/main.rs b/tests/it/main.rs index 5b9f3d5..1c108f1 100644 --- a/tests/it/main.rs +++ b/tests/it/main.rs @@ -20,7 +20,7 @@ fn setup() -> Shell { .unwrap_or_else(|err| panic!("failed to install binaries from mock_bin: {}", err)) }); - sh.set_var("PATH", target_dir); + sh.set_env_var("PATH", target_dir); sh } @@ -245,11 +245,11 @@ fn test_push_dir() { let d1 = sh.current_dir(); { - let sh = sh.push_dir("xshell-macros"); + let sh = sh.with_current_dir("xshell-macros"); let d2 = sh.current_dir(); assert_eq!(d2, d1.join("xshell-macros")); { - let sh = sh.push_dir("src"); + let sh = sh.with_current_dir("src"); let d3 = sh.current_dir(); assert_eq!(d3, d1.join("xshell-macros/src")); } @@ -266,10 +266,10 @@ fn test_push_and_change_dir() { let d1 = sh.current_dir(); { - let mut sh = sh.push_dir("xshell-macros"); + let mut sh = sh.with_current_dir("xshell-macros"); let d2 = sh.current_dir(); assert_eq!(d2, d1.join("xshell-macros")); - sh.change_dir("src"); + sh.set_current_dir("src"); let d3 = sh.current_dir(); assert_eq!(d3, d1.join("xshell-macros/src")); } @@ -283,8 +283,8 @@ fn push_dir_parent_dir() { let current = sh.current_dir(); let dirname = current.file_name().unwrap(); - let sh = sh.push_dir(".."); - let sh = sh.push_dir(dirname); + let sh = sh.with_current_dir(".."); + let sh = sh.with_current_dir(dirname); assert_eq!(sh.current_dir().canonicalize().unwrap(), current.canonicalize().unwrap()); } @@ -294,40 +294,40 @@ const VAR: &str = "SPICA"; fn test_subshells_env() { let sh = setup(); - let e1 = sh.var_os(VAR); + let e1 = sh.env_var_os(VAR); { let mut sh = sh.clone(); - sh.set_var(VAR, "1"); - let e2 = sh.var_os(VAR); + sh.set_env_var(VAR, "1"); + let e2 = sh.env_var_os(VAR); assert_eq!(e2.as_deref(), Some("1".as_ref())); { let mut sh = sh.clone(); - let _e = sh.set_var(VAR, "2"); - let e3 = sh.var_os(VAR); + let _e = sh.set_env_var(VAR, "2"); + let e3 = sh.env_var_os(VAR); assert_eq!(e3.as_deref(), Some("2".as_ref())); } - let e4 = sh.var_os(VAR); + let e4 = sh.env_var_os(VAR); assert_eq!(e4, e2); } - let e5 = sh.var_os(VAR); + let e5 = sh.env_var_os(VAR); assert_eq!(e5, e1); } #[test] -fn test_push_env_and_set_var() { +fn test_push_env_and_set_env_var() { let sh = setup(); - let e1 = sh.var_os(VAR); + let e1 = sh.env_var_os(VAR); { let mut sh = sh.clone(); - sh.set_var(VAR, "1"); - let e2 = sh.var_os(VAR); + sh.set_env_var(VAR, "1"); + let e2 = sh.env_var_os(VAR); assert_eq!(e2.as_deref(), Some("1".as_ref())); - sh.set_var(VAR, "2"); - let e3 = sh.var_os(VAR); + sh.set_env_var(VAR, "2"); + let e3 = sh.env_var_os(VAR); assert_eq!(e3.as_deref(), Some("2".as_ref())); } - let e5 = sh.var_os(VAR); + let e5 = sh.env_var_os(VAR); assert_eq!(e5, e1); } @@ -394,14 +394,14 @@ fn test_copy_file() { fn test_exists() { let mut sh = setup(); let tmp = sh.create_temp_dir().unwrap(); - let _d = sh.change_dir(tmp.path()); + let _d = sh.set_current_dir(tmp.path()); assert!(!sh.path_exists("foo.txt")); sh.write_file("foo.txt", "foo").unwrap(); assert!(sh.path_exists("foo.txt")); assert!(!sh.path_exists("bar")); sh.create_dir("bar").unwrap(); assert!(sh.path_exists("bar")); - let _d = sh.change_dir("bar"); + let _d = sh.set_current_dir("bar"); assert!(!sh.path_exists("quz.rs")); sh.write_file("quz.rs", "fn main () {}").unwrap(); assert!(sh.path_exists("quz.rs")); @@ -424,7 +424,7 @@ fn test_remove_path() { let mut sh = setup(); let tempdir = sh.create_temp_dir().unwrap(); - sh.change_dir(tempdir.path()); + sh.set_current_dir(tempdir.path()); sh.write_file(Path::new("a/b/c.rs"), "fn main() {}").unwrap(); assert!(tempdir.path().join("a/b/c.rs").exists()); sh.remove_path("./a").unwrap(); @@ -442,7 +442,7 @@ fn recovers_from_panics() { let orig = sh.current_dir(); std::panic::catch_unwind(|| { - let sh = sh.push_dir(&tempdir); + let sh = sh.with_current_dir(&tempdir); assert_eq!(sh.current_dir(), tempdir); std::panic::resume_unwind(Box::new(())); }) @@ -450,7 +450,7 @@ fn recovers_from_panics() { assert_eq!(sh.current_dir(), orig); { - let sh = sh.push_dir(&tempdir); + let sh = sh.with_current_dir(&tempdir); assert_eq!(sh.current_dir(), tempdir); } } @@ -467,7 +467,7 @@ fn string_escapes() { #[test] fn nonexistent_current_directory() { let mut sh = setup(); - sh.change_dir("nonexistent"); + sh.set_current_dir("nonexistent"); let err = cmd!(sh, "ls").run().unwrap_err(); let message = err.to_string(); if cfg!(unix) { diff --git a/tests/it/tidy.rs b/tests/it/tidy.rs index b73d464..6bcacf5 100644 --- a/tests/it/tidy.rs +++ b/tests/it/tidy.rs @@ -28,7 +28,7 @@ fn formatting() { #[test] fn current_version_in_changelog() { let mut sh = Shell::new().unwrap(); - sh.change_dir(env!("CARGO_MANIFEST_DIR")); + sh.set_current_dir(env!("CARGO_MANIFEST_DIR")); let changelog = sh.read_file("CHANGELOG.md").unwrap(); let current_version_header = format!("## {}", env!("CARGO_PKG_VERSION")); assert_eq!(changelog.lines().filter(|&line| line == current_version_header).count(), 1);