Skip to content

Commit

Permalink
cp: correctly copy ancestor dirs in --parents mode
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jfinkels authored and sylvestre committed Oct 5, 2022
1 parent 3486a6e commit 47c5bba
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 1 deletion.
37 changes: 36 additions & 1 deletion src/uu/cp/src/cp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,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!("{}", context_for(p, &target.join(p)));
}
}

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));

Expand Down Expand Up @@ -1155,7 +1187,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;
Expand Down
24 changes: 24 additions & 0 deletions tests/by-util/test_cp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1990,6 +1990,30 @@ fn test_copy_same_symlink_no_dereference_dangling() {
ucmd.args(&["-d", "a", "b"]).succeeds();
}

#[test]
fn test_cp_parents_2_dirs() {
let (at, mut ucmd) = at_and_ucmd!();
at.mkdir_all("a/b/c");
at.mkdir("d");
// 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(not(windows))]
let expected_stdout = "a -> d/a\na/b -> d/a/b\n'a/b/c' -> 'd/a/b/c'\n";
#[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() {
Expand Down

0 comments on commit 47c5bba

Please sign in to comment.