From 8a1628cf890144b1475a61aee6224d6edfe4b861 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 13 Dec 2020 12:06:44 +0100 Subject: [PATCH 1/4] fix(ls): When a file doesn't exist, exit early and fails ls Currently, running $ ls doesntexist will return 0 while it should return 1 Ditto for $ ls doesntexist a --- src/uu/ls/src/ls.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index d6c7d0d7b..fba6c600b 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -168,11 +168,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ) .parse(args); - list(matches); - 0 + list(matches) } -fn list(options: getopts::Matches) { +fn list(options: getopts::Matches) -> i32 { let locs: Vec = if options.free.is_empty() { vec![String::from(".")] } else { @@ -181,8 +180,16 @@ fn list(options: getopts::Matches) { let mut files = Vec::::new(); let mut dirs = Vec::::new(); + let mut has_failed = false; for loc in locs { let p = PathBuf::from(&loc); + if !p.exists() { + show_error!("'{}': {}", &loc, "No such file or directory"); + // We found an error, the return code of ls should not be 0 + // And no need to continue the execution + has_failed = true; + continue; + } let mut dir = false; if p.is_dir() && !options.opt_present("d") { @@ -211,6 +218,11 @@ fn list(options: getopts::Matches) { } enter_directory(&dir, &options); } + if has_failed { + 1 + } else { + 0 + } } #[cfg(any(unix, target_os = "redox"))] From ba126afe549aaf10d2186758cf08939d8a5dd162 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 13 Dec 2020 12:08:54 +0100 Subject: [PATCH 2/4] fix(ls): follow the display of GNU ls --- src/uu/ls/src/ls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index fba6c600b..20100568a 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -367,7 +367,7 @@ fn display_items(items: &[PathBuf], strip: Option<&Path>, options: &getopts::Mat match md { Err(e) => { let filename = get_file_name(i, strip); - show_error!("{}: {}", filename, e); + show_error!("'{}': {}", filename, e); None } Ok(md) => Some(display_file_name(&i, strip, &md, options)), From 4068195a94e30f2ae96453e472083a53cce1cdc7 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 13 Dec 2020 12:09:14 +0100 Subject: [PATCH 3/4] test(ls): add more tests --- tests/by-util/test_ls.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 18bd66d2e..88abab58d 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -11,6 +11,40 @@ fn test_ls_ls_i() { new_ucmd!().arg("-il").succeeds(); } +#[test] +fn test_ls_non_existing() { + new_ucmd!().arg("doesntexist").fails(); +} + +#[test] +fn test_ls_files_dirs() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("a"); + at.mkdir("a/b"); + at.mkdir("a/b/c"); + at.mkdir("z"); + at.touch(&at.plus_as_string("a/a")); + at.touch(&at.plus_as_string("a/b/b")); + + scene.ucmd().arg("a").succeeds(); + scene.ucmd().arg("a/a").succeeds(); + scene.ucmd().arg("a").arg("z").succeeds(); + + let result = scene.ucmd().arg("doesntexist").fails(); + // Doesn't exist + assert!(result + .stderr + .contains("error: 'doesntexist': No such file or directory")); + + let result = scene.ucmd().arg("a").arg("doesntexist").fails(); + // One exists, the other doesn't + assert!(result + .stderr + .contains("error: 'doesntexist': No such file or directory")); + assert!(result.stdout.contains("a:")); +} + #[test] fn test_ls_ls_color() { new_ucmd!().arg("--color").succeeds(); From b687a2742ca58d37170a3a1e9e35e06e18d3b0b3 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 13 Dec 2020 12:22:31 +0100 Subject: [PATCH 4/4] test(ls): also test ls -R --- tests/by-util/test_ls.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 88abab58d..28cdaf88d 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -45,6 +45,36 @@ fn test_ls_files_dirs() { assert!(result.stdout.contains("a:")); } +#[test] +fn test_ls_recursive() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("a"); + at.mkdir("a/b"); + at.mkdir("a/b/c"); + at.mkdir("z"); + at.touch(&at.plus_as_string("a/a")); + at.touch(&at.plus_as_string("a/b/b")); + + scene.ucmd().arg("a").succeeds(); + scene.ucmd().arg("a/a").succeeds(); + let result = scene + .ucmd() + .arg("--color=never") + .arg("-R") + .arg("a") + .arg("z") + .succeeds(); + + println!("stderr = {:?}", result.stderr); + println!("stdout = {:?}", result.stdout); + if cfg!(target_os = "windows") { + assert!(result.stdout.contains("a\\b:\nb")); + } else { + assert!(result.stdout.contains("a/b:\nb")); + } +} + #[test] fn test_ls_ls_color() { new_ucmd!().arg("--color").succeeds();