diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index 343edd2b3..7d9d4014c 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -12,17 +12,17 @@ extern crate uucore; use clap::{crate_version, Arg, Command}; use uucore::display::Quotable; -use uucore::error::{UError, UResult}; +use uucore::error::{FromIo, UError, UResult}; use uucore::format_usage; -use uucore::fs::{is_symlink, paths_refer_to_same_file}; +use uucore::fs::{is_symlink, make_path_relative_to, paths_refer_to_same_file}; use std::borrow::Cow; use std::error::Error; -use std::ffi::{OsStr, OsString}; +use std::ffi::OsString; use std::fmt::Display; use std::fs; -use std::io::{stdin, Result}; +use std::io::stdin; #[cfg(any(unix, target_os = "redox"))] use std::os::unix::fs::symlink; #[cfg(windows)] @@ -55,8 +55,7 @@ pub enum OverwriteMode { enum LnError { TargetIsDirectory(PathBuf), SomeLinksFailed, - FailedToLink(PathBuf, PathBuf, String), - SameFile(), + SameFile(PathBuf, PathBuf), MissingDestination(PathBuf), ExtraOperand(OsString), } @@ -65,13 +64,10 @@ impl Display for LnError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::TargetIsDirectory(s) => write!(f, "target {} is not a directory", s.quote()), - Self::FailedToLink(s, d, e) => { - write!(f, "failed to link {} to {}: {}", s.quote(), d.quote(), e) + Self::SameFile(s, d) => { + write!(f, "{} and {} are the same file", s.quote(), d.quote()) } - Self::SameFile() => { - write!(f, "Same file") - } - Self::SomeLinksFailed => write!(f, "some links failed to create"), + Self::SomeLinksFailed => Ok(()), Self::MissingDestination(s) => { write!(f, "missing destination file operand after {}", s.quote()) } @@ -89,14 +85,7 @@ impl Error for LnError {} impl UError for LnError { fn code(&self) -> i32 { - match self { - Self::TargetIsDirectory(_) - | Self::SomeLinksFailed - | Self::FailedToLink(_, _, _) - | Self::SameFile() - | Self::MissingDestination(_) - | Self::ExtraOperand(_) => 1, - } + 1 } } @@ -315,15 +304,7 @@ fn exec(files: &[PathBuf], settings: &Settings) -> UResult<()> { } assert!(!files.is_empty()); - match link(&files[0], &files[1], settings) { - Ok(_) => Ok(()), - Err(e) => { - Err( - LnError::FailedToLink(files[0].to_owned(), files[1].to_owned(), e.to_string()) - .into(), - ) - } - } + link(&files[0], &files[1], settings) } fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) -> UResult<()> { @@ -374,12 +355,7 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) }; if let Err(e) = link(srcpath, &targetpath, settings) { - show_error!( - "cannot link {} to {}: {}", - targetpath.quote(), - srcpath.quote(), - e - ); + show_error!("{}", e); all_successful = false; } } @@ -390,38 +366,23 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) } } -fn relative_path<'a>(src: &Path, dst: &Path) -> Result> { - let src_abs = canonicalize(src, MissingHandling::Normal, ResolveMode::Logical)?; - let mut dst_abs = canonicalize( - dst.parent().unwrap(), - MissingHandling::Normal, - ResolveMode::Logical, - )?; - dst_abs.push(dst.components().last().unwrap()); - let suffix_pos = src_abs - .components() - .zip(dst_abs.components()) - .take_while(|(s, d)| s == d) - .count(); - - let src_iter = src_abs.components().skip(suffix_pos).map(|x| x.as_os_str()); - - let mut result: PathBuf = dst_abs - .components() - .skip(suffix_pos + 1) - .map(|_| OsStr::new("..")) - .chain(src_iter) - .collect(); - if result.as_os_str().is_empty() { - result.push("."); +fn relative_path<'a>(src: &'a Path, dst: &Path) -> Cow<'a, Path> { + if let Ok(src_abs) = canonicalize(src, MissingHandling::Missing, ResolveMode::Physical) { + if let Ok(dst_abs) = canonicalize( + dst.parent().unwrap(), + MissingHandling::Missing, + ResolveMode::Physical, + ) { + return make_path_relative_to(src_abs, dst_abs).into(); + } } - Ok(result.into()) + src.into() } fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> { let mut backup_path = None; let source: Cow<'_, Path> = if settings.relative { - relative_path(src, dst)? + relative_path(src, dst) } else { src.into() }; @@ -436,11 +397,11 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> { if settings.backup == BackupMode::ExistingBackup && !settings.symbolic { // when ln --backup f f, it should detect that it is the same file if paths_refer_to_same_file(src, dst, true) { - return Err(LnError::SameFile().into()); + return Err(LnError::SameFile(src.to_owned(), dst.to_owned()).into()); } } if let Some(ref p) = backup_path { - fs::rename(dst, p)?; + fs::rename(dst, p).map_err_context(|| format!("cannot backup {}", dst.quote()))?; } match settings.overwrite { OverwriteMode::NoClobber => {} @@ -455,7 +416,7 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> { } OverwriteMode::Force => { if !is_symlink(dst) && paths_refer_to_same_file(src, dst, true) { - return Err(LnError::SameFile().into()); + return Err(LnError::SameFile(src.to_owned(), dst.to_owned()).into()); } if fs::remove_file(dst).is_ok() {}; // In case of error, don't do anything @@ -470,11 +431,18 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> { // 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 - std::fs::canonicalize(&source)? + fs::canonicalize(&source) + .map_err_context(|| format!("failed to access {}", source.quote()))? } else { source.to_path_buf() }; - fs::hard_link(&p, dst)?; + fs::hard_link(&p, dst).map_err_context(|| { + format!( + "failed to create hard link {} => {}", + source.quote(), + dst.quote() + ) + })?; } if settings.verbose { diff --git a/src/uu/realpath/src/realpath.rs b/src/uu/realpath/src/realpath.rs index db3b9c5fc..ade9d215e 100644 --- a/src/uu/realpath/src/realpath.rs +++ b/src/uu/realpath/src/realpath.rs @@ -11,12 +11,12 @@ extern crate uucore; use clap::{crate_version, Arg, ArgMatches, Command}; -use std::path::Component; use std::{ io::{stdout, Write}, path::{Path, PathBuf}, }; use uucore::error::UClapError; +use uucore::fs::make_path_relative_to; use uucore::{ display::{print_verbatim, Quotable}, error::{FromIo, UResult}, @@ -274,36 +274,13 @@ fn process_relative( ) -> PathBuf { if let Some(base) = relative_base { if path.starts_with(base) { - make_path_relative_to(&path, relative_to.unwrap_or(base)) + make_path_relative_to(path.as_path(), relative_to.unwrap_or(base)) } else { path } } else if let Some(to) = relative_to { - make_path_relative_to(&path, to) + make_path_relative_to(path.as_path(), to) } else { path } } - -/// Converts absolute `path` to be relative to absolute `to` path. -fn make_path_relative_to(path: &Path, to: &Path) -> PathBuf { - let common_prefix_size = path - .components() - .zip(to.components()) - .take_while(|(first, second)| first == second) - .count(); - let path_suffix = path - .components() - .skip(common_prefix_size) - .map(|x| x.as_os_str()); - let mut components: Vec<_> = to - .components() - .skip(common_prefix_size) - .map(|_| Component::ParentDir.as_os_str()) - .chain(path_suffix) - .collect(); - if components.is_empty() { - components.push(Component::CurDir.as_os_str()); - } - components.iter().collect() -} diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index bd5909674..7b47d2158 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -488,6 +488,31 @@ pub fn paths_refer_to_same_file>(p1: P, p2: P, dereference: bool) false } +/// Converts absolute `path` to be relative to absolute `to` path. +pub fn make_path_relative_to>(path: P, to: P) -> PathBuf { + let path = path.as_ref(); + let to = to.as_ref(); + let common_prefix_size = path + .components() + .zip(to.components()) + .take_while(|(first, second)| first == second) + .count(); + let path_suffix = path + .components() + .skip(common_prefix_size) + .map(|x| x.as_os_str()); + let mut components: Vec<_> = to + .components() + .skip(common_prefix_size) + .map(|_| Component::ParentDir.as_os_str()) + .chain(path_suffix) + .collect(); + if components.is_empty() { + components.push(Component::CurDir.as_os_str()); + } + components.iter().collect() +} + #[cfg(test)] mod tests { // Note this useful idiom: importing names from outer (for mod tests) scope.