Skip to content

cksum: Fix -a blake3/shake*#10531

Closed
oech3 wants to merge 2 commits intouutils:mainfrom
oech3:ckb3
Closed

cksum: Fix -a blake3/shake*#10531
oech3 wants to merge 2 commits intouutils:mainfrom
oech3:ckb3

Conversation

@oech3
Copy link
Contributor

@oech3 oech3 commented Jan 28, 2026

I really want remove them completely since it has a maintainance cost, but broken cksum is one of a blocker for hashsum deletion. So I fix it.

Closes #10002

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 28, 2026

Merging this PR will degrade performance by 99.77%

❌ 1 regressed benchmark
✅ 283 untouched benchmarks
⏩ 38 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation cksum_blake3 215.7 µs 93,209.5 µs -99.77%

Comparing oech3:ckb3 (061f3f3) with main (da7ad84)

Open in CodSpeed

Footnotes

  1. 38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@oech3
Copy link
Contributor Author

oech3 commented Jan 28, 2026

Note that we are benchmarking a bunch of errors currently. So regression is not happening.

@github-actions
Copy link

github-actions bot commented Feb 1, 2026

GNU testsuite comparison:

GNU test failed: tests/ls/abmon-align. tests/ls/abmon-align is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/tty/tty-eof (fails in this run but passes in the 'main' branch)

@oech3
Copy link
Contributor Author

oech3 commented Feb 2, 2026

ok?

@RenjiSann
Copy link
Collaborator

Can you add some tests for this in test_cksum.rs ?

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

GNU testsuite comparison:

GNU test failed: tests/cp/nfs-removal-race. tests/cp/nfs-removal-race is passing on 'main'. Maybe you have to rebase?
Note: The gnu test tests/runcon/runcon-compute is now being skipped but was previously passing.

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

GNU testsuite comparison:

Congrats! The gnu test tests/printf/printf-surprise is now passing!

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

GNU testsuite comparison:

GNU test failed: tests/tail/symlink. tests/tail/symlink is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/shuf/shuf-reservoir (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/sort/sort-stale-thread-mem (fails in this run but passes in the 'main' branch)

@oech3 oech3 force-pushed the ckb3 branch 2 times, most recently from 40b17ea to df80fca Compare February 5, 2026 12:01
@oech3
Copy link
Contributor Author

oech3 commented Feb 5, 2026

Typo...

@oech3
Copy link
Contributor Author

oech3 commented Feb 5, 2026

Failing test is tac. So this PR is ready.

@RenjiSann
Copy link
Collaborator

Is there a reason why we require a --length argument for shake* algorithms, even though it's not used to compute the digest underneath ?

@oech3
Copy link
Contributor Author

oech3 commented Feb 6, 2026

shake* is XOF. Does not our cksum support it?

@RenjiSann
Copy link
Collaborator

shake* is XOF. Does not our cksum support it?

Well I fear it does not, see

impl_digest_shake!(Shake128, 256);
impl_digest_shake!(Shake256, 512);

@oech3
Copy link
Contributor Author

oech3 commented Feb 6, 2026

Oops... We can leave frozen length as a bug and split PR...

@oech3
Copy link
Contributor Author

oech3 commented Feb 6, 2026

Without adding code, I have

$ target/debug/cksum -a shake128 /dev/null
cksum: --length required for 'SHAKE128'
$ target/debug/cksum -a shake128 -l 128 /dev/null
cksum: --length is only supported with --algorithm blake2b, sha2, or sha3

@github-actions
Copy link

github-actions bot commented Feb 6, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/usage_vs_getopt (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

@github-actions
Copy link

github-actions bot commented Feb 6, 2026

GNU testsuite comparison:

Congrats! The gnu test tests/factor/t26 is no longer failing!

@oech3
Copy link
Contributor Author

oech3 commented Feb 7, 2026

@RenjiSann OK to close this PR as replaced?

@RenjiSann
Copy link
Collaborator

It is still relevant for blake3, but we can do it somewhere else, yes 👍

@oech3 oech3 closed this Feb 7, 2026
@oech3 oech3 deleted the ckb3 branch February 7, 2026 14:30
@oech3
Copy link
Contributor Author

oech3 commented Feb 7, 2026

Personally, I think non-GNU (reference) XOF function is unnecessary complexity for us. It is really unclear how to write tags and determine (default) length...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cksum: unknown algorithm: blake3: clap should have prevented this case

3 participants