From d9609038c6b5c679490a9dd26e0486a05ee35a64 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Sun, 9 Jul 2023 17:44:02 -0400 Subject: [PATCH 1/3] Emit more informative error message when cwd does not exist --- src/lib.rs | 12 +++++++++++- tests/it/main.rs | 11 +++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 02d6276..871f509 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1013,7 +1013,17 @@ impl<'a> Cmd<'a> { None => Stdio::null(), }); - command.spawn().map_err(|err| Error::new_cmd_io(self, err))? + command.spawn().map_err(|err| { + // Try to determine whether the command failed because the current + // directory does not exist. Return an appropriate error in such a + // case. + if matches!(err.kind(), io::ErrorKind::NotFound) { + if let Err(err) = self.shell.cwd.borrow().metadata() { + return Error::new_current_dir(err); + } + } + Error::new_cmd_io(self, err) + })? }; let mut io_thread = None; diff --git a/tests/it/main.rs b/tests/it/main.rs index 6ac8fe0..078ab37 100644 --- a/tests/it/main.rs +++ b/tests/it/main.rs @@ -460,3 +460,14 @@ fn string_escapes() { assert_eq!(cmd!(sh, "\"\"\"asdf\"\"\"").to_string(), r##""""asdf""""##); assert_eq!(cmd!(sh, "\\\\").to_string(), r#"\\"#); } + +#[test] +fn nonexistent_current_directory() { + let sh = setup(); + sh.change_dir("nonexistent"); + let err = cmd!(sh, "ls").run().unwrap_err(); + assert_eq!( + err.to_string(), + "failed to get current directory: No such file or directory (os error 2)" + ); +} From ceda10698439f8a62f89d64cb57d2b1c0b7ea6fa Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Sun, 9 Jul 2023 17:58:54 -0400 Subject: [PATCH 2/3] Fix test for Windows --- tests/it/main.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/it/main.rs b/tests/it/main.rs index 078ab37..f83f85c 100644 --- a/tests/it/main.rs +++ b/tests/it/main.rs @@ -468,6 +468,10 @@ fn nonexistent_current_directory() { let err = cmd!(sh, "ls").run().unwrap_err(); assert_eq!( err.to_string(), - "failed to get current directory: No such file or directory (os error 2)" + if cfg!(unix) { + "failed to get current directory: No such file or directory (os error 2)" + } else { + "io error when running command `ls`: The directory name is invalid. (os error 267)" + } ); } From e52c384321302f2d226551984f1db4ec65aa4612 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Sun, 9 Jul 2023 20:25:33 -0400 Subject: [PATCH 3/3] Include directory in error message --- src/error.rs | 12 ++++++++---- src/lib.rs | 7 ++++--- tests/it/main.rs | 15 ++++++++------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/error.rs b/src/error.rs index 3e0f5dc..c35dcf4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -12,7 +12,7 @@ pub struct Error { /// Note: this is intentionally not public. enum ErrorKind { - CurrentDir { err: io::Error }, + CurrentDir { err: io::Error, path: Option }, Var { err: env::VarError, var: OsString }, ReadFile { err: io::Error, path: PathBuf }, ReadDir { err: io::Error, path: PathBuf }, @@ -37,7 +37,11 @@ impl From for Error { impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match &*self.kind { - ErrorKind::CurrentDir { err } => write!(f, "failed to get current directory: {err}"), + ErrorKind::CurrentDir { err, path } => { + let suffix = + path.as_ref().map_or(String::new(), |path| format!(" `{}`", path.display())); + write!(f, "failed to get current directory{suffix}: {err}") + } ErrorKind::Var { err, var } => { let var = var.to_string_lossy(); write!(f, "failed to get environment variable `{var}`: {err}") @@ -113,8 +117,8 @@ 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) -> Error { - ErrorKind::CurrentDir { err }.into() + pub(crate) fn new_current_dir(err: io::Error, path: Option) -> Error { + ErrorKind::CurrentDir { err, path }.into() } pub(crate) fn new_var(err: env::VarError, var: OsString) -> Error { diff --git a/src/lib.rs b/src/lib.rs index 871f509..71be097 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -391,7 +391,7 @@ impl Shell { /// /// Fails if [`std::env::current_dir`] returns an error. pub fn new() -> Result { - let cwd = current_dir().map_err(Error::new_current_dir)?; + 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 }) @@ -1018,8 +1018,9 @@ impl<'a> Cmd<'a> { // directory does not exist. Return an appropriate error in such a // case. if matches!(err.kind(), io::ErrorKind::NotFound) { - if let Err(err) = self.shell.cwd.borrow().metadata() { - return Error::new_current_dir(err); + let cwd = self.shell.cwd.borrow(); + if let Err(err) = cwd.metadata() { + return Error::new_current_dir(err, Some(cwd.clone())); } } Error::new_cmd_io(self, err) diff --git a/tests/it/main.rs b/tests/it/main.rs index f83f85c..81f341d 100644 --- a/tests/it/main.rs +++ b/tests/it/main.rs @@ -466,12 +466,13 @@ fn nonexistent_current_directory() { let sh = setup(); sh.change_dir("nonexistent"); let err = cmd!(sh, "ls").run().unwrap_err(); - assert_eq!( - err.to_string(), - if cfg!(unix) { - "failed to get current directory: No such file or directory (os error 2)" - } else { + if cfg!(unix) { + assert!(err.to_string().starts_with("failed to get current directory")); + assert!(err.to_string().ends_with("No such file or directory (os error 2)")); + } else { + assert_eq!( + err.to_string(), "io error when running command `ls`: The directory name is invalid. (os error 267)" - } - ); + ); + } }