From df83d59b2dbf49af92c6237eba9aa46d095b9f6a Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Mon, 4 Jul 2022 17:22:58 -0400 Subject: [PATCH 1/3] uucore: add backport for Path::is_symlink() Add a `uucore::fs::is_symlink()` function that takes in a `std::path::Path` and decides whether the given path is a symbolic link. This is essentially a backport of the `Path::is_symlink()` function that appears in Rust version 1.58.0. This commit also replaces some now-duplicate code in `chmod`, `cp`, `ln`, and `rmdir` that checks whether a path is a symbolic link with a call to `is_symlink()`. Technically, this commit slightly changes the behavior of `cp`. Previously, there was a line of code like this if fs::symlink_metadata(&source)?.file_type().is_symlink() { where the `?` operator propagates an error from `symlink_metadata()` to the caller. Now the line of code is if is_symlink(source) { in which any error from `symlink_metadata()` has been converted to just be a `false` value. I believe this is a satisfactory tradeoff to make, since an error in accessing the file will likely cause an error later in the same code path. --- src/uu/chmod/src/chmod.rs | 8 +------- src/uu/cp/src/cp.rs | 26 +++++++------------------- src/uu/ln/src/ln.rs | 8 +------- src/uu/rmdir/src/rmdir.rs | 9 ++------- src/uucore/src/lib/features/fs.rs | 10 ++++++++++ src/uucore/src/lib/features/perms.rs | 9 ++------- 6 files changed, 23 insertions(+), 47 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index d4b81ee37d9..abde1bf7b2d 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -14,6 +14,7 @@ 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; @@ -380,10 +381,3 @@ impl Chmoder { } } } - -pub fn is_symlink>(path: P) -> bool { - match fs::symlink_metadata(path) { - Ok(m) => m.file_type().is_symlink(), - Err(_) => false, - } -} diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 91cbc635ff5..9daf35efd94 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -56,7 +56,7 @@ use std::str::FromStr; use std::string::ToString; use uucore::backup_control::{self, BackupMode}; use uucore::error::{set_exit_code, ExitCode, UClapError, UError, UResult}; -use uucore::fs::{canonicalize, MissingHandling, ResolveMode}; +use uucore::fs::{canonicalize, is_symlink, MissingHandling, ResolveMode}; use walkdir::WalkDir; quick_error! { @@ -999,9 +999,7 @@ fn copy_directory( } // if no-dereference is enabled and this is a symlink, copy it as a file - if !options.dereference && fs::symlink_metadata(root).unwrap().file_type().is_symlink() - // replace by is_symlink in rust>=1.58 - { + if !options.dereference && is_symlink(root) { return copy_file(root, target, options, symlinked_files); } @@ -1044,8 +1042,6 @@ fn copy_directory( .follow_links(options.dereference) { let p = or_continue!(path); - let is_symlink = fs::symlink_metadata(p.path())?.file_type().is_symlink(); - // replace by is_symlink in rust >=1.58 let path = current_dir.join(&p.path()); let local_to_root_parent = match root_parent { @@ -1069,7 +1065,7 @@ fn copy_directory( }; let local_to_target = target.join(&local_to_root_parent); - if is_symlink && !options.dereference { + if is_symlink(p.path()) && !options.dereference { copy_link(&path, &local_to_target, symlinked_files)?; } else if path.is_dir() && !local_to_target.exists() { if target.is_file() { @@ -1091,7 +1087,7 @@ fn copy_directory( ) { Ok(_) => Ok(()), Err(err) => { - if fs::symlink_metadata(&source)?.file_type().is_symlink() { + if is_symlink(source) { // silent the error with a symlink // In case we do --archive, we might copy the symlink // before the file itself @@ -1306,10 +1302,7 @@ fn copy_file( } // Fail if dest is a dangling symlink or a symlink this program created previously - if fs::symlink_metadata(dest) - .map(|m| m.file_type().is_symlink()) // replace by is_symlink in rust>=1.58 - .unwrap_or(false) - { + if is_symlink(dest) { if FileInformation::from_path(dest, false) .map(|info| symlinked_files.contains(&info)) .unwrap_or(false) @@ -1351,9 +1344,7 @@ fn copy_file( #[cfg(not(unix))] let source_is_fifo = false; - let dest_already_exists_as_symlink = fs::symlink_metadata(&dest) - .map(|meta| meta.file_type().is_symlink()) - .unwrap_or(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() @@ -1457,10 +1448,7 @@ fn copy_file( }; // TODO: implement something similar to gnu's lchown - if fs::symlink_metadata(&dest) - .map(|meta| !meta.file_type().is_symlink()) - .unwrap_or(false) - { + if !is_symlink(&dest) { // 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. diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index 5d7639d619e..ac128011789 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -14,6 +14,7 @@ use clap::{crate_version, Arg, Command}; use uucore::display::Quotable; use uucore::error::{UError, UResult}; use uucore::format_usage; +use uucore::fs::is_symlink; use std::borrow::Cow; use std::error::Error; @@ -533,10 +534,3 @@ pub fn symlink, P2: AsRef>(src: P1, dst: P2) -> Result<()> symlink_file(src, dst) } } - -pub fn is_symlink>(path: P) -> bool { - match fs::symlink_metadata(path) { - Ok(m) => m.file_type().is_symlink(), - Err(_) => false, - } -} diff --git a/src/uu/rmdir/src/rmdir.rs b/src/uu/rmdir/src/rmdir.rs index be4e537806d..ea3b27f9ab2 100644 --- a/src/uu/rmdir/src/rmdir.rs +++ b/src/uu/rmdir/src/rmdir.rs @@ -16,6 +16,7 @@ use std::io; use std::path::Path; use uucore::display::Quotable; use uucore::error::{set_exit_code, strip_errno, UResult}; +use uucore::fs::is_symlink; use uucore::{format_usage, util_name}; static ABOUT: &str = "Remove the DIRECTORY(ies), if they are empty."; @@ -65,10 +66,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { use std::ffi::OsStr; use std::os::unix::ffi::OsStrExt; - fn is_symlink(path: &Path) -> io::Result { - Ok(path.symlink_metadata()?.file_type().is_symlink()) - } - fn points_to_directory(path: &Path) -> io::Result { Ok(path.metadata()?.file_type().is_dir()) } @@ -77,9 +74,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).unwrap_or(false) - && points_to_directory(no_slash).unwrap_or(true) - { + if is_symlink(no_slash) && points_to_directory(no_slash).unwrap_or(true) { show_error!( "failed to remove {}: Symbolic link not followed", path.quote() diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 3963d865c27..fd0fbccb314 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -221,6 +221,16 @@ 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>(original: P) -> Result { const MAX_LINKS_FOLLOWED: u32 = 255; let mut followed = 0; diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 4574ba96c98..1d2fe04cfa7 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::resolve_relative_path; +use crate::fs::{is_symlink, resolve_relative_path}; use crate::show_error; use clap::Arg; use clap::ArgMatches; @@ -274,12 +274,7 @@ impl ChownExecutor { let root = root.as_ref(); // walkdir always dereferences the root directory, so we have to check it ourselves - // TODO: replace with `root.is_symlink()` once it is stable - if self.traverse_symlinks == TraverseSymlinks::None - && std::fs::symlink_metadata(root) - .map(|m| m.file_type().is_symlink()) - .unwrap_or(false) - { + if self.traverse_symlinks == TraverseSymlinks::None && is_symlink(root) { return 0; } From 11bbf4664719dcea95824ad3b74dd0cea1385eba Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 5 Jul 2022 08:30:52 +0200 Subject: [PATCH 2/3] Only include is_symink for #[cfg(unix)] --- src/uu/rmdir/src/rmdir.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uu/rmdir/src/rmdir.rs b/src/uu/rmdir/src/rmdir.rs index ea3b27f9ab2..4318f54298a 100644 --- a/src/uu/rmdir/src/rmdir.rs +++ b/src/uu/rmdir/src/rmdir.rs @@ -16,6 +16,8 @@ use std::io; 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}; From 7ca2ae4627c298c24fd85e37824058aa44eed935 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 5 Jul 2022 08:31:45 +0200 Subject: [PATCH 3/3] spell: Ignore backport --- src/uucore/src/lib/features/fs.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index fd0fbccb314..fe8726575db 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -8,6 +8,8 @@ //! Set of functions to manage files and symlinks +// spell-checker:ignore backport + #[cfg(unix)] use libc::{ mode_t, S_IFBLK, S_IFCHR, S_IFDIR, S_IFIFO, S_IFLNK, S_IFMT, S_IFREG, S_IFSOCK, S_IRGRP,