From 172be3a8c62e77f50d86065a26b32e2f999f63d2 Mon Sep 17 00:00:00 2001 From: ndd7xv Date: Wed, 23 Feb 2022 22:34:34 -0500 Subject: [PATCH 1/2] install: better error messages when invalid arguments executing `install` or `install file` no longer panics and insteads returns errors similar to GNU's install when it encounters similar args --- src/uu/install/src/install.rs | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 5a106d8fc..9b72627fa 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -18,7 +18,7 @@ use filetime::{set_file_times, FileTime}; use uucore::backup_control::{self, BackupMode}; use uucore::display::Quotable; use uucore::entries::{grp2gid, usr2uid}; -use uucore::error::{FromIo, UError, UIoError, UResult}; +use uucore::error::{FromIo, UError, UIoError, UResult, UUsageError}; use uucore::format_usage; use uucore::mode::get_umask; use uucore::perms::{wrap_chown, Verbosity, VerbosityLevel}; @@ -442,11 +442,14 @@ fn is_new_file_path(path: &Path) -> bool { /// Returns a Result type with the Err variant containing the error message. /// fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { - let target: PathBuf = b - .target_dir - .clone() - .unwrap_or_else(|| paths.pop().unwrap()) - .into(); + let target: PathBuf = if let Some(path) = &b.target_dir { + path.into() + } else { + paths + .pop() + .ok_or_else(|| UUsageError::new(1, "missing file operand"))? + .into() + }; let sources = &paths.iter().map(PathBuf::from).collect::>(); @@ -468,7 +471,19 @@ fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { } if target.is_file() || is_new_file_path(&target) { - copy(&sources[0], &target, b) + copy( + sources.get(0).ok_or_else(|| { + UUsageError::new( + 1, + format!( + "missing destination file operand after '{}'", + target.to_str().unwrap() + ), + ) + })?, + &target, + b, + ) } else { Err(InstallError::InvalidTarget(target).into()) } From 47be40afaeaa4176c237a1863311b54f55b74d1d Mon Sep 17 00:00:00 2001 From: ndd7xv Date: Fri, 25 Feb 2022 21:17:43 -0500 Subject: [PATCH 2/2] install: add tests for error handling --- tests/by-util/test_install.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 23bebf224..3d78331f9 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -1100,3 +1100,27 @@ fn test_install_backup_off() { assert!(at.file_exists(file_b)); assert!(!at.file_exists(&format!("{}~", file_b))); } + +#[test] +fn test_install_missing_arguments() { + let scene = TestScenario::new(util_name!()); + + scene + .ucmd() + .fails() + .stderr_contains("install: missing file operand"); +} + +#[test] +fn test_install_missing_destination() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_1 = "source_file1"; + + at.touch(file_1); + scene.ucmd().arg(file_1).fails().stderr_contains(format!( + "install: missing destination file operand after '{}'", + file_1 + )); +}