From 38cbada518b8953b069aea63234d9bd5c325784d Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Mon, 10 Oct 2022 10:41:43 -0400 Subject: [PATCH 1/3] cp: make test for interactive mode more specific --- tests/by-util/test_cp.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 2c7157851cb..13a7c79f14d 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -217,15 +217,18 @@ fn test_cp_target_directory_is_file() { #[test] fn test_cp_arg_interactive() { - new_ucmd!() - .arg(TEST_HELLO_WORLD_SOURCE) - .arg(TEST_HOW_ARE_YOU_SOURCE) - .arg("-i") + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("a"); + at.touch("b"); + // TODO The prompt in GNU cp is different, and it doesn't have the + // response either. + // + // See . + ucmd.args(&["-i", "a", "b"]) .pipe_in("N\n") .succeeds() .no_stdout() - .stderr_contains(format!("overwrite '{}'?", TEST_HOW_ARE_YOU_SOURCE)) - .stderr_contains("Not overwriting"); + .stderr_is("cp: overwrite 'b'? [y/N]: cp: Not overwriting 'b' at user request\n"); } #[test] From 7868e41b12b927228084a41d5786586ef7007131 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Mon, 10 Oct 2022 12:07:17 -0400 Subject: [PATCH 2/3] cp: make cp -a not fail on Windows Before this commit, `cp -a` would terminate with a non-zero status code on Windows because there are no extended attributes (xattr) to copy. However, the GNU documentation for cp states > Try to preserve SELinux security context and extended attributes > (xattr), but ignore any failure to do that and print no > corresponding diagnostic. so it seems reasonable to do nothing instead of exiting with an error in this case. --- src/uu/cp/src/cp.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index c69fe7dd02b..1fb17afd645 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1316,7 +1316,16 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu } #[cfg(not(unix))] { - return Err("XAttrs are only supported on unix.".to_string().into()); + // The documentation for GNU cp states: + // + // > Try to preserve SELinux security context and + // > extended attributes (xattr), but ignore any failure + // > to do that and print no corresponding diagnostic. + // + // so we simply do nothing here. + // + // TODO Silently ignore failures in the `#[cfg(unix)]` + // block instead of terminating immediately on errors. } } }; From ada706473e3a323b0e9550b533efb3b50418caa9 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Thu, 1 Sep 2022 22:12:16 -0400 Subject: [PATCH 3/3] cp: correctly copy ancestor dirs in --parents mode Fix a bug where `cp` failed to copy ancestor directories when using the `--parents` option. For example, before this commit: $ mkdir -p a/b/c d $ cp --parents a/b/c d $ find d d d/c After this commit $ mkdir -p a/b/c d $ cp --parents a/b/c d $ find d d d/a d/a/b d/a/b/c This commit also adds the correct messages for `--verbose` mode: $ cp -r --parents --verbose a/b/c d a -> d/a a/b -> d/a/b 'a/b/c' -> 'd/a/b/c' Fixes #3332. --- src/uu/cp/src/cp.rs | 37 ++++++++++++++++++++++++++++++++++++- tests/by-util/test_cp.rs | 25 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 1fb17afd645..79fd590915f 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1086,6 +1086,38 @@ fn copy_directory( .into()); } + // If in `--parents` mode, create all the necessary ancestor directories. + // + // For example, if the command is `cp --parents a/b/c d`, that + // means we need to copy the two ancestor directories first: + // + // a -> d/a + // a/b -> d/a/b + // + let tmp = if options.parents { + if let Some(parent) = root.parent() { + let new_target = target.join(parent); + std::fs::create_dir_all(&new_target)?; + + if options.verbose { + let mut ancestors = vec![]; + for p in parent.ancestors() { + ancestors.push(p); + } + for p in ancestors.iter().rev().skip(1) { + println!("{} -> {}", p.display(), target.join(p).display()); + } + } + + new_target + } else { + target.to_path_buf() + } + } else { + target.to_path_buf() + }; + let target = tmp.as_path(); + let current_dir = env::current_dir().unwrap_or_else(|e| crash!(1, "failed to get current directory {}", e)); @@ -1144,7 +1176,10 @@ fn copy_directory( if target.is_file() { return Err("cannot overwrite non-directory with directory".into()); } - fs::create_dir_all(local_to_target)?; + fs::create_dir_all(&local_to_target)?; + if options.verbose { + println!("{}", context_for(p.path(), &local_to_target)); + } } else if !path.is_dir() { if preserve_hard_links { let mut found_hard_link = false; diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 13a7c79f14d..25063354fc7 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1997,6 +1997,31 @@ fn test_copy_same_symlink_no_dereference_dangling() { ucmd.args(&["-d", "a", "b"]).succeeds(); } +#[cfg(not(windows))] +#[test] +fn test_cp_parents_2_dirs() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir_all("a/b/c"); + at.mkdir("d"); + #[cfg(not(windows))] + let expected_stdout = "a -> d/a\na/b -> d/a/b\n'a/b/c' -> 'd/a/b/c'\n"; + // TODO We should iron out exactly what the `--verbose` behavior + // should be on Windows. Currently, we have it printing, for + // example, + // + // a/b -> d\a/b + // + // Should the path separators all be forward slashes? All + // backslashes? + #[cfg(windows)] + let expected_stdout = "a -> d\\a\na/b -> d\\a/b\n'a/b/c' -> 'd\\a/b\\c'\n"; + ucmd.args(&["--verbose", "-a", "--parents", "a/b/c", "d"]) + .succeeds() + .no_stderr() + .stdout_is(expected_stdout); + assert!(at.dir_exists("d/a/b/c")); +} + #[test] #[ignore = "issue #3332"] fn test_cp_parents_2() {