From b8a5588b81072ac47c41fb64ae840d0fb599cab4 Mon Sep 17 00:00:00 2001 From: David Matos Date: Tue, 27 Sep 2022 23:03:51 +0200 Subject: [PATCH 1/4] ls: add support for +FORMAT in timestyle --- src/uu/ls/src/ls.rs | 88 ++++++++++++++++++++++++++-------------- tests/by-util/test_ls.rs | 12 ++++++ 2 files changed, 70 insertions(+), 30 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index fd9ae34dbd4..1be54adcea9 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -10,7 +10,10 @@ #[macro_use] extern crate uucore; -use clap::{builder::ValueParser, crate_version, Arg, Command}; +use clap::{ + builder::{NonEmptyStringValueParser, ValueParser}, + crate_version, Arg, Command, +}; use glob::Pattern; use lscolors::LsColors; use number_prefix::NumberPrefix; @@ -156,6 +159,7 @@ enum LsError { IOErrorContext(std::io::Error, PathBuf, bool), BlockSizeParseError(String), AlreadyListedError(PathBuf), + TimeStyleParseError(String, Vec), } impl UError for LsError { @@ -167,6 +171,7 @@ impl UError for LsError { Self::IOErrorContext(_, _, true) => 2, Self::BlockSizeParseError(_) => 1, Self::AlreadyListedError(_) => 2, + Self::TimeStyleParseError(_, _) => 1, } } } @@ -179,6 +184,14 @@ impl Display for LsError { Self::BlockSizeParseError(s) => { write!(f, "invalid --block-size argument {}", s.quote()) } + Self::TimeStyleParseError(s, possible_time_styles) => { + write!( + f, + "invalid --time-style argument {}\nPossible values are: {:?}\n\nFor more information try --help", + s.quote(), + possible_time_styles + ) + } Self::InvalidLineWidth(s) => write!(f, "invalid line width: {}", s.quote()), Self::IOError(e) => write!(f, "general io error: {}", e), Self::IOErrorContext(e, p, _) => { @@ -300,8 +313,46 @@ enum TimeStyle { LongIso, Iso, Locale, + Format(String), } +fn parse_time_style(options: &clap::ArgMatches) -> Result { + let possible_time_styles = vec![ + "full-iso".to_string(), + "long-iso".to_string(), + "iso".to_string(), + "locale".to_string(), + "+FORMAT (e.g., +%H:%M) for a 'date'-style format".to_string(), + ]; + if let Some(field) = options.get_one::(options::TIME_STYLE) { + //If both FULL_TIME and TIME_STYLE are present + //The one added last is dominant + if options.contains_id(options::FULL_TIME) + && options.indices_of(options::FULL_TIME).unwrap().last() + > options.indices_of(options::TIME_STYLE).unwrap().last() + { + Ok(TimeStyle::FullIso) + } else { + match field.as_str() { + "full-iso" => Ok(TimeStyle::FullIso), + "long-iso" => Ok(TimeStyle::LongIso), + "iso" => Ok(TimeStyle::Iso), + "locale" => Ok(TimeStyle::Locale), + _ => match field.chars().next().unwrap() { + '+' => Ok(TimeStyle::Format(String::from(&field[1..]))), + _ => Err(LsError::TimeStyleParseError( + String::from(field), + possible_time_styles, + )), + }, + } + } + } else if options.contains_id(options::FULL_TIME) { + Ok(TimeStyle::FullIso) + } else { + Ok(TimeStyle::Locale) + } +} enum Dereference { None, DirArgs, @@ -700,31 +751,7 @@ impl Config { } else { IndicatorStyle::None }; - - let time_style = if let Some(field) = options.get_one::(options::TIME_STYLE) { - //If both FULL_TIME and TIME_STYLE are present - //The one added last is dominant - if options.contains_id(options::FULL_TIME) - && options.indices_of(options::FULL_TIME).unwrap().last() - > options.indices_of(options::TIME_STYLE).unwrap().last() - { - TimeStyle::FullIso - } else { - //Clap handles the env variable "TIME_STYLE" - match field.as_str() { - "full-iso" => TimeStyle::FullIso, - "long-iso" => TimeStyle::LongIso, - "iso" => TimeStyle::Iso, - "locale" => TimeStyle::Locale, - // below should never happen as clap already restricts the values. - _ => unreachable!("Invalid field for --time-style"), - } - } - } else if options.contains_id(options::FULL_TIME) { - TimeStyle::FullIso - } else { - TimeStyle::Locale - }; + let time_style = parse_time_style(options)?; let mut ignore_patterns: Vec = Vec::new(); @@ -1511,13 +1538,13 @@ pub fn uu_app<'a>() -> Command<'a> { ]), ) .arg( - //This still needs support for posix-*, +FORMAT + //This still needs support for posix-* Arg::new(options::TIME_STYLE) .long(options::TIME_STYLE) .help("time/date format with -l; see TIME_STYLE below") .value_name("TIME_STYLE") .env("TIME_STYLE") - .value_parser(["full-iso", "long-iso", "iso", "locale"]) + .value_parser(NonEmptyStringValueParser::new()) .overrides_with_all(&[options::TIME_STYLE]), ) .arg( @@ -1549,7 +1576,7 @@ pub fn uu_app<'a>() -> Command<'a> { .value_parser(ValueParser::os_string()), ) .after_help( - "The TIME_STYLE argument can be full-iso, long-iso, iso. \ + "The TIME_STYLE argument can be full-iso, long-iso, iso, locale or +FORMAT. FORMAT is interpreted like in date. \ Also the TIME_STYLE environment variable sets the default style to use.", ) } @@ -2519,7 +2546,7 @@ fn display_date(metadata: &Metadata, config: &Config) -> String { //According to GNU a Gregorian year has 365.2425 * 24 * 60 * 60 == 31556952 seconds on the average. let recent = time + chrono::Duration::seconds(31_556_952 / 2) > chrono::Local::now(); - match config.time_style { + match &config.time_style { TimeStyle::FullIso => time.format("%Y-%m-%d %H:%M:%S.%f %z"), TimeStyle::LongIso => time.format("%Y-%m-%d %H:%M"), TimeStyle::Iso => time.format(if recent { "%m-%d %H:%M" } else { "%Y-%m-%d " }), @@ -2534,6 +2561,7 @@ fn display_date(metadata: &Metadata, config: &Config) -> String { time.format(fmt) } + TimeStyle::Format(e) => time.format(e), } .to_string() } diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 79f3b07afae..e083464d64e 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -1653,6 +1653,7 @@ fn test_ls_styles() { let re_iso = Regex::new(r"[a-z-]* \d* \w* \w* \d* \d{2}-\d{2} \d{2}:\d{2} test\n").unwrap(); let re_locale = Regex::new(r"[a-z-]* \d* \w* \w* \d* [A-Z][a-z]{2} ( |\d)\d \d{2}:\d{2} test\n").unwrap(); + let re_custom_format = Regex::new(r"[a-z-]* \d* \w* \w* \d* \d{4}__\d{2} test\n").unwrap(); //full-iso let result = scene @@ -1675,6 +1676,17 @@ fn test_ls_styles() { let result = scene.ucmd().arg("-l").arg("--time-style=locale").succeeds(); assert!(re_locale.is_match(result.stdout_str())); + //+FORMAT + let result = scene + .ucmd() + .arg("-l") + .arg("--time-style=+%Y__%M") + .succeeds(); + assert!(re_custom_format.is_match(result.stdout_str())); + + // Also fails due to not having full clap support for time_styles + scene.ucmd().arg("-l").arg("-time-style=invalid").fails(); + //Overwrite options tests let result = scene .ucmd() From adf4bab03c9d7ddb60caed4c54095f67396fe6dd Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Mon, 3 Oct 2022 18:12:58 +0200 Subject: [PATCH 2/4] tests: do not generate module structure in build.rs Generating the tests to run in build.rs created problems for tooling. For example, cargo fmt, was ignoring the test_*.rs files and needed to be passed these files manually to be formatted. Now we simply use the feature mechanism to decide which tests to run. --- .github/workflows/CICD.yml | 14 -- build.rs | 51 ----- tests/tests.rs | 412 ++++++++++++++++++++++++++++++++++++- 3 files changed, 411 insertions(+), 66 deletions(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 07c768afdeb..d2f6f57264b 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -138,20 +138,6 @@ jobs: # * convert any errors/warnings to GHA UI annotations; ref: S=$(cargo fmt -- --check) && printf "%s\n" "$S" || { printf "%s\n" "$S" ; printf "%s\n" "$S" | sed -E -n -e "s/^Diff[[:space:]]+in[[:space:]]+${PWD//\//\\/}\/(.*)[[:space:]]+at[[:space:]]+[^0-9]+([0-9]+).*$/::${fault_type} file=\1,line=\2::${fault_prefix}: \`cargo fmt\`: style violation (file:'\1', line:\2; use \`cargo fmt -- \"\1\"\`)/p" ; fault=true ; } if [ -n "${{ steps.vars.outputs.FAIL_ON_FAULT }}" ] && [ -n "$fault" ]; then exit 1 ; fi - - name: "`cargo fmt` testing of integration tests" - if: success() || failure() # run regardless of prior step success/failure - shell: bash - run: | - ## `cargo fmt` testing of integration tests - unset fault - fault_type="${{ steps.vars.outputs.FAULT_TYPE }}" - fault_prefix=$(echo "$fault_type" | tr '[:lower:]' '[:upper:]') - # 'tests' is the standard/usual integration test directory - if [ -d tests ]; then - # * convert any errors/warnings to GHA UI annotations; ref: - S=$(find tests -name "*.rs" -print0 | xargs -0 cargo fmt -- --check) && printf "%s\n" "$S" || { printf "%s\n" "$S" ; printf "%s\n" "$S" | sed -E -n "s/^Diff[[:space:]]+in[[:space:]]+${PWD//\//\\/}\/(.*)[[:space:]]+at[[:space:]]+[^0-9]+([0-9]+).*$/::${fault_type} file=\1,line=\2::${fault_prefix}: \`cargo fmt\`: style violation (file:'\1', line:\2; use \`cargo fmt \"\1\"\`)/p" ; fault=true ; } - fi - if [ -n "${{ steps.vars.outputs.FAIL_ON_FAULT }}" ] && [ -n "$fault" ]; then exit 1 ; fi style_lint: name: Style/lint diff --git a/build.rs b/build.rs index 734dfc350a8..c9f07259aef 100644 --- a/build.rs +++ b/build.rs @@ -18,10 +18,6 @@ pub fn main() { let out_dir = env::var("OUT_DIR").unwrap(); // println!("cargo:warning=out_dir={}", out_dir); - let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap().replace('\\', "/"); - // println!("cargo:warning=manifest_dir={}", manifest_dir); - let util_tests_dir = format!("{}/tests/by-util", manifest_dir); - // println!("cargo:warning=util_tests_dir={}", util_tests_dir); let mut crates = Vec::new(); for (key, val) in env::vars() { @@ -41,7 +37,6 @@ pub fn main() { crates.sort(); let mut mf = File::create(Path::new(&out_dir).join("uutils_map.rs")).unwrap(); - let mut tf = File::create(Path::new(&out_dir).join("test_modules.rs")).unwrap(); mf.write_all( "type UtilityMap = phf::Map<&'static str, (fn(T) -> i32, fn() -> Command<'static>)>;\n\ @@ -60,42 +55,15 @@ pub fn main() { "uu_test" => { phf_map.entry("test", &map_value); phf_map.entry("[", &map_value); - - tf.write_all( - format!( - "#[path=\"{dir}/test_test.rs\"]\nmod test_test;\n", - dir = util_tests_dir, - ) - .as_bytes(), - ) - .unwrap(); } k if k.starts_with(OVERRIDE_PREFIX) => { phf_map.entry(&k[OVERRIDE_PREFIX.len()..], &map_value); - tf.write_all( - format!( - "#[path=\"{dir}/test_{k}.rs\"]\nmod test_{k};\n", - k = &krate[OVERRIDE_PREFIX.len()..], - dir = util_tests_dir, - ) - .as_bytes(), - ) - .unwrap(); } "false" | "true" => { phf_map.entry( krate, &format!("(r#{krate}::uumain, r#{krate}::uu_app)", krate = krate), ); - tf.write_all( - format!( - "#[path=\"{dir}/test_{krate}.rs\"]\nmod test_{krate};\n", - krate = krate, - dir = util_tests_dir, - ) - .as_bytes(), - ) - .unwrap(); } "hashsum" => { phf_map.entry( @@ -123,27 +91,9 @@ pub fn main() { phf_map.entry("shake256sum", &map_value_bits); phf_map.entry("b2sum", &map_value); phf_map.entry("b3sum", &map_value_b3sum); - tf.write_all( - format!( - "#[path=\"{dir}/test_{krate}.rs\"]\nmod test_{krate};\n", - krate = krate, - dir = util_tests_dir, - ) - .as_bytes(), - ) - .unwrap(); } _ => { phf_map.entry(krate, &map_value); - tf.write_all( - format!( - "#[path=\"{dir}/test_{krate}.rs\"]\nmod test_{krate};\n", - krate = krate, - dir = util_tests_dir, - ) - .as_bytes(), - ) - .unwrap(); } } } @@ -151,5 +101,4 @@ pub fn main() { mf.write_all(b"\n}\n").unwrap(); mf.flush().unwrap(); - tf.flush().unwrap(); } diff --git a/tests/tests.rs b/tests/tests.rs index 51c6f6b11a6..17581799dd7 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -4,4 +4,414 @@ mod common; #[cfg(unix)] extern crate rust_users; -include!(concat!(env!("OUT_DIR"), "/test_modules.rs")); +#[cfg(feature = "arch")] +#[path = "by-util/test_arch.rs"] +mod test_arch; + +#[cfg(feature = "base32")] +#[path = "by-util/test_base32.rs"] +mod test_base32; + +#[cfg(feature = "base64")] +#[path = "by-util/test_base64.rs"] +mod test_base64; + +#[cfg(feature = "basename")] +#[path = "by-util/test_basename.rs"] +mod test_basename; + +#[cfg(feature = "basenc")] +#[path = "by-util/test_basenc.rs"] +mod test_basenc; + +#[cfg(feature = "cat")] +#[path = "by-util/test_cat.rs"] +mod test_cat; + +#[cfg(feature = "chcon")] +#[path = "by-util/test_chcon.rs"] +mod test_chcon; + +#[cfg(feature = "chgrp")] +#[path = "by-util/test_chgrp.rs"] +mod test_chgrp; + +#[cfg(feature = "chmod")] +#[path = "by-util/test_chmod.rs"] +mod test_chmod; + +#[cfg(feature = "chown")] +#[path = "by-util/test_chown.rs"] +mod test_chown; + +#[cfg(feature = "chroot")] +#[path = "by-util/test_chroot.rs"] +mod test_chroot; + +#[cfg(feature = "cksum")] +#[path = "by-util/test_cksum.rs"] +mod test_cksum; + +#[cfg(feature = "comm")] +#[path = "by-util/test_comm.rs"] +mod test_comm; + +#[cfg(feature = "cp")] +#[path = "by-util/test_cp.rs"] +mod test_cp; + +#[cfg(feature = "csplit")] +#[path = "by-util/test_csplit.rs"] +mod test_csplit; + +#[cfg(feature = "cut")] +#[path = "by-util/test_cut.rs"] +mod test_cut; + +#[cfg(feature = "date")] +#[path = "by-util/test_date.rs"] +mod test_date; + +#[cfg(feature = "dd")] +#[path = "by-util/test_dd.rs"] +mod test_dd; + +#[cfg(feature = "df")] +#[path = "by-util/test_df.rs"] +mod test_df; + +#[cfg(feature = "dir")] +#[path = "by-util/test_dir.rs"] +mod test_dir; + +#[cfg(feature = "dircolors")] +#[path = "by-util/test_dircolors.rs"] +mod test_dircolors; + +#[cfg(feature = "dirname")] +#[path = "by-util/test_dirname.rs"] +mod test_dirname; + +#[cfg(feature = "du")] +#[path = "by-util/test_du.rs"] +mod test_du; + +#[cfg(feature = "echo")] +#[path = "by-util/test_echo.rs"] +mod test_echo; + +#[cfg(feature = "env")] +#[path = "by-util/test_env.rs"] +mod test_env; + +#[cfg(feature = "expand")] +#[path = "by-util/test_expand.rs"] +mod test_expand; + +#[cfg(feature = "expr")] +#[path = "by-util/test_expr.rs"] +mod test_expr; + +#[cfg(feature = "factor")] +#[path = "by-util/test_factor.rs"] +mod test_factor; + +#[cfg(feature = "false")] +#[path = "by-util/test_false.rs"] +mod test_false; + +#[cfg(feature = "fmt")] +#[path = "by-util/test_fmt.rs"] +mod test_fmt; + +#[cfg(feature = "fold")] +#[path = "by-util/test_fold.rs"] +mod test_fold; + +#[cfg(feature = "groups")] +#[path = "by-util/test_groups.rs"] +mod test_groups; + +#[cfg(feature = "hashsum")] +#[path = "by-util/test_hashsum.rs"] +mod test_hashsum; + +#[cfg(feature = "head")] +#[path = "by-util/test_head.rs"] +mod test_head; + +#[cfg(feature = "hostid")] +#[path = "by-util/test_hostid.rs"] +mod test_hostid; + +#[cfg(feature = "hostname")] +#[path = "by-util/test_hostname.rs"] +mod test_hostname; + +#[cfg(feature = "id")] +#[path = "by-util/test_id.rs"] +mod test_id; + +#[cfg(feature = "install")] +#[path = "by-util/test_install.rs"] +mod test_install; + +#[cfg(feature = "join")] +#[path = "by-util/test_join.rs"] +mod test_join; + +#[cfg(feature = "kill")] +#[path = "by-util/test_kill.rs"] +mod test_kill; + +#[cfg(feature = "link")] +#[path = "by-util/test_link.rs"] +mod test_link; + +#[cfg(feature = "ln")] +#[path = "by-util/test_ln.rs"] +mod test_ln; + +#[cfg(feature = "logname")] +#[path = "by-util/test_logname.rs"] +mod test_logname; + +#[cfg(feature = "ls")] +#[path = "by-util/test_ls.rs"] +mod test_ls; + +#[cfg(feature = "mkdir")] +#[path = "by-util/test_mkdir.rs"] +mod test_mkdir; + +#[cfg(feature = "mkfifo")] +#[path = "by-util/test_mkfifo.rs"] +mod test_mkfifo; + +#[cfg(feature = "mknod")] +#[path = "by-util/test_mknod.rs"] +mod test_mknod; + +#[cfg(feature = "mktemp")] +#[path = "by-util/test_mktemp.rs"] +mod test_mktemp; + +#[cfg(feature = "more")] +#[path = "by-util/test_more.rs"] +mod test_more; + +#[cfg(feature = "mv")] +#[path = "by-util/test_mv.rs"] +mod test_mv; + +#[cfg(feature = "nice")] +#[path = "by-util/test_nice.rs"] +mod test_nice; + +#[cfg(feature = "nl")] +#[path = "by-util/test_nl.rs"] +mod test_nl; + +#[cfg(feature = "nohup")] +#[path = "by-util/test_nohup.rs"] +mod test_nohup; + +#[cfg(feature = "nproc")] +#[path = "by-util/test_nproc.rs"] +mod test_nproc; + +#[cfg(feature = "numfmt")] +#[path = "by-util/test_numfmt.rs"] +mod test_numfmt; + +#[cfg(feature = "od")] +#[path = "by-util/test_od.rs"] +mod test_od; + +#[cfg(feature = "paste")] +#[path = "by-util/test_paste.rs"] +mod test_paste; + +#[cfg(feature = "pathchk")] +#[path = "by-util/test_pathchk.rs"] +mod test_pathchk; + +#[cfg(feature = "pinky")] +#[path = "by-util/test_pinky.rs"] +mod test_pinky; + +#[cfg(feature = "pr")] +#[path = "by-util/test_pr.rs"] +mod test_pr; + +#[cfg(feature = "printenv")] +#[path = "by-util/test_printenv.rs"] +mod test_printenv; + +#[cfg(feature = "printf")] +#[path = "by-util/test_printf.rs"] +mod test_printf; + +#[cfg(feature = "ptx")] +#[path = "by-util/test_ptx.rs"] +mod test_ptx; + +#[cfg(feature = "pwd")] +#[path = "by-util/test_pwd.rs"] +mod test_pwd; + +#[cfg(feature = "readlink")] +#[path = "by-util/test_readlink.rs"] +mod test_readlink; + +#[cfg(feature = "realpath")] +#[path = "by-util/test_realpath.rs"] +mod test_realpath; + +#[cfg(feature = "relpath")] +#[path = "by-util/test_relpath.rs"] +mod test_relpath; + +#[cfg(feature = "rm")] +#[path = "by-util/test_rm.rs"] +mod test_rm; + +#[cfg(feature = "rmdir")] +#[path = "by-util/test_rmdir.rs"] +mod test_rmdir; + +#[cfg(feature = "runcon")] +#[path = "by-util/test_runcon.rs"] +mod test_runcon; + +#[cfg(feature = "seq")] +#[path = "by-util/test_seq.rs"] +mod test_seq; + +#[cfg(feature = "shred")] +#[path = "by-util/test_shred.rs"] +mod test_shred; + +#[cfg(feature = "shuf")] +#[path = "by-util/test_shuf.rs"] +mod test_shuf; + +#[cfg(feature = "sleep")] +#[path = "by-util/test_sleep.rs"] +mod test_sleep; + +#[cfg(feature = "sort")] +#[path = "by-util/test_sort.rs"] +mod test_sort; + +#[cfg(feature = "split")] +#[path = "by-util/test_split.rs"] +mod test_split; + +#[cfg(feature = "stat")] +#[path = "by-util/test_stat.rs"] +mod test_stat; + +#[cfg(feature = "stdbuf")] +#[path = "by-util/test_stdbuf.rs"] +mod test_stdbuf; + +#[cfg(feature = "stty")] +#[path = "by-util/test_stty.rs"] +mod test_stty; + +#[cfg(feature = "sum")] +#[path = "by-util/test_sum.rs"] +mod test_sum; + +#[cfg(feature = "sync")] +#[path = "by-util/test_sync.rs"] +mod test_sync; + +#[cfg(feature = "tac")] +#[path = "by-util/test_tac.rs"] +mod test_tac; + +#[cfg(feature = "tail")] +#[path = "by-util/test_tail.rs"] +mod test_tail; + +#[cfg(feature = "tee")] +#[path = "by-util/test_tee.rs"] +mod test_tee; + +#[cfg(feature = "test")] +#[path = "by-util/test_test.rs"] +mod test_test; + +#[cfg(feature = "timeout")] +#[path = "by-util/test_timeout.rs"] +mod test_timeout; + +#[cfg(feature = "touch")] +#[path = "by-util/test_touch.rs"] +mod test_touch; + +#[cfg(feature = "tr")] +#[path = "by-util/test_tr.rs"] +mod test_tr; + +#[cfg(feature = "true")] +#[path = "by-util/test_true.rs"] +mod test_true; + +#[cfg(feature = "truncate")] +#[path = "by-util/test_truncate.rs"] +mod test_truncate; + +#[cfg(feature = "tsort")] +#[path = "by-util/test_tsort.rs"] +mod test_tsort; + +#[cfg(feature = "tty")] +#[path = "by-util/test_tty.rs"] +mod test_tty; + +#[cfg(feature = "uname")] +#[path = "by-util/test_uname.rs"] +mod test_uname; + +#[cfg(feature = "unexpand")] +#[path = "by-util/test_unexpand.rs"] +mod test_unexpand; + +#[cfg(feature = "uniq")] +#[path = "by-util/test_uniq.rs"] +mod test_uniq; + +#[cfg(feature = "unlink")] +#[path = "by-util/test_unlink.rs"] +mod test_unlink; + +#[cfg(feature = "uptime")] +#[path = "by-util/test_uptime.rs"] +mod test_uptime; + +#[cfg(feature = "users")] +#[path = "by-util/test_users.rs"] +mod test_users; + +#[cfg(feature = "vdir")] +#[path = "by-util/test_vdir.rs"] +mod test_vdir; + +#[cfg(feature = "wc")] +#[path = "by-util/test_wc.rs"] +mod test_wc; + +#[cfg(feature = "who")] +#[path = "by-util/test_who.rs"] +mod test_who; + +#[cfg(feature = "whoami")] +#[path = "by-util/test_whoami.rs"] +mod test_whoami; + +#[cfg(feature = "yes")] +#[path = "by-util/test_yes.rs"] +mod test_yes; From 493a2628d246aeaa5ffe754958dd3f7e961e5bef Mon Sep 17 00:00:00 2001 From: Pat Laster Date: Wed, 5 Oct 2022 06:35:31 -0500 Subject: [PATCH 3/4] rm: Added descend messages for interactive mode Fixes #3817 (#3931) Co-authored-by: Terts Diepraam --- src/uu/rm/src/rm.rs | 31 ++++++++++++++++++++++++-- tests/by-util/test_rm.rs | 47 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index f05e5d12c0c..5406979f7e4 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -303,13 +303,36 @@ fn handle_dir(path: &Path, options: &Options) -> bool { } } else { let mut dirs: VecDeque = VecDeque::new(); + // The Paths to not descend into. We need to this because WalkDir doesn't have a way, afaik, to not descend into a directory + // So we have to just ignore paths as they come up if they start with a path we aren't descending into + let mut not_descended: Vec = Vec::new(); - for entry in WalkDir::new(path) { + 'outer: for entry in WalkDir::new(path) { match entry { Ok(entry) => { + if options.interactive == InteractiveMode::Always { + for not_descend in ¬_descended { + if entry.path().starts_with(not_descend) { + // We don't need to continue the rest of code in this loop if we are in a directory we don't want to descend into + continue 'outer; + } + } + } let file_type = entry.file_type(); if file_type.is_dir() { - dirs.push_back(entry); + // If we are in Interactive Mode Always and the directory isn't empty we ask if we should descend else we push this directory onto dirs vector + if options.interactive == InteractiveMode::Always + && fs::read_dir(entry.path()).unwrap().count() != 0 + { + // If we don't descend we push this directory onto our not_descended vector else we push this directory onto dirs vector + if prompt_descend(entry.path()) { + dirs.push_back(entry); + } else { + not_descended.push(entry.path().to_path_buf()); + } + } else { + dirs.push_back(entry); + } } else { had_err = remove_file(entry.path(), options).bitor(had_err); } @@ -447,6 +470,10 @@ fn prompt_write_protected(path: &Path, is_dir: bool, options: &Options) -> bool } } +fn prompt_descend(path: &Path) -> bool { + prompt(&(format!("rm: descend into directory {}? ", path.quote()))) +} + fn prompt_file(path: &Path, is_dir: bool) -> bool { if is_dir { prompt(&(format!("rm: remove directory {}? ", path.quote()))) diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index 472d12bedee..b600c2090ff 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -353,6 +353,53 @@ fn test_rm_interactive_never() { assert!(!at.file_exists(file_2)); } +#[test] +fn test_rm_descend_directory() { + // This test descends into each directory and deletes the files and folders inside of them + // This test will have the rm process asks 6 question and us answering Y to them will delete all the files and folders + use std::io::Write; + use std::process::Child; + + // Needed for talking with stdin on platforms where CRLF or LF matters + const END_OF_LINE: &str = if cfg!(windows) { "\r\n" } else { "\n" }; + + let yes = format!("y{}", END_OF_LINE); + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let file_1 = "a/at.txt"; + let file_2 = "a/b/bt.txt"; + + at.mkdir_all("a/b/"); + at.touch(file_1); + at.touch(file_2); + + let mut child: Child = scene.ucmd().arg("-ri").arg("a").run_no_wait(); + + // Needed so that we can talk to the rm program + let mut child_stdin = child.stdin.take().unwrap(); + child_stdin.write_all(yes.as_bytes()).unwrap(); + child_stdin.flush().unwrap(); + child_stdin.write_all(yes.as_bytes()).unwrap(); + child_stdin.flush().unwrap(); + child_stdin.write_all(yes.as_bytes()).unwrap(); + child_stdin.flush().unwrap(); + child_stdin.write_all(yes.as_bytes()).unwrap(); + child_stdin.flush().unwrap(); + child_stdin.write_all(yes.as_bytes()).unwrap(); + child_stdin.flush().unwrap(); + child_stdin.write_all(yes.as_bytes()).unwrap(); + child_stdin.flush().unwrap(); + + child.wait_with_output().unwrap(); + + assert!(!at.dir_exists("a/b")); + assert!(!at.dir_exists("a")); + assert!(!at.file_exists(file_1)); + assert!(!at.file_exists(file_2)); +} + #[test] #[ignore = "issue #3722"] fn test_rm_directory_rights_rm1() { From 8bfd96fb598d276f37259f312df4e01061793bf3 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 25 Sep 2022 17:09:01 -0400 Subject: [PATCH 4/4] cp: correct error message on copying dir to itself Correct the error message produced when attempting to copy a directory into itself with `cp`. Before this commit, the error message was $ cp -R d d cp: cannot copy a directory, 'd', into itself, 'd' After this commit, the error message is $ cp -R d d cp: cannot copy a directory, 'd', into itself, 'd/d' --- src/uu/cp/src/cp.rs | 2 +- tests/by-util/test_cp.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index ef8fb93ec3b..4772a642472 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1092,7 +1092,7 @@ fn copy_directory( return Err(format!( "cannot copy a directory, {}, into itself, {}", root.quote(), - target.quote() + target.join(root.file_name().unwrap()).quote() ) .into()); } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 45b33157663..94bf17026d3 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -2095,3 +2095,31 @@ fn test_cp_mode_hardlink_no_dereference() { assert!(at.symlink_exists("z")); assert_eq!(at.read_symlink("z"), "slink"); } + +/// Test that copying a directory to itself is disallowed. +#[test] +fn test_copy_directory_to_itself_disallowed() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("d"); + #[cfg(not(windows))] + let expected = "cp: cannot copy a directory, 'd', into itself, 'd/d'"; + #[cfg(windows)] + let expected = "cp: cannot copy a directory, 'd', into itself, 'd\\d'"; + ucmd.args(&["-R", "d", "d"]).fails().stderr_only(expected); +} + +/// Test that copying a nested directory to itself is disallowed. +#[test] +fn test_copy_nested_directory_to_itself_disallowed() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("a"); + at.mkdir("a/b"); + at.mkdir("a/b/c"); + #[cfg(not(windows))] + let expected = "cp: cannot copy a directory, 'a/b', into itself, 'a/b/c/b'"; + #[cfg(windows)] + let expected = "cp: cannot copy a directory, 'a/b', into itself, 'a/b/c\\b'"; + ucmd.args(&["-R", "a/b", "a/b/c"]) + .fails() + .stderr_only(expected); +}