-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cp: correctly copy ancestor dirs in --parents mode #3894
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ancestors are printed without quotes, so using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably a good idea to add that info as a comment in the code! |
||
} | ||
} | ||
|
||
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; | ||
|
@@ -1316,7 +1351,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. | ||
} | ||
} | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 <https://github.com/uutils/coreutils/issues/4023>. | ||
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] | ||
|
@@ -1994,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"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two tests below here that are currently ignored. They are marked as "issue #3332", but they are actually covering use cases that are slightly different than the one originally reported in that issue. Because they are slightly different use cases (and completely different code paths), I decided not to try to resolve them in this pull request. |
||
fn test_cp_parents_2() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this work?