From 29655818478e901aa8807040d8834c90250ce150 Mon Sep 17 00:00:00 2001 From: Elijah Sink Date: Mon, 20 Jun 2022 14:26:46 -0500 Subject: [PATCH 1/3] tests/install: add failing test for #3650 --- tests/by-util/test_install.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index dca04ac56..ff8f3c18c 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -1197,12 +1197,20 @@ fn test_install_dir_req_verbose() { .succeeds() .stdout_contains("install: creating directory 'sub3'\ninstall: creating directory 'sub3/a'\ninstall: creating directory 'sub3/a/b'\ninstall: creating directory 'sub3/a/b/c'\n'source_file1' -> 'sub3/a/b/c/file'"); + scene + .ucmd() + .arg("-t sub4/a") + .arg("-Dv") + .arg(file_1) + .succeeds() + .stdout_contains("install: creating directory 'sub4'\ninstall: creating directory 'sub4/a'\n'source_file1' -> 'sub4/a/source_file1'"); + at.mkdir(dest_dir); scene .ucmd() .arg("-Dv") .arg(file_1) - .arg("sub4/a/b/c/file") + .arg("sub5/a/b/c/file") .succeeds() - .stdout_contains("install: creating directory 'sub4/a'\ninstall: creating directory 'sub4/a/b'\ninstall: creating directory 'sub4/a/b/c'\n'source_file1' -> 'sub4/a/b/c/file'"); + .stdout_contains("install: creating directory 'sub5/a'\ninstall: creating directory 'sub5/a/b'\ninstall: creating directory 'sub5/a/b/c'\n'source_file1' -> 'sub5/a/b/c/file'"); } From 81c45cde4764958844c23afe9f1a66743a714a77 Mon Sep 17 00:00:00 2001 From: Elijah Sink Date: Mon, 20 Jun 2022 17:31:10 -0500 Subject: [PATCH 2/3] tests/install: fix mistakes when adding previous --- tests/by-util/test_install.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index ff8f3c18c..d7ef32368 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -1187,7 +1187,6 @@ fn test_install_dir_req_verbose() { let at = &scene.fixtures; let file_1 = "source_file1"; - let dest_dir = "sub4"; at.touch(file_1); scene .ucmd() @@ -1199,13 +1198,14 @@ fn test_install_dir_req_verbose() { scene .ucmd() - .arg("-t sub4/a") + .arg("-t") + .arg("sub4/a") .arg("-Dv") .arg(file_1) .succeeds() .stdout_contains("install: creating directory 'sub4'\ninstall: creating directory 'sub4/a'\n'source_file1' -> 'sub4/a/source_file1'"); - at.mkdir(dest_dir); + at.mkdir("sub5"); scene .ucmd() .arg("-Dv") From 2b70ccd61bc29bbd9e1fbc7b853122ed50e1da29 Mon Sep 17 00:00:00 2001 From: Elijah Sink Date: Mon, 20 Jun 2022 17:32:28 -0500 Subject: [PATCH 3/3] install: handle when both -t and -D used together --- src/uu/install/src/install.rs | 57 +++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 6750560d2..e89c0e584 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -471,13 +471,19 @@ fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { if sources.len() > 1 || (target.exists() && target.is_dir()) { copy_files_into_dir(sources, &target, b) } else { - if let Some(parent) = target.parent() { - if !parent.exists() && b.create_leading { + // if -t is used in combination with -D, create whole target because it does not include filename + let to_create: Option<&Path> = if b.target_dir.is_some() && b.create_leading { + Some(target.as_path()) + } else { + target.parent() + }; + + if let Some(to_create) = to_create { + if !to_create.exists() && b.create_leading { if b.verbose { let mut result = PathBuf::new(); - // When creating directories with -Dv, show directory creations step - // by step - for part in parent.components() { + // When creating directories with -Dv, show directory creations step by step + for part in to_create.components() { result.push(part.as_os_str()); if !Path::new(part.as_os_str()).is_dir() { // Don't display when the directory already exists @@ -486,32 +492,39 @@ fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { } } - if let Err(e) = fs::create_dir_all(parent) { - return Err(InstallError::CreateDirFailed(parent.to_path_buf(), e).into()); + if let Err(e) = fs::create_dir_all(to_create) { + return Err(InstallError::CreateDirFailed(to_create.to_path_buf(), e).into()); } // Silent the warning as we want to the error message #[allow(clippy::question_mark)] - if mode::chmod(parent, b.mode()).is_err() { - return Err(InstallError::ChmodFailed(parent.to_path_buf()).into()); + if mode::chmod(to_create, b.mode()).is_err() { + return Err(InstallError::ChmodFailed(to_create.to_path_buf()).into()); } } } - if target.is_file() || is_new_file_path(&target) { - copy( - sources.get(0).ok_or_else(|| { - UUsageError::new( - 1, - format!( - "missing destination file operand after '{}'", - target.to_str().unwrap() - ), - ) - })?, - &target, - b, + let source = sources.first().ok_or_else(|| { + UUsageError::new( + 1, + format!( + "missing destination file operand after '{}'", + target.to_str().unwrap() + ), ) + })?; + + // If the -D flag was passed (target does not include filename), + // we need to add the source name to the target_dir + // because `copy` expects `to` to be a file, not a directory + let target = if target.is_dir() && b.create_leading { + target.join(source) + } else { + target // already includes dest filename + }; + + if target.is_file() || is_new_file_path(&target) { + copy(source, &target, b) } else { Err(InstallError::InvalidTarget(target).into()) }