diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 23b838d29..7f1eb911f 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -14,7 +14,6 @@ use std::path::Path; use uucore::display::Quotable; use uucore::error::{ExitCode, UResult, USimpleError, UUsageError}; use uucore::fs::display_permissions_unix; -use uucore::fs::is_symlink; use uucore::libc::mode_t; #[cfg(not(windows))] use uucore::mode; @@ -195,7 +194,7 @@ impl Chmoder { let filename = &filename[..]; let file = Path::new(filename); if !file.exists() { - if is_symlink(file) { + if file.is_symlink() { println!( "failed to change mode of {} from 0000 (---------) to 0000 (---------)", filename.quote() @@ -237,10 +236,10 @@ impl Chmoder { fn walk_dir(&self, file_path: &Path) -> UResult<()> { let mut r = self.chmod_file(file_path); - if !is_symlink(file_path) && file_path.is_dir() { + if !file_path.is_symlink() && file_path.is_dir() { for dir_entry in file_path.read_dir()? { let path = dir_entry?.path(); - if !is_symlink(&path) { + if !path.is_symlink() { r = self.walk_dir(path.as_path()); } } @@ -262,7 +261,7 @@ impl Chmoder { let fperm = match fs::metadata(file) { Ok(meta) => meta.mode() & 0o7777, Err(err) => { - if is_symlink(file) { + if file.is_symlink() { if self.verbose { println!( "neither symbolic link {} nor referent has been changed", diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index e82399b42..2c5222bc9 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -47,7 +47,7 @@ use std::str::FromStr; use std::string::ToString; use uucore::backup_control::{self, BackupMode}; use uucore::error::{set_exit_code, UClapError, UError, UResult, UUsageError}; -use uucore::fs::{canonicalize, is_symlink, MissingHandling, ResolveMode}; +use uucore::fs::{canonicalize, MissingHandling, ResolveMode}; use walkdir::WalkDir; quick_error! { @@ -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, @@ -496,6 +497,11 @@ pub fn uu_app<'a>() -> Command<'a> { .overrides_with(options::NO_DEREFERENCE) .help("always follow symbolic links in SOURCE"), ) + .arg( + Arg::new(options::CLI_SYMBOLIC_LINKS) + .short('H') + .help("follow command-line symbolic links in SOURCE"), + ) .arg( Arg::new(options::ARCHIVE) .short('a') @@ -543,11 +549,6 @@ pub fn uu_app<'a>() -> Command<'a> { default type", ), ) - .arg( - Arg::new(options::CLI_SYMBOLIC_LINKS) - .short('H') - .help("NotImplemented: follow command-line symbolic links in SOURCE"), - ) // END TODO .arg( Arg::new(options::PATHS) @@ -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 @@ -1128,7 +1141,7 @@ fn copy_directory( }; let local_to_target = target.join(&local_to_root_parent); - if is_symlink(p.path()) && !options.dereference { + if p.path().is_symlink() && !options.dereference { copy_link(&path, &local_to_target, symlinked_files)?; } else if path.is_dir() && !local_to_target.exists() { if target.is_file() { @@ -1147,10 +1160,11 @@ fn copy_directory( local_to_target.as_path(), options, symlinked_files, + false, ) { Ok(_) => Ok(()), Err(err) => { - if is_symlink(source) { + if source.is_symlink() { // silent the error with a symlink // In case we do --archive, we might copy the symlink // before the file itself @@ -1167,6 +1181,7 @@ fn copy_directory( local_to_target.as_path(), options, symlinked_files, + false, )?; } } @@ -1204,7 +1219,7 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu // permissions of a symbolic link. In that case, we just // do nothing, since every symbolic link has the same // permissions. - if !is_symlink(dest) { + if !dest.is_symlink() { fs::set_permissions(dest, source_metadata.permissions()).context(context)?; // FIXME: Implement this for windows as well #[cfg(feature = "feat_acl")] @@ -1239,7 +1254,7 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu Attribute::Timestamps => { let atime = FileTime::from_last_access_time(&source_metadata); let mtime = FileTime::from_last_modification_time(&source_metadata); - if is_symlink(dest) { + if dest.is_symlink() { filetime::set_symlink_file_times(dest, atime, mtime)?; } else { filetime::set_file_times(dest, atime, mtime)?; @@ -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,13 +1390,14 @@ 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 - if is_symlink(dest) { + if dest.is_symlink() { if FileInformation::from_path(dest, false) .map(|info| symlinked_files.contains(&info)) .unwrap_or(false) @@ -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, @@ -1521,17 +1539,17 @@ fn copy_file( }; // TODO: implement something similar to gnu's lchown - if !is_symlink(&dest) { + if !dest.is_symlink() { // Here, to match GNU semantics, we quietly ignore an error // if a user does not have the correct ownership to modify // the permissions of a 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(()) } @@ -1630,7 +1648,7 @@ fn copy_link( } else { // we always need to remove the file to be able to create a symlink, // even if it is writeable. - if is_symlink(dest) || dest.is_file() { + if dest.is_symlink() || dest.is_file() { fs::remove_file(dest)?; } dest.into() diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index 10900260b..93e0c8dce 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -14,7 +14,7 @@ use clap::{crate_version, Arg, Command}; use uucore::display::Quotable; use uucore::error::{FromIo, UError, UResult}; use uucore::format_usage; -use uucore::fs::{is_symlink, make_path_relative_to, paths_refer_to_same_file}; +use uucore::fs::{make_path_relative_to, paths_refer_to_same_file}; use std::borrow::Cow; use std::error::Error; @@ -318,7 +318,7 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) if settings.no_dereference && matches!(settings.overwrite, OverwriteMode::Force) { // In that case, we don't want to do link resolution // We need to clean the target - if is_symlink(target_dir) { + if target_dir.is_symlink() { if target_dir.is_file() { if let Err(e) = fs::remove_file(target_dir) { show_error!("Could not update {}: {}", target_dir.quote(), e); @@ -387,7 +387,7 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> { src.into() }; - if is_symlink(dst) || dst.exists() { + if dst.is_symlink() || dst.exists() { backup_path = match settings.backup { BackupMode::NoBackup => None, BackupMode::SimpleBackup => Some(simple_backup_path(dst, &settings.suffix)), @@ -415,7 +415,7 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> { // In case of error, don't do anything } OverwriteMode::Force => { - if !is_symlink(dst) && paths_refer_to_same_file(src, dst, true) { + if !dst.is_symlink() && paths_refer_to_same_file(src, dst, true) { return Err(LnError::SameFile(src.to_owned(), dst.to_owned()).into()); } if fs::remove_file(dst).is_ok() {}; @@ -427,7 +427,7 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> { if settings.symbolic { symlink(&source, dst)?; } else { - let p = if settings.logical && is_symlink(&source) { + let p = if settings.logical && source.is_symlink() { // if we want to have an hard link, // source is a symlink and -L is passed // we want to resolve the symlink to create the hardlink diff --git a/src/uu/rmdir/src/rmdir.rs b/src/uu/rmdir/src/rmdir.rs index 5a87290fe..5f44b4d2b 100644 --- a/src/uu/rmdir/src/rmdir.rs +++ b/src/uu/rmdir/src/rmdir.rs @@ -19,8 +19,6 @@ use std::path::Path; use uucore::display::Quotable; use uucore::error::{set_exit_code, strip_errno, UResult}; -#[cfg(unix)] -use uucore::fs::is_symlink; use uucore::{format_usage, util_name}; static ABOUT: &str = "Remove the DIRECTORY(ies), if they are empty."; @@ -78,7 +76,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { if error.raw_os_error() == Some(libc::ENOTDIR) && bytes.ends_with(b"/") { // Strip the trailing slash or .symlink_metadata() will follow the symlink let no_slash: &Path = OsStr::from_bytes(&bytes[..bytes.len() - 1]).as_ref(); - if is_symlink(no_slash) && points_to_directory(no_slash).unwrap_or(true) { + if no_slash.is_symlink() && points_to_directory(no_slash).unwrap_or(true) { show_error!( "failed to remove {}: Symbolic link not followed", path.quote() diff --git a/src/uu/sort/src/chunks.rs b/src/uu/sort/src/chunks.rs index d7e795efa..9b60d5f5b 100644 --- a/src/uu/sort/src/chunks.rs +++ b/src/uu/sort/src/chunks.rs @@ -7,6 +7,9 @@ //! Utilities for reading files as chunks. +#![allow(dead_code)] +// Ignores non-used warning for `borrow_buffer` in `Chunk` + use std::{ io::{ErrorKind, Read}, sync::mpsc::SyncSender, diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 2af6a77c9..2d41db418 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -235,16 +235,6 @@ pub fn normalize_path(path: &Path) -> PathBuf { ret } -/// Decide whether the given path is a symbolic link. -/// -/// This function is essentially a backport of the -/// [`std::path::Path::is_symlink`] function that exists in Rust -/// version 1.58 and greater. This can be removed when the minimum -/// supported version of Rust is 1.58. -pub fn is_symlink>(path: P) -> bool { - fs::symlink_metadata(path).map_or(false, |m| m.file_type().is_symlink()) -} - fn resolve_symlink>(path: P) -> IOResult> { let result = if fs::symlink_metadata(&path)?.file_type().is_symlink() { Some(fs::read_link(&path)?) diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index f1bc4912c..bd6ef2ca9 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -10,7 +10,7 @@ use crate::error::strip_errno; use crate::error::UResult; use crate::error::USimpleError; pub use crate::features::entries; -use crate::fs::{is_symlink, resolve_relative_path}; +use crate::fs::resolve_relative_path; use crate::show_error; use clap::Arg; use clap::ArgMatches; @@ -274,7 +274,7 @@ impl ChownExecutor { let root = root.as_ref(); // walkdir always dereferences the root directory, so we have to check it ourselves - if self.traverse_symlinks == TraverseSymlinks::None && is_symlink(root) { + if self.traverse_symlinks == TraverseSymlinks::None && root.is_symlink() { return 0; } 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,