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

@@ -154,23 +154,26 @@ 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 {
             return Err(ChecksumError::BinaryTextConflict.into());
         }

src/uu/cksum/src/cksum.rs

@@ -5,8 +5,6 @@
 
 // spell-checker:ignore (ToDO) fname, algo, bitlen
 
-use std::ffi::OsStr;
-
 use clap::Command;
 use uu_checksum_common::{ChecksumCommand, checksum_main, default_checksum_app, options};
 
@@ -42,48 +40,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>,
@@ -122,13 +78,18 @@ 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 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),
         tag,
-        binary,
+        !text,
         /* raw */ matches.get_flag(options::RAW),
         /* base64 */ matches.get_flag(options::BASE64),
     );

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

@@ -144,6 +144,7 @@ impl OutputFormat {
         let mut text = true;
         let mut tag = false;
 
+        // todo: switch to clap's matches.get_flag
         for arg in args {
             if arg == "--" {
                 break;
@@ -153,10 +154,6 @@ impl OutputFormat {
             } 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;
             }
         }

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]

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
+curl -L ${repo}/raw/refs/heads/master/tests/cksum/cksum-a.sh > tests/cksum/cksum-a.sh
+curl -L ${repo}/raw/refs/heads/master/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