cksum family: Backport new errors for --binary, --text and --tag

Author: oech3

src/uu/checksum_common/src/cli.rs

@@ -131,6 +131,7 @@ impl ChecksumCommand for Command {
                 .long(options::BINARY)
                 .short('b')
                 .hide(true)
+                .overrides_with(options::TEXT)
                 .action(ArgAction::SetTrue),
         )
     }
@@ -169,6 +170,7 @@ impl ChecksumCommand for Command {
             Arg::new(options::UNTAGGED)
                 .long(options::UNTAGGED)
                 .help(translate!("ck-common-help-untagged"))
+                .overrides_with(options::TAG)
                 .action(ArgAction::SetTrue),
         )
     }

src/uu/checksum_common/src/lib.rs

@@ -73,7 +73,10 @@ pub fn standalone_with_length_main(
         .transpose()?
         .flatten();
 
-    let format = OutputFormat::from_standalone(std::env::args_os());
+    //todo: deduplicate matches.get_flag
+    let text = !matches.get_flag(options::BINARY);
+    let tag = matches.get_flag(options::TAG);
+    let format = OutputFormat::from_standalone(text, tag);
 
     checksum_main(algo, length, matches, format?)
 }
@@ -82,8 +85,10 @@ pub fn standalone_with_length_main(
 pub fn standalone_main(algo: AlgoKind, cmd: Command, args: impl uucore::Args) -> UResult<()> {
     let matches = uucore::clap_localization::handle_clap_result(cmd, args)?;
     let algo = Some(algo);
-
-    let format = OutputFormat::from_standalone(std::env::args_os());
+    //todo: deduplicate matches.get_flag
+    let text = !matches.get_flag(options::BINARY);
+    let tag = matches.get_flag(options::TAG);
+    let format = OutputFormat::from_standalone(text, tag);
 
     checksum_main(algo, None, matches, format?)
 }
@@ -154,24 +159,30 @@ pub fn checksum_main(
     let quiet = check_flag("quiet")?;
     let strict = check_flag("strict")?;
     let status = check_flag("status")?;
+    let text_flag = matches.get_flag(options::TEXT);
+    let binary_flag = matches.get_flag(options::BINARY);
+    let tag = matches.get_flag(options::TAG);
 
     // clap provides the default value -. So we unwrap() safety.
     let files = matches
         .get_many::<OsString>(options::FILE)
         .unwrap()
         .map(|s| s.as_os_str());
 
+    if text_flag && tag {
+        return Err(ChecksumError::TextAfterTag.into());
+    }
+
     if check {
         // cksum does not support '--check'ing legacy algorithms
         if algo.is_some_and(AlgoKind::is_legacy) {
             return Err(ChecksumError::AlgorithmNotSupportedWithCheck.into());
         }
-
-        let text_flag = matches.get_flag(options::TEXT);
-        let binary_flag = matches.get_flag(options::BINARY);
-        let tag = matches.get_flag(options::TAG);
-
-        if tag || binary_flag || text_flag {
+        // Maybe, we should just use clap
+        if tag {
+            return Err(ChecksumError::TagCheck.into());
+        }
+        if binary_flag || text_flag {
             return Err(ChecksumError::BinaryTextConflict.into());
         }
 

src/uu/cksum/src/cksum.rs

@@ -44,48 +44,6 @@ fn print_cpu_debug_info() {
     }
 }
 
-/// cksum has a bunch of legacy behavior. We handle this in this function to
-/// make sure they are self contained and "easier" to understand.
-///
-/// Returns a pair of boolean. The first one indicates if we should use tagged
-/// output format, the second one indicates if we should use the binary flag in
-/// the untagged case.
-fn handle_tag_text_binary_flags<S: AsRef<OsStr>>(
-    args: impl Iterator<Item = S>,
-) -> UResult<(bool, bool)> {
-    let mut tag = true;
-    let mut binary = false;
-    let mut text = false;
-
-    // --binary, --tag and --untagged are tight together: none of them
-    // conflicts with each other but --tag will reset "binary" and "text" and
-    // set "tag".
-
-    for arg in args {
-        let arg = arg.as_ref();
-        if arg == "-b" || arg == "--binary" {
-            text = false;
-            binary = true;
-        } else if arg == "--text" {
-            text = true;
-            binary = false;
-        } else if arg == "--tag" {
-            tag = true;
-            binary = false;
-            text = false;
-        } else if arg == "--untagged" {
-            tag = false;
-        }
-    }
-
-    // Specifying --text without ever mentioning --untagged fails.
-    if text && tag {
-        return Err(ChecksumError::TextWithoutUntagged.into());
-    }
-
-    Ok((tag, binary))
-}
-
 /// Sanitize the `--length` argument depending on `--algorithm` and `--length`.
 fn maybe_sanitize_length(
     algo_cli: Option<AlgoKind>,
@@ -124,8 +82,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         .map(String::as_str);
 
     let length = maybe_sanitize_length(algo_cli, input_length)?;
+    let tag = !matches.get_flag(options::UNTAGGED);
+    let binary = matches.get_flag(options::BINARY);
+    let text = matches.get_flag(options::TEXT);
 
-    let (tag, binary) = handle_tag_text_binary_flags(std::env::args_os())?;
+    //Specifying --text without ever mentioning --untagged fails.
+    if text && tag {
+        return Err(ChecksumError::TextWithoutUntagged.into());
+    }
 
     let output_format = OutputFormat::from_cksum(
         algo_cli.unwrap_or(AlgoKind::Crc),

src/uucore/src/lib/features/checksum/compute.rs

@@ -5,7 +5,7 @@
 
 // spell-checker:ignore bitlen
 
-use std::ffi::{OsStr, OsString};
+use std::ffi::OsStr;
 use std::fs::File;
 use std::io::{self, BufReader, Read, Write};
 use std::path::Path;
@@ -140,27 +140,7 @@ impl OutputFormat {
     ///
     /// Since standalone utils can't use the Raw or Legacy output format, it is
     /// decided only using the --tag, --binary and --text arguments.
-    pub fn from_standalone(args: impl Iterator<Item = OsString>) -> UResult<Self> {
-        let mut text = true;
-        let mut tag = false;
-
-        for arg in args {
-            if arg == "--" {
-                break;
-            } else if arg == "--tag" {
-                tag = true;
-                text = false;
-            } else if arg == "--binary" || arg == "-b" {
-                text = false;
-            } else if arg == "--text" || arg == "-t" {
-                // Finding a `--text` after `--tag` is an error.
-                if tag {
-                    return Err(ChecksumError::TextAfterTag.into());
-                }
-                text = true;
-            }
-        }
-
+    pub fn from_standalone(text: bool, tag: bool) -> UResult<Self> {
         if tag {
             Ok(Self::Tagged(DigestFormat::Hexadecimal))
         } else {

src/uucore/src/lib/features/checksum/mod.rs

@@ -397,6 +397,8 @@ pub enum ChecksumError {
     BinaryTextConflict,
     #[error("--text mode is only supported with --untagged")]
     TextWithoutUntagged,
+    #[error("the --tag option is meaningless when verifying checksums")]
+    TagCheck,
     #[error("--tag does not support --text mode")]
     TextAfterTag,
     #[error("--check is not supported with --algorithm={{bsd,sysv,crc,crc32b}}")]

tests/by-util/test_cksum.rs

@@ -1010,13 +1010,13 @@ fn test_reset_binary() {
 
     scene
         .ucmd()
-        .arg("--binary") // should disappear because of the following option
+        .arg("--binary")
         .arg("--tag")
         .arg("--untagged")
         .arg("--algorithm=md5")
         .arg(at.subdir.join("f"))
         .succeeds()
-        .stdout_contains("d41d8cd98f00b204e9800998ecf8427e  ");
+        .stdout_contains("d41d8cd98f00b204e9800998ecf8427e *");
 }
 
 #[test]
@@ -1038,7 +1038,6 @@ fn test_reset_binary_but_set() {
         .stdout_contains("d41d8cd98f00b204e9800998ecf8427e *");
 }
 
-/// Test legacy behaviors with --tag, --untagged, --binary and --text
 mod output_format {
     use super::*;
 
@@ -1047,13 +1046,11 @@ mod output_format {
         let (at, mut ucmd) = at_and_ucmd!();
         at.touch("f");
 
-        ucmd.arg("--text") // should disappear because of the following option
+        ucmd.arg("--text")
             .arg("--tag")
             .args(&["-a", "md5"])
             .arg(at.subdir.join("f"))
-            .succeeds()
-            // Tagged output is used
-            .stdout_contains("f) = d41d8cd98f00b204e9800998ecf8427e");
+            .fails();
     }
 
     #[test]
@@ -1179,7 +1176,7 @@ fn test_binary_file() {
         .arg("--untagged")
         .arg("lorem_ipsum.txt")
         .succeeds()
-        .stdout_is("cd724690f7dc61775dfac400a71f2caa  lorem_ipsum.txt\n");
+        .stdout_is("cd724690f7dc61775dfac400a71f2caa *lorem_ipsum.txt\n");
 }
 
 #[test]
@@ -1227,9 +1224,7 @@ fn test_conflicting_options() {
         .arg("md5")
         .fails_with_code(1)
         .no_stdout()
-        .stderr_contains(
-            "cksum: the --binary and --text options are meaningless when verifying checksums",
-        );
+        .stderr_contains("cksum: the --tag option is meaningless when verifying checksums");
 }
 
 #[test]

util/fetch-gnu.sh

@@ -18,6 +18,9 @@ curl -L ${repo}/raw/refs/heads/master/tests/nproc/nproc-quota-systemd.sh > tests
 curl -L ${repo}/raw/refs/heads/master/tests/stty/bad-speed.sh > tests/stty/bad-speed.sh
 # Better support for single binary
 curl -L ${repo}/raw/refs/heads/master/tests/env/env.sh > tests/env/env.sh
+# Accurate errors on *sum with --binary --text and --tag. Freeze commit to avoid regression.
+curl -L ${repo}/raw/refs/heads/master/tests/cksum/cksum-a.sh > tests/cksum/cksum-a.sh
+curl -L https://raw.githubusercontent.com/coreutils/coreutils/9200747f65e3523f7b48184a331eec5a1225b435/tests/cksum/cksum-c.sh > tests/cksum/cksum-c.sh
 # Avoid incorrect PASS
 curl -L ${repo}/raw/refs/heads/master/tests/runcon/runcon-compute.sh > tests/runcon/runcon-compute.sh
 curl -L ${repo}/raw/refs/heads/master/tests/tac/tac-continue.sh > tests/tac/tac-continue.sh