From 2628f3ed60f82c31c1218dfd5ddc6c2189a1bee0 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 2 Apr 2022 11:06:19 +0200 Subject: [PATCH 1/4] install: support of `-d dir/.` to match GNU's --- src/uu/install/src/install.rs | 16 +++++++++++++--- tests/by-util/test_install.rs | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 28d1aa702..f50b7e81b 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -395,6 +395,16 @@ fn directory(paths: &[String], b: &Behavior) -> UResult<()> { for path in paths.iter().map(Path::new) { // if the path already exist, don't try to create it again if !path.exists() { + // Special case to match GNU's behavior: + // install -d foo/. should work and just create foo/ + // std::fs::create_dir("foo/."); fails in pure Rust + // See also mkdir.rs for another occurrence of this + let path_to_create = if path.to_string_lossy().ends_with("/.") { + // Do a simple dance to strip the "/." + Path::new(path).components().collect::() + } else { + path.to_path_buf() + }; // Differently than the primary functionality // (MainFunction::Standard), the directory functionality should // create all ancestors (or components) of a directory @@ -404,15 +414,15 @@ fn directory(paths: &[String], b: &Behavior) -> UResult<()> { // target directory. All created ancestor directories will have // the default mode. Hence it is safe to use fs::create_dir_all // and then only modify the target's dir mode. - if let Err(e) = - fs::create_dir_all(path).map_err_context(|| path.maybe_quote().to_string()) + if let Err(e) = fs::create_dir_all(path_to_create.as_path()) + .map_err_context(|| path_to_create.as_path().maybe_quote().to_string()) { show!(e); continue; } if b.verbose { - println!("creating directory {}", path.quote()); + println!("creating directory {}", path_to_create.quote()); } } diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index c4ed0d617..1cbbdfe1c 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -1124,3 +1124,27 @@ fn test_install_missing_destination() { file_1 )); } + +#[test] +fn test_install_dir_dot() { + // To match tests/install/d-slashdot.sh + let scene = TestScenario::new(util_name!()); + + scene.ucmd().arg("-d").arg("dir1/.").succeeds(); + scene.ucmd().arg("-d").arg("dir2/..").succeeds(); + // Tests that we don't have dir3/. in the output + // but only 'dir3' + scene + .ucmd() + .arg("-d") + .arg("dir3/.") + .arg("-v") + .succeeds() + .stdout_contains("creating directory 'dir3'"); + + let at = &scene.fixtures; + + assert!(at.dir_exists("dir1")); + assert!(at.dir_exists("dir2")); + assert!(at.dir_exists("dir3")); +} From 845b2294e1ec8f09a4022a63d895eecffa7607e3 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 2 Apr 2022 13:10:28 +0200 Subject: [PATCH 2/4] create a function dir_strip_dot_for_creation to manage the /. issue --- src/uu/install/src/install.rs | 8 ++------ src/uucore/src/lib/features/fs.rs | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index f50b7e81b..330124467 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -20,6 +20,7 @@ use uucore::display::Quotable; use uucore::entries::{grp2gid, usr2uid}; use uucore::error::{FromIo, UError, UIoError, UResult, UUsageError}; use uucore::format_usage; +use uucore::fs::dir_strip_dot_for_creation; use uucore::mode::get_umask; use uucore::perms::{wrap_chown, Verbosity, VerbosityLevel}; @@ -399,12 +400,7 @@ fn directory(paths: &[String], b: &Behavior) -> UResult<()> { // install -d foo/. should work and just create foo/ // std::fs::create_dir("foo/."); fails in pure Rust // See also mkdir.rs for another occurrence of this - let path_to_create = if path.to_string_lossy().ends_with("/.") { - // Do a simple dance to strip the "/." - Path::new(path).components().collect::() - } else { - path.to_path_buf() - }; + let path_to_create = dir_strip_dot_for_creation(path.to_path_buf()); // Differently than the primary functionality // (MainFunction::Standard), the directory functionality should // create all ancestors (or components) of a directory diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index f5295f17f..1b7fb24b3 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -470,6 +470,20 @@ pub fn display_permissions_unix(mode: mode_t, display_file_type: bool) -> String result } +// For some programs like install or mkdir, dir/. can be provided +// Special case to match GNU's behavior: +// install -d foo/. should work and just create foo/ +// std::fs::create_dir("foo/."); fails in pure Rust +// See also mkdir.rs for another occurrence of this +pub fn dir_strip_dot_for_creation(path: PathBuf) -> PathBuf { + if path.to_string_lossy().ends_with("/.") { + // Do a simple dance to strip the "/." + Path::new(&path).components().collect::() + } else { + path.to_path_buf() + } +} + #[cfg(test)] mod tests { // Note this useful idiom: importing names from outer (for mod tests) scope. From c00a277448d4527260403e5b75f320345594a289 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 2 Apr 2022 13:14:09 +0200 Subject: [PATCH 3/4] mkdir: also use the dir_strip_dot_for_creation function --- src/uu/install/src/install.rs | 2 +- src/uu/mkdir/src/mkdir.rs | 7 +++---- src/uucore/src/lib/features/fs.rs | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 330124467..e3154aa51 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -400,7 +400,7 @@ fn directory(paths: &[String], b: &Behavior) -> UResult<()> { // install -d foo/. should work and just create foo/ // std::fs::create_dir("foo/."); fails in pure Rust // See also mkdir.rs for another occurrence of this - let path_to_create = dir_strip_dot_for_creation(path.to_path_buf()); + let path_to_create = dir_strip_dot_for_creation(path); // Differently than the primary functionality // (MainFunction::Standard), the directory functionality should // create all ancestors (or components) of a directory diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs index 35078d296..7c8d4e413 100644 --- a/src/uu/mkdir/src/mkdir.rs +++ b/src/uu/mkdir/src/mkdir.rs @@ -12,12 +12,12 @@ extern crate uucore; use clap::{crate_version, Arg, ArgMatches, Command, OsValues}; use std::path::{Path, PathBuf}; -use uucore::display::Quotable; #[cfg(not(windows))] use uucore::error::FromIo; use uucore::error::{UResult, USimpleError}; #[cfg(not(windows))] use uucore::mode; +use uucore::{display::Quotable, fs::dir_strip_dot_for_creation}; use uucore::{format_usage, InvalidEncodingHandling}; static DEFAULT_PERM: u32 = 0o755; @@ -146,9 +146,8 @@ fn exec(dirs: OsValues, recursive: bool, mode: u32, verbose: bool) -> UResult<() // Special case to match GNU's behavior: // mkdir -p foo/. should work and just create foo/ // std::fs::create_dir("foo/."); fails in pure Rust - let path = if recursive && dir.to_string_lossy().ends_with("/.") { - // Do a simple dance to strip the "/." - Path::new(dir).components().collect::() + let path = if recursive { + dir_strip_dot_for_creation(&PathBuf::from(dir)) } else { // Normal case PathBuf::from(dir) diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 1b7fb24b3..5cd6d253d 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -475,7 +475,7 @@ pub fn display_permissions_unix(mode: mode_t, display_file_type: bool) -> String // install -d foo/. should work and just create foo/ // std::fs::create_dir("foo/."); fails in pure Rust // See also mkdir.rs for another occurrence of this -pub fn dir_strip_dot_for_creation(path: PathBuf) -> PathBuf { +pub fn dir_strip_dot_for_creation(path: &Path) -> PathBuf { if path.to_string_lossy().ends_with("/.") { // Do a simple dance to strip the "/." Path::new(&path).components().collect::() From 74a348161ee232de89adeaaa8a9e6b5489f8a123 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 2 Apr 2022 14:40:12 +0200 Subject: [PATCH 4/4] install: add tests to test with multiple directories to please @calixteman --- tests/by-util/test_install.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 1cbbdfe1c..0c775d145 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -1141,10 +1141,26 @@ fn test_install_dir_dot() { .arg("-v") .succeeds() .stdout_contains("creating directory 'dir3'"); + scene + .ucmd() + .arg("-d") + .arg("dir4/./cal") + .arg("-v") + .succeeds() + .stdout_contains("creating directory 'dir4/./cal'"); + scene + .ucmd() + .arg("-d") + .arg("dir5/./cali/.") + .arg("-v") + .succeeds() + .stdout_contains("creating directory 'dir5/cali'"); let at = &scene.fixtures; assert!(at.dir_exists("dir1")); assert!(at.dir_exists("dir2")); assert!(at.dir_exists("dir3")); + assert!(at.dir_exists("dir4/cal")); + assert!(at.dir_exists("dir5/cali")); }