RGBCube/uutils-coreutils
1
Fork 0
mirror of https://github.com/RGBCube/uutils-coreutils synced 2025-07-28 11:37:44 +00:00
Code Issues Activity

cp: fix cp throwing error when dest is symlink and options backup and --rem is given

Browse source
This commit is contained in:
mhead 2024-07-20 20:37:45 +05:30 • committed by Sylvestre Ledru
parent 12eacd1cf2
commit 36237a2568
2 changed files with 51 additions and 32 deletions
Download patch file Download diff file Expand all files Collapse all files

63
src/uu/cp/src/cp.rs
View file

@ -1669,34 +1669,33 @@ fn handle_existing_dest(
backup_dest(dest, &backup_path, is_dest_removed)?; backup_dest(dest, &backup_path, is_dest_removed)?;
} }
} }
match options.overwrite { if !is_dest_removed {
// FIXME: print that the file was removed if --verbose is enabled match options.overwrite {
OverwriteMode::Clobber(ClobberMode::Force) => { // FIXME: print that the file was removed if --verbose is enabled
if !is_dest_removed OverwriteMode::Clobber(ClobberMode::Force) => {
&& (is_symlink_loop(dest) || fs::metadata(dest)?.permissions().readonly()) if is_symlink_loop(dest) || fs::metadata(dest)?.permissions().readonly() {
{ fs::remove_file(dest)?;
}
}
OverwriteMode::Clobber(ClobberMode::RemoveDestination) => {
fs::remove_file(dest)?; fs::remove_file(dest)?;
} }
} OverwriteMode::Clobber(ClobberMode::Standard) => {
OverwriteMode::Clobber(ClobberMode::RemoveDestination) => { // Consider the following files:
fs::remove_file(dest)?; //
} // * `src/f` - a regular file
OverwriteMode::Clobber(ClobberMode::Standard) => { // * `src/link` - a hard link to `src/f`
// Consider the following files: // * `dest/src/f` - a different regular file
// //
// * `src/f` - a regular file // In this scenario, if we do `cp -a src/ dest/`, it is
// * `src/link` - a hard link to `src/f` // possible that the order of traversal causes `src/link`
// * `dest/src/f` - a different regular file // to get copied first (to `dest/src/link`). In that case,
// // in order to make sure `dest/src/link` is a hard link to
// In this scenario, if we do `cp -a src/ dest/`, it is // `dest/src/f` and `dest/src/f` has the contents of
// possible that the order of traversal causes `src/link` // `src/f`, we delete the existing file to allow the hard
// to get copied first (to `dest/src/link`). In that case, // linking.
// in order to make sure `dest/src/link` is a hard link to
// `dest/src/f` and `dest/src/f` has the contents of
// `src/f`, we delete the existing file to allow the hard
// linking.
if options.preserve_hard_links() if options.preserve_hard_links()
// only try to remove dest file only if the current source // only try to remove dest file only if the current source
// is hardlink to a file that is already copied // is hardlink to a file that is already copied
&& copied_files.contains_key( && copied_files.contains_key(
@ -1705,14 +1704,13 @@ fn handle_existing_dest(
options.dereference(source_in_command_line), options.dereference(source_in_command_line),
) )
.context(format!("cannot stat {}", source.quote()))?, .context(format!("cannot stat {}", source.quote()))?,
) ) {
&& !is_dest_removed fs::remove_file(dest)?;
{ }
fs::remove_file(dest)?;
} }
} _ => (),
_ => (), };
}; }
Ok(()) Ok(())
} }
@ -2044,6 +2042,7 @@ fn copy_file(
options.overwrite, options.overwrite,
OverwriteMode::Clobber(ClobberMode::RemoveDestination) OverwriteMode::Clobber(ClobberMode::RemoveDestination)
) )
&& options.backup == BackupMode::NoBackup
{ {
fs::remove_file(dest)?; fs::remove_file(dest)?;
} }

20
tests/by-util/test_cp.rs
View file

@ -5696,3 +5696,23 @@ fn test_cp_parents_absolute_path() {
let res = format!("dest{}/a/b/f", at.root_dir_resolved()); let res = format!("dest{}/a/b/f", at.root_dir_resolved());
at.file_exists(res); at.file_exists(res);
} }
// make sure that cp backup dest symlink before removing it.
#[test]
fn test_cp_with_options_backup_and_rem_when_dest_is_symlink() {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;
at.write("file", "xyz");
at.mkdir("inner_dir");
at.write("inner_dir/inner_file", "abc");
at.relative_symlink_file("inner_file", "inner_dir/sl");
scene
.ucmd()
.args(&["-b", "--rem", "file", "inner_dir/sl"])
.succeeds();
assert!(at.file_exists("inner_dir/inner_file"));
assert_eq!(at.read("inner_dir/inner_file"), "abc");
assert!(at.symlink_exists("inner_dir/sl~"));
assert!(!at.symlink_exists("inner_dir/sl"));
assert_eq!(at.read("inner_dir/sl"), "xyz");
}