From 8c4e076096425275d39ba6404c054a37b6f0fb55 Mon Sep 17 00:00:00 2001 From: Niyaz Nigmatullin Date: Mon, 11 Jul 2022 17:34:17 +0300 Subject: [PATCH 1/7] realpath: introduce relative options, make correct exit codes, make pass GNU test mist/realpath.sh --- src/uu/realpath/src/realpath.rs | 143 ++++++++++++++++++++++++++++---- 1 file changed, 129 insertions(+), 14 deletions(-) diff --git a/src/uu/realpath/src/realpath.rs b/src/uu/realpath/src/realpath.rs index 320545d412e..5a2c274a0cd 100644 --- a/src/uu/realpath/src/realpath.rs +++ b/src/uu/realpath/src/realpath.rs @@ -10,11 +10,13 @@ #[macro_use] extern crate uucore; -use clap::{crate_version, Arg, Command}; +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::{ display::{print_verbatim, Quotable}, error::{FromIo, UResult}, @@ -32,12 +34,14 @@ static OPT_PHYSICAL: &str = "physical"; static OPT_LOGICAL: &str = "logical"; const OPT_CANONICALIZE_MISSING: &str = "canonicalize-missing"; const OPT_CANONICALIZE_EXISTING: &str = "canonicalize-existing"; +const OPT_RELATIVE_TO: &str = "relative-to"; +const OPT_RELATIVE_BASE: &str = "relative-base"; static ARG_FILES: &str = "files"; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = uu_app().get_matches_from(args); + let matches = uu_app().try_get_matches_from(args).with_exit_code(1)?; /* the list of files */ @@ -58,8 +62,23 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } else { MissingHandling::Normal }; + let resolve_mode = if strip { + ResolveMode::None + } else if logical { + ResolveMode::Logical + } else { + ResolveMode::Physical + }; + let (relative_to, relative_base) = prepare_relative_options(&matches, can_mode, resolve_mode)?; for path in &paths { - let result = resolve_path(path, strip, zero, logical, can_mode); + let result = resolve_path( + path, + zero, + resolve_mode, + can_mode, + relative_to.as_deref(), + relative_base.as_deref(), + ); if !quiet { show_if_err!(result.map_err_context(|| path.maybe_quote().to_string())); } @@ -126,16 +145,76 @@ pub fn uu_app<'a>() -> Command<'a> { given name recursively, without requirements on components existence", ), ) + .arg( + Arg::new(OPT_RELATIVE_TO) + .long(OPT_RELATIVE_TO) + .takes_value(true) + .value_name("DIR") + .forbid_empty_values(true) + .help("print the resolved path relative to DIR"), + ) + .arg( + Arg::new(OPT_RELATIVE_BASE) + .long(OPT_RELATIVE_BASE) + .takes_value(true) + .value_name("DIR") + .forbid_empty_values(true) + .help("print absolute paths unless paths below DIR"), + ) .arg( Arg::new(ARG_FILES) .multiple_occurrences(true) - .takes_value(true) .required(true) - .min_values(1) + .forbid_empty_values(true) .value_hint(clap::ValueHint::AnyPath), ) } +fn prepare_relative_options( + matches: &ArgMatches, + can_mode: MissingHandling, + resolve_mode: ResolveMode, +) -> UResult<(Option, Option)> { + let relative_to = matches.value_of(OPT_RELATIVE_TO).map(|p| PathBuf::from(p)); + let relative_base = matches + .value_of(OPT_RELATIVE_BASE) + .map(|p| PathBuf::from(p)); + let relative_to = canonicalize_relative_option(relative_to, can_mode, resolve_mode)?; + let relative_base = canonicalize_relative_option(relative_base, can_mode, resolve_mode)?; + if let (Some(base), Some(to)) = (relative_base.as_deref(), relative_to.as_deref()) { + if !to.starts_with(base) { + return Ok((None, None)); + } + } + Ok((relative_to, relative_base)) +} + +fn canonicalize_relative_option( + relative: Option, + can_mode: MissingHandling, + resolve_mode: ResolveMode, +) -> UResult> { + Ok(match relative { + None => None, + Some(p) => Some( + canonicalize_relative(&p, can_mode, resolve_mode) + .map_err_context(|| p.maybe_quote().to_string())?, + ), + }) +} + +fn canonicalize_relative( + r: &Path, + can_mode: MissingHandling, + resolve: ResolveMode, +) -> std::io::Result { + let abs = canonicalize(r, can_mode, resolve)?; + if can_mode == MissingHandling::Existing && !abs.is_dir() { + abs.read_dir()?; // raise not a directory error + } + Ok(abs) +} + /// Resolve a path to an absolute form and print it. /// /// If `strip` is `true`, then this function does not attempt to resolve @@ -149,22 +228,58 @@ pub fn uu_app<'a>() -> Command<'a> { /// symbolic links. fn resolve_path( p: &Path, - strip: bool, zero: bool, - logical: bool, + resolve: ResolveMode, can_mode: MissingHandling, + relative_to: Option<&Path>, + relative_base: Option<&Path>, ) -> std::io::Result<()> { - let resolve = if strip { - ResolveMode::None - } else if logical { - ResolveMode::Logical - } else { - ResolveMode::Physical - }; let abs = canonicalize(p, can_mode, resolve)?; let line_ending = if zero { b'\0' } else { b'\n' }; + let abs = process_relative(abs, relative_base, relative_to); + print_verbatim(&abs)?; stdout().write_all(&[line_ending])?; Ok(()) } + +fn process_relative( + path: PathBuf, + relative_base: Option<&Path>, + relative_to: Option<&Path>, +) -> PathBuf { + if let Some(base) = relative_base { + if path.starts_with(base) { + make_path_relative_to(&path, relative_to.unwrap_or(base)) + } else { + path + } + } else if let Some(to) = relative_to { + make_path_relative_to(&path, to) + } else { + 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() +} From 6dc14c9a2eb6bc523cfd1e9b5d9a6d31980c32b4 Mon Sep 17 00:00:00 2001 From: Niyaz Nigmatullin Date: Mon, 11 Jul 2022 17:35:04 +0300 Subject: [PATCH 2/7] tests/realpath: add tests for realpath for new features --- tests/by-util/test_realpath.rs | 77 ++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/tests/by-util/test_realpath.rs b/tests/by-util/test_realpath.rs index 3318c214d4d..219fa2c20e7 100644 --- a/tests/by-util/test_realpath.rs +++ b/tests/by-util/test_realpath.rs @@ -263,3 +263,80 @@ fn test_realpath_when_symlink_part_is_missing() { .stderr_contains("realpath: dir1/foo2: No such file or directory\n") .stderr_contains("realpath: dir1/foo4: No such file or directory\n"); } + +#[test] +fn test_relative_existing_require_directories() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("dir1"); + at.touch("dir1/f"); + ucmd.args(&["-e", "--relative-base=.", "--relative-to=dir1/f", "."]) + .fails() + .code_is(1) + .stderr_contains("Not a directory"); +} + +#[test] +fn test_relative_existing_require_directories_2() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("dir1"); + at.touch("dir1/f"); + ucmd.args(&["-e", "--relative-base=.", "--relative-to=dir1", "."]) + .succeeds() + .stdout_is("..\n"); +} + +#[test] +fn test_relative_base_not_prefix_of_relative_to() { + new_ucmd!() + .args(&[ + "-sm", + "--relative-base=/usr/local", + "--relative-to=/usr", + "/usr", + "/usr/local", + ]) + .succeeds() + .stdout_is("/usr\n/usr/local\n"); +} + +#[test] +fn test_relative_string_handling() { + new_ucmd!() + .args(&["-m", "--relative-to=prefix", "prefixed/1"]) + .succeeds() + .stdout_is("../prefixed/1\n"); + new_ucmd!() + .args(&["-m", "--relative-to=prefixed", "prefix/1"]) + .succeeds() + .stdout_is("../prefix/1\n"); + new_ucmd!() + .args(&["-m", "--relative-to=prefixed", "prefixed/1"]) + .succeeds() + .stdout_is("1\n"); +} + +#[test] +fn test_relative() { + new_ucmd!() + .args(&[ + "-sm", + "--relative-base=/usr", + "--relative-to=/usr", + "/tmp", + "/usr", + ]) + .succeeds() + .stdout_is("/tmp\n.\n"); + new_ucmd!() + .args(&["-sm", "--relative-base=/", "--relative-to=/", "/", "/usr"]) + .succeeds() + .stdout_is(".\nusr\n"); + new_ucmd!() + .args(&["-sm", "--relative-base=/usr", "/tmp", "/usr"]) + .succeeds() + .stdout_is("/tmp\n.\n"); + new_ucmd!() + .args(&["-sm", "--relative-base=/", "/", "/usr"]) + .succeeds() + .stdout_is(".\nusr\n"); +} From 3f0308e58ce0f84fd55b2ad8376454dc1cb184d2 Mon Sep 17 00:00:00 2001 From: Niyaz Nigmatullin Date: Mon, 11 Jul 2022 17:44:09 +0300 Subject: [PATCH 3/7] realpath: follow clippy advice for cleaner code --- src/uu/realpath/src/realpath.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/realpath/src/realpath.rs b/src/uu/realpath/src/realpath.rs index 5a2c274a0cd..aa4d4aad818 100644 --- a/src/uu/realpath/src/realpath.rs +++ b/src/uu/realpath/src/realpath.rs @@ -175,10 +175,10 @@ fn prepare_relative_options( can_mode: MissingHandling, resolve_mode: ResolveMode, ) -> UResult<(Option, Option)> { - let relative_to = matches.value_of(OPT_RELATIVE_TO).map(|p| PathBuf::from(p)); + let relative_to = matches.value_of(OPT_RELATIVE_TO).map(PathBuf::from); let relative_base = matches .value_of(OPT_RELATIVE_BASE) - .map(|p| PathBuf::from(p)); + .map(PathBuf::from); let relative_to = canonicalize_relative_option(relative_to, can_mode, resolve_mode)?; let relative_base = canonicalize_relative_option(relative_base, can_mode, resolve_mode)?; if let (Some(base), Some(to)) = (relative_base.as_deref(), relative_to.as_deref()) { From 1c1784b5b488424ef2d6fbb5b4945d01018124d3 Mon Sep 17 00:00:00 2001 From: Niyaz Nigmatullin Date: Mon, 11 Jul 2022 17:44:27 +0300 Subject: [PATCH 4/7] tests/realpath: ignore spell-checker errors --- tests/by-util/test_realpath.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_realpath.rs b/tests/by-util/test_realpath.rs index 219fa2c20e7..29d1d906d78 100644 --- a/tests/by-util/test_realpath.rs +++ b/tests/by-util/test_realpath.rs @@ -330,7 +330,7 @@ fn test_relative() { new_ucmd!() .args(&["-sm", "--relative-base=/", "--relative-to=/", "/", "/usr"]) .succeeds() - .stdout_is(".\nusr\n"); + .stdout_is(".\nusr\n"); // spell-checker:disable-line new_ucmd!() .args(&["-sm", "--relative-base=/usr", "/tmp", "/usr"]) .succeeds() @@ -338,5 +338,5 @@ fn test_relative() { new_ucmd!() .args(&["-sm", "--relative-base=/", "/", "/usr"]) .succeeds() - .stdout_is(".\nusr\n"); + .stdout_is(".\nusr\n"); // spell-checker:disable-line } From 2f7de6ce2f504026fb71bde49685563b822c2d22 Mon Sep 17 00:00:00 2001 From: Niyaz Nigmatullin Date: Mon, 11 Jul 2022 17:50:37 +0300 Subject: [PATCH 5/7] realpath: reformat using rustfmt --- src/uu/realpath/src/realpath.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/uu/realpath/src/realpath.rs b/src/uu/realpath/src/realpath.rs index aa4d4aad818..22f3df1974c 100644 --- a/src/uu/realpath/src/realpath.rs +++ b/src/uu/realpath/src/realpath.rs @@ -176,9 +176,7 @@ fn prepare_relative_options( resolve_mode: ResolveMode, ) -> UResult<(Option, Option)> { let relative_to = matches.value_of(OPT_RELATIVE_TO).map(PathBuf::from); - let relative_base = matches - .value_of(OPT_RELATIVE_BASE) - .map(PathBuf::from); + let relative_base = matches.value_of(OPT_RELATIVE_BASE).map(PathBuf::from); let relative_to = canonicalize_relative_option(relative_to, can_mode, resolve_mode)?; let relative_base = canonicalize_relative_option(relative_base, can_mode, resolve_mode)?; if let (Some(base), Some(to)) = (relative_base.as_deref(), relative_to.as_deref()) { From 890bac2495a801118fceee497b4dfd6ad3eb7ecd Mon Sep 17 00:00:00 2001 From: Niyaz Nigmatullin Date: Mon, 11 Jul 2022 19:02:27 +0300 Subject: [PATCH 6/7] tests/realpath: made tests passing in windows --- tests/by-util/test_realpath.rs | 56 ++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/tests/by-util/test_realpath.rs b/tests/by-util/test_realpath.rs index 29d1d906d78..2f6bc0f7fc7 100644 --- a/tests/by-util/test_realpath.rs +++ b/tests/by-util/test_realpath.rs @@ -1,5 +1,7 @@ use crate::common::util::*; +#[cfg(windows)] +use regex::Regex; use std::path::{Path, MAIN_SEPARATOR}; static GIBBERISH: &str = "supercalifragilisticexpialidocious"; @@ -272,7 +274,7 @@ fn test_relative_existing_require_directories() { ucmd.args(&["-e", "--relative-base=.", "--relative-to=dir1/f", "."]) .fails() .code_is(1) - .stderr_contains("Not a directory"); + .stderr_contains("directory"); } #[test] @@ -287,7 +289,7 @@ fn test_relative_existing_require_directories_2() { #[test] fn test_relative_base_not_prefix_of_relative_to() { - new_ucmd!() + let result = new_ucmd!() .args(&[ "-sm", "--relative-base=/usr/local", @@ -295,20 +297,33 @@ fn test_relative_base_not_prefix_of_relative_to() { "/usr", "/usr/local", ]) - .succeeds() - .stdout_is("/usr\n/usr/local\n"); + .succeeds(); + + #[cfg(windows)] + result.stdout_matches(&Regex::new(r"^.*:\\usr\n.*:\\usr\\local$").unwrap()); + + #[cfg(not(windows))] + result.stdout_is("/usr\n/usr/local\n"); } #[test] fn test_relative_string_handling() { - new_ucmd!() + let result = new_ucmd!() .args(&["-m", "--relative-to=prefix", "prefixed/1"]) - .succeeds() - .stdout_is("../prefixed/1\n"); - new_ucmd!() + .succeeds(); + #[cfg(not(windows))] + result.stdout_is("../prefixed/1\n"); + #[cfg(windows)] + result.stdout_is("..\\prefixed\\1\n"); + + let result = new_ucmd!() .args(&["-m", "--relative-to=prefixed", "prefix/1"]) - .succeeds() - .stdout_is("../prefix/1\n"); + .succeeds(); + #[cfg(not(windows))] + result.stdout_is("../prefix/1\n"); + #[cfg(windows)] + result.stdout_is("..\\prefix\\1\n"); + new_ucmd!() .args(&["-m", "--relative-to=prefixed", "prefixed/1"]) .succeeds() @@ -317,7 +332,7 @@ fn test_relative_string_handling() { #[test] fn test_relative() { - new_ucmd!() + let result = new_ucmd!() .args(&[ "-sm", "--relative-base=/usr", @@ -325,16 +340,25 @@ fn test_relative() { "/tmp", "/usr", ]) - .succeeds() - .stdout_is("/tmp\n.\n"); + .succeeds(); + #[cfg(not(windows))] + result.stdout_is("/tmp\n.\n"); + #[cfg(windows)] + result.stdout_matches(&Regex::new(r"^.*:\\tmp\n\.$").unwrap()); + new_ucmd!() .args(&["-sm", "--relative-base=/", "--relative-to=/", "/", "/usr"]) .succeeds() .stdout_is(".\nusr\n"); // spell-checker:disable-line - new_ucmd!() + + let result = new_ucmd!() .args(&["-sm", "--relative-base=/usr", "/tmp", "/usr"]) - .succeeds() - .stdout_is("/tmp\n.\n"); + .succeeds(); + #[cfg(not(windows))] + result.stdout_is("/tmp\n.\n"); + #[cfg(windows)] + result.stdout_matches(&Regex::new(r"^.*:\\tmp\n\.$").unwrap()); + new_ucmd!() .args(&["-sm", "--relative-base=/", "/", "/usr"]) .succeeds() From 14a26d80aa1f22df8dee7df304fb55b66427c3de Mon Sep 17 00:00:00 2001 From: Niyaz Nigmatullin Date: Tue, 12 Jul 2022 07:48:42 +0300 Subject: [PATCH 7/7] realpath: documentation to the functions is written/fixed --- src/uu/realpath/src/realpath.rs | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/uu/realpath/src/realpath.rs b/src/uu/realpath/src/realpath.rs index 22f3df1974c..db3b9c5fc4f 100644 --- a/src/uu/realpath/src/realpath.rs +++ b/src/uu/realpath/src/realpath.rs @@ -170,6 +170,10 @@ pub fn uu_app<'a>() -> Command<'a> { ) } +/// Prepare `--relative-to` and `--relative-base` options. +/// Convert them to their absolute values. +/// Check if `--relative-to` is a descendant of `--relative-base`, +/// otherwise nullify their value. fn prepare_relative_options( matches: &ArgMatches, can_mode: MissingHandling, @@ -187,6 +191,7 @@ fn prepare_relative_options( Ok((relative_to, relative_base)) } +/// Prepare single `relative-*` option. fn canonicalize_relative_option( relative: Option, can_mode: MissingHandling, @@ -201,6 +206,13 @@ fn canonicalize_relative_option( }) } +/// Make `relative-to` or `relative-base` path values absolute. +/// +/// # Errors +/// +/// If the given path is not a directory the function returns an error. +/// If some parts of the file don't exist, or symlinks make loops, or +/// some other IO error happens, the function returns error, too. fn canonicalize_relative( r: &Path, can_mode: MissingHandling, @@ -215,8 +227,10 @@ fn canonicalize_relative( /// Resolve a path to an absolute form and print it. /// -/// If `strip` is `true`, then this function does not attempt to resolve -/// symbolic links in the path. If `zero` is `true`, then this function +/// If `relative_to` and/or `relative_base` is given +/// the path is printed in a relative form to one of this options. +/// See the details in `process_relative` function. +/// If `zero` is `true`, then this function /// prints the path followed by the null byte (`'\0'`) instead of a /// newline character (`'\n'`). /// @@ -242,6 +256,17 @@ fn resolve_path( Ok(()) } +/// Conditionally converts an absolute path to a relative form, +/// according to the rules: +/// 1. if only `relative_to` is given, the result is relative to `relative_to` +/// 1. if only `relative_base` is given, it checks whether given `path` is a descendant +/// of `relative_base`, on success the result is relative to `relative_base`, otherwise +/// the result is the given `path` +/// 1. if both `relative_to` and `relative_base` are given, the result is relative to `relative_to` +/// if `path` is a descendant of `relative_base`, otherwise the result is `path` +/// +/// For more information see +/// fn process_relative( path: PathBuf, relative_base: Option<&Path>, @@ -260,6 +285,7 @@ fn process_relative( } } +/// 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()