Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sylvestre merged 1 commit into from
Feb 20, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/uu/checksum_common/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ impl ChecksumCommand for Command {
.long(options::BINARY)
.short('b')
.hide(true)
.overrides_with(options::TEXT)
.action(ArgAction::SetTrue),
)
}
Expand Down Expand Up @@ -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),
)
}
Expand Down
29 changes: 20 additions & 9 deletions src/uu/checksum_common/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,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);
Copy link
Collaborator

@ChrisDryden ChrisDryden Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to brainstorm to clean this code up and I'm wondering if its better to just pass the matches in the OutputFormat?

Then you can also move the error handling validation into there and it will make the code reduce by a bunch


checksum_main(algo, length, matches, format?)
}
Expand All @@ -83,8 +86,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?)
}
Expand Down Expand Up @@ -155,24 +160,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(Borrow::borrow);

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 {
Copy link
Collaborator

@ChrisDryden ChrisDryden Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this check is invalid to have in the checksum_common library since it only applies to cksum and not the other ones like md5sum and sha256sum.

When running on the 9.10 version I'm finding that this is valid:

sha256sum --text --check file

return Err(ChecksumError::BinaryTextConflict.into());
}

Expand Down
51 changes: 7 additions & 44 deletions src/uu/cksum/src/cksum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

// spell-checker:ignore (ToDO) fname, algo, bitlen

use std::ffi::OsStr;
use std::io::{Write, stderr};

use clap::Command;
Expand Down Expand Up @@ -44,48 +43,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>,
Expand Down Expand Up @@ -134,8 +91,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),
Expand Down
25 changes: 3 additions & 22 deletions src/uucore/src/lib/features/checksum/compute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -115,27 +115,8 @@ 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;
}
}

#[allow(clippy::unnecessary_wraps)]
pub fn from_standalone(text: bool, tag: bool) -> UResult<Self> {
if tag {
Ok(Self::Tagged(DigestFormat::Hexadecimal))
} else {
Expand Down
2 changes: 2 additions & 0 deletions src/uucore/src/lib/features/checksum/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,8 @@ pub enum ChecksumError {
BinaryTextConflict,
#[error("--text mode is only supported with --untagged")]
TextWithoutUntagged,
#[error("the --tag option is meaningless when verifying checksums")]
Copy link
Contributor

@sylvestre sylvestre Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be done a different pr but it will need translate!() at some point

TagCheck,
#[error("--tag does not support --text mode")]
TextAfterTag,
#[error("--check is not supported with --algorithm={{bsd,sysv,crc,crc32b}}")]
Expand Down
17 changes: 6 additions & 11 deletions tests/by-util/test_cksum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,13 +1012,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]
Expand All @@ -1040,7 +1040,6 @@ fn test_reset_binary_but_set() {
.stdout_contains("d41d8cd98f00b204e9800998ecf8427e *");
}

/// Test legacy behaviors with --tag, --untagged, --binary and --text
mod output_format {
use super::*;

Expand All @@ -1049,13 +1048,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]
Expand Down Expand Up @@ -1181,7 +1178,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]
Expand Down Expand Up @@ -1229,9 +1226,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]
Expand Down
Loading