From 84a741fe91b2395159850649db5b17d99395cb6a Mon Sep 17 00:00:00 2001 From: Niyaz Nigmatullin Date: Fri, 23 Sep 2022 20:31:24 +0300 Subject: [PATCH] cp: add `-H` option, add tests, fix test --- src/uu/cp/src/cp.rs | 104 +++++++++++++++++++++++---------------- tests/by-util/test_cp.rs | 70 ++++++++++++++++++++++++-- tests/common/util.rs | 9 ++++ 3 files changed, 137 insertions(+), 46 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index e82399b42..be4bab1ac 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -207,6 +207,7 @@ pub struct Options { attributes_only: bool, backup: BackupMode, copy_contents: bool, + cli_dereference: bool, copy_mode: CopyMode, dereference: bool, no_target_dir: bool, @@ -760,6 +761,7 @@ impl Options { let options = Self { attributes_only: matches.contains_id(options::ATTRIBUTES_ONLY), copy_contents: matches.contains_id(options::COPY_CONTENTS), + cli_dereference: matches.contains_id(options::CLI_SYMBOLIC_LINKS), copy_mode: CopyMode::from_matches(matches), // No dereference is set with -p, -d and --archive dereference: !(matches.contains_id(options::NO_DEREFERENCE) @@ -823,6 +825,10 @@ impl Options { Ok(options) } + + fn dereference(&self, in_command_line: bool) -> bool { + self.dereference || (in_command_line && self.cli_dereference) + } } impl TargetType { @@ -1017,11 +1023,11 @@ fn copy_source( let source_path = Path::new(&source); if source_path.is_dir() { // Copy as directory - copy_directory(source, target, options, symlinked_files) + copy_directory(source, target, options, symlinked_files, true) } else { // Copy as file let dest = construct_dest_path(source_path, target, target_type, options)?; - copy_file(source_path, dest.as_path(), options, symlinked_files) + copy_file(source_path, dest.as_path(), options, symlinked_files, true) } } @@ -1056,14 +1062,21 @@ fn copy_directory( target: &TargetSlice, options: &Options, symlinked_files: &mut HashSet, + source_in_command_line: bool, ) -> CopyResult<()> { if !options.recursive { return Err(format!("omitting directory {}", root.quote()).into()); } // if no-dereference is enabled and this is a symlink, copy it as a file - if !options.dereference && is_symlink(root) { - return copy_file(root, target, options, symlinked_files); + if !options.dereference(source_in_command_line) && root.is_symlink() { + return copy_file( + root, + target, + options, + symlinked_files, + source_in_command_line, + ); } // check if root is a prefix of target @@ -1147,6 +1160,7 @@ fn copy_directory( local_to_target.as_path(), options, symlinked_files, + false, ) { Ok(_) => Ok(()), Err(err) => { @@ -1167,6 +1181,7 @@ fn copy_directory( local_to_target.as_path(), options, symlinked_files, + false, )?; } } @@ -1316,8 +1331,14 @@ fn backup_dest(dest: &Path, backup_path: &Path) -> CopyResult { Ok(backup_path.into()) } -fn handle_existing_dest(source: &Path, dest: &Path, options: &Options) -> CopyResult<()> { - let dereference_to_compare = options.dereference || !is_symlink(source); +fn handle_existing_dest( + source: &Path, + dest: &Path, + options: &Options, + source_in_command_line: bool, +) -> CopyResult<()> { + let dereference_to_compare = + options.dereference(source_in_command_line) || !source.is_symlink(); if paths_refer_to_same_file(source, dest, dereference_to_compare) { return Err(format!("{}: same file", context_for(source, dest)).into()); } @@ -1369,9 +1390,10 @@ fn copy_file( dest: &Path, options: &Options, symlinked_files: &mut HashSet, + source_in_command_line: bool, ) -> CopyResult<()> { if dest.exists() { - handle_existing_dest(source, dest, options)?; + handle_existing_dest(source, dest, options, source_in_command_line)?; } // Fail if dest is a dangling symlink or a symlink this program created previously @@ -1386,7 +1408,7 @@ fn copy_file( dest.display() ))); } - let copy_contents = options.dereference || !is_symlink(source); + let copy_contents = options.dereference(source_in_command_line) || !source.is_symlink(); if copy_contents && !dest.exists() { return Err(Error::Error(format!( "not writing through dangling symlink '{}'", @@ -1403,14 +1425,15 @@ fn copy_file( let context = context_for(source, dest); let context = context.as_str(); - // canonicalize dest and source so that later steps can work with the paths directly - let source = if options.dereference { - canonicalize(source, MissingHandling::Missing, ResolveMode::Physical).unwrap() - } else { - source.to_owned() + let source_metadata = { + let result = if options.dereference(source_in_command_line) { + fs::metadata(source) + } else { + fs::symlink_metadata(source) + }; + result.context(context)? }; - - let source_file_type = fs::symlink_metadata(&source).context(context)?.file_type(); + let source_file_type = source_metadata.file_type(); let source_is_symlink = source_file_type.is_symlink(); #[cfg(unix)] @@ -1418,21 +1441,11 @@ fn copy_file( #[cfg(not(unix))] let source_is_fifo = false; - let dest_already_exists_as_symlink = is_symlink(dest); - - let dest = if !(source_is_symlink && dest_already_exists_as_symlink) { - canonicalize(dest, MissingHandling::Missing, ResolveMode::Physical).unwrap() - } else { - // Don't canonicalize a symlink copied over another symlink, because - // then we'll end up overwriting the destination's target. - dest.to_path_buf() - }; - let dest_permissions = if dest.exists() { dest.symlink_metadata().context(context)?.permissions() } else { #[allow(unused_mut)] - let mut permissions = source.symlink_metadata().context(context)?.permissions(); + let mut permissions = source_metadata.permissions(); #[cfg(unix)] { use uucore::mode::get_umask; @@ -1455,19 +1468,25 @@ fn copy_file( CopyMode::Link => { if dest.exists() { let backup_path = - backup_control::get_backup_path(options.backup, &dest, &options.backup_suffix); + backup_control::get_backup_path(options.backup, dest, &options.backup_suffix); if let Some(backup_path) = backup_path { - backup_dest(&dest, &backup_path)?; - fs::remove_file(&dest)?; + backup_dest(dest, &backup_path)?; + fs::remove_file(dest)?; } } - - fs::hard_link(&source, &dest).context(context)?; + if options.dereference(source_in_command_line) && source.is_symlink() { + let resolved = + canonicalize(source, MissingHandling::Missing, ResolveMode::Physical).unwrap(); + fs::hard_link(resolved, dest) + } else { + fs::hard_link(source, dest) + } + .context(context)?; } CopyMode::Copy => { copy_helper( - &source, - &dest, + source, + dest, options, context, source_is_symlink, @@ -1476,21 +1495,20 @@ fn copy_file( )?; } CopyMode::SymLink => { - symlink_file(&source, &dest, context, symlinked_files)?; + symlink_file(source, dest, context, symlinked_files)?; } CopyMode::Update => { if dest.exists() { - let src_metadata = fs::symlink_metadata(&source)?; - let dest_metadata = fs::symlink_metadata(&dest)?; + let dest_metadata = fs::symlink_metadata(dest)?; - let src_time = src_metadata.modified()?; + let src_time = source_metadata.modified()?; let dest_time = dest_metadata.modified()?; if src_time <= dest_time { return Ok(()); } else { copy_helper( - &source, - &dest, + source, + dest, options, context, source_is_symlink, @@ -1500,8 +1518,8 @@ fn copy_file( } } else { copy_helper( - &source, - &dest, + source, + dest, options, context, source_is_symlink, @@ -1528,10 +1546,10 @@ fn copy_file( // // FWIW, the OS will throw an error later, on the write op, if // the user does not have permission to write to the file. - fs::set_permissions(&dest, dest_permissions).ok(); + fs::set_permissions(dest, dest_permissions).ok(); } for attribute in &options.preserve_attributes { - copy_attribute(&source, &dest, attribute)?; + copy_attribute(source, dest, attribute)?; } Ok(()) } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 7671e31bb..d6b00c344 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -7,8 +7,6 @@ use std::fs::set_permissions; #[cfg(not(windows))] use std::os::unix::fs; -#[cfg(unix)] -use std::os::unix::fs::symlink as symlink_file; #[cfg(all(unix, not(target_os = "freebsd")))] use std::os::unix::fs::MetadataExt; #[cfg(all(unix, not(target_os = "freebsd")))] @@ -1684,7 +1682,7 @@ fn test_canonicalize_symlink() { let (at, mut ucmd) = at_and_ucmd!(); at.mkdir("dir"); at.touch("dir/file"); - symlink_file("../dir/file", at.plus("dir/file-ln")).unwrap(); + at.relative_symlink_file("../dir/file", "dir/file-ln"); ucmd.arg("dir/file-ln") .arg(".") .succeeds() @@ -2013,3 +2011,69 @@ fn test_cp_parents_2_link() { assert!(at.dir_exists("d/a/link") && !at.symlink_exists("d/a/link")); assert!(at.file_exists("d/a/link/c")); } + +#[test] +fn test_cp_copy_symlink_contents_recursive() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("src-dir"); + at.mkdir("dest-dir"); + at.touch("f"); + at.write("f", "f"); + at.relative_symlink_file("f", "slink"); + at.relative_symlink_file("no-file", &path_concat!("src-dir", "slink")); + ucmd.args(&["-H", "-R", "slink", "src-dir", "dest-dir"]) + .succeeds(); + assert!(at.dir_exists("src-dir")); + assert!(at.dir_exists("dest-dir")); + assert!(at.dir_exists(&path_concat!("dest-dir", "src-dir"))); + let regular_file = path_concat!("dest-dir", "slink"); + assert!(!at.symlink_exists(®ular_file) && at.file_exists(®ular_file)); + assert_eq!(at.read(®ular_file), "f"); +} + +#[test] +fn test_cp_mode_symlink() { + for from in ["file", "slink", "slink2"] { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("file"); + at.write("file", "f"); + at.relative_symlink_file("file", "slink"); + at.relative_symlink_file("slink", "slink2"); + ucmd.args(&["-s", "-L", from, "z"]).succeeds(); + assert!(at.symlink_exists("z")); + assert_eq!(at.read_symlink("z"), from); + } +} + +// Android doesn't allow creating hard links +#[cfg(not(target_os = "android"))] +#[test] +fn test_cp_mode_hardlink() { + for from in ["file", "slink", "slink2"] { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("file"); + at.write("file", "f"); + at.relative_symlink_file("file", "slink"); + at.relative_symlink_file("slink", "slink2"); + ucmd.args(&["--link", "-L", from, "z"]).succeeds(); + assert!(at.file_exists("z") && !at.symlink_exists("z")); + assert_eq!(at.read("z"), "f"); + // checking that it's the same hard link + at.append("z", "g"); + assert_eq!(at.read("file"), "fg"); + } +} + +// Android doesn't allow creating hard links +#[cfg(not(target_os = "android"))] +#[test] +fn test_cp_mode_hardlink_no_dereference() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("file"); + at.write("file", "f"); + at.relative_symlink_file("file", "slink"); + at.relative_symlink_file("slink", "slink2"); + ucmd.args(&["--link", "-P", "slink2", "z"]).succeeds(); + assert!(at.symlink_exists("z")); + assert_eq!(at.read_symlink("z"), "slink"); +} diff --git a/tests/common/util.rs b/tests/common/util.rs index 7f2c2c5a4..2597af2c1 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -751,6 +751,15 @@ impl AtPath { } } + pub fn read_symlink(&self, path: &str) -> String { + log_info("read_symlink", self.plus_as_string(path)); + fs::read_link(&self.plus(path)) + .unwrap() + .to_str() + .unwrap() + .to_owned() + } + pub fn symlink_metadata(&self, path: &str) -> fs::Metadata { match fs::symlink_metadata(&self.plus(path)) { Ok(m) => m,