Merge branch 'main' into test_ls_fix

src/uu/chmod/src/chmod.rs

@@ -8,6 +8,7 @@
 // spell-checker:ignore (ToDO) Chmoder cmode fmode fperm fref ugoa RFILE RFILE's
 
 use clap::{crate_version, Arg, ArgAction, Command};
+use std::ffi::OsString;
 use std::fs;
 use std::os::unix::fs::{MetadataExt, PermissionsExt};
 use std::path::Path;
@@ -35,14 +36,64 @@ mod options {
     pub const FILE: &str = "FILE";
 }
 
+/// Extract negative modes (starting with '-') from the rest of the arguments.
+///
+/// This is mainly required for GNU compatibility, where "non-positional negative" modes are used
+/// as the actual positional MODE. Some examples of these cases are:
+/// * "chmod -w -r file", which is the same as "chmod -w,-r file"
+/// * "chmod -w file -r", which is the same as "chmod -w,-r file"
+///
+/// These can currently not be handled by clap.
+/// Therefore it might be possible that a pseudo MODE is inserted to pass clap parsing.
+/// The pseudo MODE is later replaced by the extracted (and joined) negative modes.
+fn extract_negative_modes(mut args: impl uucore::Args) -> (Option<String>, Vec<OsString>) {
+    // we look up the args until "--" is found
+    // "-mode" will be extracted into parsed_cmode_vec
+    let (parsed_cmode_vec, pre_double_hyphen_args): (Vec<OsString>, Vec<OsString>) =
+        args.by_ref().take_while(|a| a != "--").partition(|arg| {
+            let arg = if let Some(arg) = arg.to_str() {
+                arg.to_string()
+            } else {
+                return false;
+            };
+            arg.len() >= 2
+                && arg.starts_with('-')
+                && matches!(
+                    arg.chars().nth(1).unwrap(),
+                    'r' | 'w' | 'x' | 'X' | 's' | 't' | 'u' | 'g' | 'o' | '0'..='7'
+                )
+        });
+
+    let mut clean_args = Vec::new();
+    if !parsed_cmode_vec.is_empty() {
+        // we need a pseudo cmode for clap, which won't be used later.
+        // this is required because clap needs the default "chmod MODE FILE" scheme.
+        clean_args.push("w".into());
+    }
+    clean_args.extend(pre_double_hyphen_args);
+
+    if let Some(arg) = args.next() {
+        // as there is still something left in the iterator, we previously consumed the "--"
+        // -> add it to the args again
+        clean_args.push("--".into());
+        clean_args.push(arg);
+    }
+    clean_args.extend(args);
+
+    let parsed_cmode = Some(
+        parsed_cmode_vec
+            .iter()
+            .map(|s| s.to_str().unwrap())
+            .collect::<Vec<&str>>()
+            .join(","),
+    )
+    .filter(|s| !s.is_empty());
+    (parsed_cmode, clean_args)
+}
+
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-    let mut args = args.collect_lossy();
-
-    // Before we can parse 'args' with clap (and previously getopts),
-    // a possible MODE prefix '-' needs to be removed (e.g. "chmod -x FILE").
-    let mode_had_minus_prefix = mode::strip_minus_from_mode(&mut args);
-
+    let (parsed_cmode, args) = extract_negative_modes(args.skip(1)); // skip binary name
     let matches = uu_app().after_help(LONG_USAGE).try_get_matches_from(args)?;
 
     let changes = matches.get_flag(options::CHANGES);
@@ -62,13 +113,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         },
         None => None,
     };
-    let modes = matches.get_one::<String>(options::MODE).unwrap(); // should always be Some because required
-    let cmode = if mode_had_minus_prefix {
-        // clap parsing is finished, now put prefix back
-        format!("-{modes}")
+
+    let modes = matches.get_one::<String>(options::MODE);
+    let cmode = if let Some(parsed_cmode) = parsed_cmode {
+        parsed_cmode
     } else {
-        modes.to_string()
+        modes.unwrap().to_string() // modes is required
     };
+    // FIXME: enable non-utf8 paths
     let mut files: Vec<String> = matches
         .get_many::<String>(options::FILE)
         .map(|v| v.map(ToString::to_string).collect())
@@ -107,6 +159,7 @@ pub fn uu_app() -> Command {
         .override_usage(format_usage(USAGE))
         .args_override_self(true)
         .infer_long_args(true)
+        .no_binary_name(true)
         .arg(
             Arg::new(options::CHANGES)
                 .long(options::CHANGES)
@@ -376,3 +429,34 @@ impl Chmoder {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_extract_negative_modes() {
+        // "chmod -w -r file" becomes "chmod -w,-r file". clap does not accept "-w,-r" as MODE.
+        // Therefore, "w" is added as pseudo mode to pass clap.
+        let (c, a) = extract_negative_modes(vec!["-w", "-r", "file"].iter().map(OsString::from));
+        assert_eq!(c, Some("-w,-r".to_string()));
+        assert_eq!(a, vec!["w", "file"]);
+
+        // "chmod -w file -r" becomes "chmod -w,-r file". clap does not accept "-w,-r" as MODE.
+        // Therefore, "w" is added as pseudo mode to pass clap.
+        let (c, a) = extract_negative_modes(vec!["-w", "file", "-r"].iter().map(OsString::from));
+        assert_eq!(c, Some("-w,-r".to_string()));
+        assert_eq!(a, vec!["w", "file"]);
+
+        // "chmod -w -- -r file" becomes "chmod -w -r file", where "-r" is interpreted as file.
+        // Again, "w" is needed as pseudo mode.
+        let (c, a) = extract_negative_modes(vec!["-w", "--", "-r", "f"].iter().map(OsString::from));
+        assert_eq!(c, Some("-w".to_string()));
+        assert_eq!(a, vec!["w", "--", "-r", "f"]);
+
+        // "chmod -- -r file" becomes "chmod -r file".
+        let (c, a) = extract_negative_modes(vec!["--", "-r", "file"].iter().map(OsString::from));
+        assert_eq!(c, None);
+        assert_eq!(a, vec!["--", "-r", "file"]);
+    }
+}

src/uu/cksum/cksum.md

@@ -0,0 +1,23 @@
+# cksum
+
+```
+cksum [OPTIONS] [FILE]...
+```
+
+Print CRC and size for each file
+
+## After Help
+
+DIGEST determines the digest algorithm and default output format:
+
+- `-a=sysv`:    (equivalent to sum -s)
+- `-a=bsd`:     (equivalent to sum -r)
+- `-a=crc`:     (equivalent to cksum)
+- `-a=md5`:     (equivalent to md5sum)
+- `-a=sha1`:    (equivalent to sha1sum)
+- `-a=sha224`:  (equivalent to sha224sum)
+- `-a=sha256`:  (equivalent to sha256sum)
+- `-a=sha384`:  (equivalent to sha384sum)
+- `-a=sha512`:  (equivalent to sha512sum)
+- `-a=blake2b`: (equivalent to b2sum)
+- `-a=sm3`:     (only available through cksum)

src/uu/cksum/src/cksum.rs

@@ -15,15 +15,16 @@ use std::iter;
 use std::path::Path;
 use uucore::{
     error::{FromIo, UResult},
-    format_usage,
+    format_usage, help_about, help_section, help_usage,
     sum::{
         div_ceil, Blake2b, Digest, DigestWriter, Md5, Sha1, Sha224, Sha256, Sha384, Sha512, Sm3,
         BSD, CRC, SYSV,
     },
 };
 
-const USAGE: &str = "{} [OPTIONS] [FILE]...";
-const ABOUT: &str = "Print CRC and size for each file";
+const USAGE: &str = help_usage!("cksum.md");
+const ABOUT: &str = help_about!("cksum.md");
+const AFTER_HELP: &str = help_section!("after help", "cksum.md");
 
 const ALGORITHM_OPTIONS_SYSV: &str = "sysv";
 const ALGORITHM_OPTIONS_BSD: &str = "bsd";
@@ -205,21 +206,6 @@ mod options {
     pub static ALGORITHM: &str = "algorithm";
 }
 
-const ALGORITHM_HELP_DESC: &str =
-    "DIGEST determines the digest algorithm and default output format:\n\
-\n\
--a=sysv:    (equivalent to sum -s)\n\
--a=bsd:     (equivalent to sum -r)\n\
--a=crc:     (equivalent to cksum)\n\
--a=md5:     (equivalent to md5sum)\n\
--a=sha1:    (equivalent to sha1sum)\n\
--a=sha224:  (equivalent to sha224sum)\n\
--a=sha256:  (equivalent to sha256sum)\n\
--a=sha384:  (equivalent to sha384sum)\n\
--a=sha512:  (equivalent to sha512sum)\n\
--a=blake2b: (equivalent to b2sum)\n\
--a=sm3:     (only available through cksum)\n";
-
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let args = args.collect_ignore();
@@ -278,5 +264,5 @@ pub fn uu_app() -> Command {
                     ALGORITHM_OPTIONS_SM3,
                 ]),
         )
-        .after_help(ALGORITHM_HELP_DESC)
+        .after_help(AFTER_HELP)
 }

src/uu/date/src/date.rs

@@ -114,8 +114,8 @@ impl<'a> From<&'a str> for Iso8601Format {
             SECONDS | SECOND => Self::Seconds,
             NS => Self::Ns,
             DATE => Self::Date,
-            // Should be caught by clap
-            _ => panic!("Invalid format: {s}"),
+            // Note: This is caught by clap via `possible_values`
+            _ => unreachable!(),
         }
     }
 }
@@ -291,6 +291,9 @@ pub fn uu_app() -> Command {
                 .short('I')
                 .long(OPT_ISO_8601)
                 .value_name("FMT")
+                .value_parser([DATE, HOUR, HOURS, MINUTE, MINUTES, SECOND, SECONDS, NS])
+                .num_args(0..=1)
+                .default_missing_value(OPT_DATE)
                 .help(ISO_8601_HELP_STRING),
         )
         .arg(

src/uu/uptime/src/uptime.rs

@@ -11,17 +11,13 @@
 use chrono::{Local, TimeZone, Utc};
 use clap::{crate_version, Arg, ArgAction, Command};
 
-use uucore::format_usage;
-// import crate time from utmpx
-pub use uucore::libc;
 use uucore::libc::time_t;
+use uucore::{format_usage, help_about, help_usage};
 
 use uucore::error::{UResult, USimpleError};
 
-static ABOUT: &str = "Display the current time, the length of time the system has been up,\n\
-                      the number of users on the system, and the average number of jobs\n\
-                      in the run queue over the last 1, 5 and 15 minutes.";
-const USAGE: &str = "{} [OPTION]...";
+const ABOUT: &str = help_about!("uptime.md");
+const USAGE: &str = help_usage!("uptime.md");
 pub mod options {
     pub static SINCE: &str = "since";
 }

src/uu/uptime/uptime.md

@@ -0,0 +1,9 @@
+# uptime
+
+```
+uptime [OPTION]...
+```
+
+Display the current time, the length of time the system has been up,
+the number of users on the system, and the average number of jobs
+in the run queue over the last 1, 5 and 15 minutes.

src/uu/whoami/src/whoami.rs

@@ -11,10 +11,12 @@ use clap::{crate_version, Command};
 
 use uucore::display::println_verbatim;
 use uucore::error::{FromIo, UResult};
+use uucore::{format_usage, help_about, help_usage};
 
 mod platform;
 
-static ABOUT: &str = "Print the current username.";
+const ABOUT: &str = help_about!("whoami.md");
+const USAGE: &str = help_usage!("whoami.md");
 
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
@@ -28,5 +30,6 @@ pub fn uu_app() -> Command {
     Command::new(uucore::util_name())
         .version(crate_version!())
         .about(ABOUT)
+        .override_usage(format_usage(USAGE))
         .infer_long_args(true)
 }

src/uu/whoami/whoami.md

@@ -0,0 +1,7 @@
+# whoami
+
+```
+whoami
+```
+
+Print the current username.

src/uucore/src/lib/features/mode.rs

@@ -122,6 +122,15 @@ fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) {
             'o' => srwx = ((fperm << 6) & 0o700) | ((fperm << 3) & 0o070) | (fperm & 0o007),
             _ => break,
         };
+        if ch == 'u' || ch == 'g' || ch == 'o' {
+            // symbolic modes only allows perms to be a single letter of 'ugo'
+            // therefore this must either be the first char or it is unexpected
+            if pos != 0 {
+                break;
+            }
+            pos = 1;
+            break;
+        }
         pos += 1;
     }
     if pos == 0 {

tests/by-util/test_chmod.rs

@@ -4,9 +4,8 @@ use std::fs::{metadata, set_permissions, OpenOptions, Permissions};
 use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
 use std::sync::Mutex;
 
-extern crate libc;
-use uucore::mode::strip_minus_from_mode;
 extern crate chmod;
+extern crate libc;
 use self::libc::umask;
 
 static TEST_FILE: &str = "file";
@@ -503,35 +502,6 @@ fn test_chmod_symlink_non_existing_file_recursive() {
         .no_stderr();
 }
 
-#[test]
-fn test_chmod_strip_minus_from_mode() {
-    let tests = vec![
-        // ( before, after )
-        ("chmod -v -xw -R FILE", "chmod -v xw -R FILE"),
-        ("chmod g=rwx FILE -c", "chmod g=rwx FILE -c"),
-        (
-            "chmod -c -R -w,o+w FILE --preserve-root",
-            "chmod -c -R w,o+w FILE --preserve-root",
-        ),
-        ("chmod -c -R +w FILE ", "chmod -c -R +w FILE "),
-        ("chmod a=r,=xX FILE", "chmod a=r,=xX FILE"),
-        (
-            "chmod -v --reference REF_FILE -R FILE",
-            "chmod -v --reference REF_FILE -R FILE",
-        ),
-        ("chmod -Rvc -w-x FILE", "chmod -Rvc w-x FILE"),
-        ("chmod 755 -v FILE", "chmod 755 -v FILE"),
-        ("chmod -v +0004 FILE -R", "chmod -v +0004 FILE -R"),
-        ("chmod -v -0007 FILE -R", "chmod -v 0007 FILE -R"),
-    ];
-
-    for test in tests {
-        let mut args: Vec<String> = test.0.split(' ').map(|v| v.to_string()).collect();
-        let _mode_had_minus_prefix = strip_minus_from_mode(&mut args);
-        assert_eq!(test.1, args.join(" "));
-    }
-}
-
 #[test]
 fn test_chmod_keep_setgid() {
     for (from, arg, to) in [
@@ -671,3 +641,68 @@ fn test_quiet_n_verbose_used_multiple_times() {
         .arg("file")
         .succeeds();
 }
+
+#[test]
+fn test_gnu_invalid_mode() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.touch("file");
+    scene.ucmd().arg("u+gr").arg("file").fails();
+}
+
+#[test]
+fn test_gnu_options() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.touch("file");
+    scene.ucmd().arg("-w").arg("file").succeeds();
+    scene.ucmd().arg("file").arg("-w").succeeds();
+    scene.ucmd().arg("-w").arg("--").arg("file").succeeds();
+}
+
+#[test]
+fn test_gnu_repeating_options() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.touch("file");
+    scene.ucmd().arg("-w").arg("-w").arg("file").succeeds();
+    scene
+        .ucmd()
+        .arg("-w")
+        .arg("-w")
+        .arg("-w")
+        .arg("file")
+        .succeeds();
+}
+
+#[test]
+fn test_gnu_special_filenames() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    let perms_before = Permissions::from_mode(0o100640);
+    let perms_after = Permissions::from_mode(0o100440);
+
+    make_file(&at.plus_as_string("--"), perms_before.mode());
+    scene.ucmd().arg("-w").arg("--").arg("--").succeeds();
+    assert_eq!(at.metadata("--").permissions(), perms_after);
+    set_permissions(at.plus("--"), perms_before.clone()).unwrap();
+    scene.ucmd().arg("--").arg("-w").arg("--").succeeds();
+    assert_eq!(at.metadata("--").permissions(), perms_after);
+    at.remove("--");
+
+    make_file(&at.plus_as_string("-w"), perms_before.mode());
+    scene.ucmd().arg("-w").arg("--").arg("-w").succeeds();
+    assert_eq!(at.metadata("-w").permissions(), perms_after);
+    set_permissions(at.plus("-w"), perms_before).unwrap();
+    scene.ucmd().arg("--").arg("-w").arg("-w").succeeds();
+    assert_eq!(at.metadata("-w").permissions(), perms_after);
+}
+
+#[test]
+fn test_gnu_special_options() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.touch("file");
+    scene.ucmd().arg("--").arg("--").arg("file").succeeds();
+    scene.ucmd().arg("--").arg("--").fails();
+}

tests/by-util/test_date.rs

@@ -44,16 +44,84 @@ fn test_date_rfc_3339() {
 }
 
 #[test]
-fn test_date_rfc_8601() {
+fn test_date_rfc_8601_default() {
+    let re = Regex::new(r"^\d{4}-\d{2}-\d{2}\n$").unwrap();
     for param in ["--iso-8601", "--i"] {
-        new_ucmd!().arg(format!("{param}=ns")).succeeds();
+        new_ucmd!().arg(param).succeeds().stdout_matches(&re);
+    }
+}
+
+#[test]
+fn test_date_rfc_8601() {
+    let re = Regex::new(r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2},\d{9}[+-]\d{2}:\d{2}\n$").unwrap();
+    for param in ["--iso-8601", "--i"] {
+        new_ucmd!()
+            .arg(format!("{param}=ns"))
+            .succeeds()
+            .stdout_matches(&re);
+    }
+}
+
+#[test]
+fn test_date_rfc_8601_invalid_arg() {
+    for param in ["--iso-8601", "--i"] {
+        new_ucmd!().arg(format!("{param}=@")).fails();
     }
 }
 
 #[test]
 fn test_date_rfc_8601_second() {
+    let re = Regex::new(r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}[+-]\d{2}:\d{2}\n$").unwrap();
     for param in ["--iso-8601", "--i"] {
-        new_ucmd!().arg(format!("{param}=second")).succeeds();
+        new_ucmd!()
+            .arg(format!("{param}=second"))
+            .succeeds()
+            .stdout_matches(&re);
+        new_ucmd!()
+            .arg(format!("{param}=seconds"))
+            .succeeds()
+            .stdout_matches(&re);
+    }
+}
+
+#[test]
+fn test_date_rfc_8601_minute() {
+    let re = Regex::new(r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}[+-]\d{2}:\d{2}\n$").unwrap();
+    for param in ["--iso-8601", "--i"] {
+        new_ucmd!()
+            .arg(format!("{param}=minute"))
+            .succeeds()
+            .stdout_matches(&re);
+        new_ucmd!()
+            .arg(format!("{param}=minutes"))
+            .succeeds()
+            .stdout_matches(&re);
+    }
+}
+
+#[test]
+fn test_date_rfc_8601_hour() {
+    let re = Regex::new(r"^\d{4}-\d{2}-\d{2}T\d{2}[+-]\d{2}:\d{2}\n$").unwrap();
+    for param in ["--iso-8601", "--i"] {
+        new_ucmd!()
+            .arg(format!("{param}=hour"))
+            .succeeds()
+            .stdout_matches(&re);
+        new_ucmd!()
+            .arg(format!("{param}=hours"))
+            .succeeds()
+            .stdout_matches(&re);
+    }
+}
+
+#[test]
+fn test_date_rfc_8601_date() {
+    let re = Regex::new(r"^\d{4}-\d{2}-\d{2}\n$").unwrap();
+    for param in ["--iso-8601", "--i"] {
+        new_ucmd!()
+            .arg(format!("{param}=date"))
+            .succeeds()
+            .stdout_matches(&re);
     }
 }
 

tests/by-util/test_shred.rs

@@ -25,7 +25,6 @@ fn test_shred_remove() {
     assert!(at.file_exists(file_b));
 }
 
-#[cfg(not(target_os = "freebsd"))]
 #[test]
 fn test_shred_force() {
     let scene = TestScenario::new(util_name!());