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

Open
oech3 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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 @@ -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
Copy link
Contributor Author

@oech3 oech3 Jan 31, 2026

Choose a reason for hiding this comment

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

@RenjiSann

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?)
}
Expand All @@ -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?)
}
Expand Down Expand Up @@ -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());
}

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 @@ -124,8 +81,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
24 changes: 2 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 @@ -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 {
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 @@ -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}}")]
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 @@ -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]
Expand All @@ -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::*;

Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
3 changes: 3 additions & 0 deletions util/fetch-gnu.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading