Skip to content

Conversation

@RenjiSann
Copy link
Collaborator

This PR drops the hashsum binary, that was the way for uutils to implement standalone checksum algorithms, to introduce specific binaries.

I chose to regroup all the common CLI handling into a cli submodule under the uucore::checksum feature, that's debatable, as it introduces a clap dependency in uucore, which.

Another way I can see it would be to replicate the way base32, base64 and basenc work, where all the common implementation lives under base32, and the other utils depend from it, but I'm not really convinced with this.

Let's discuss it !

@oech3
Copy link
Contributor

oech3 commented Dec 22, 2025

Do you want to avoid ln -s cksum md5sum (and dropping ln -s coreutils md5sum)?

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/tty/tty-eof. tests/tty/tty-eof is passing on 'main'. Maybe you have to rebase?

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 22, 2025

CodSpeed Performance Report

Merging #9776 will degrade performance by 16.5%

Comparing RenjiSann:checksum-common-cli (9f2c7fb) with main (8d3774b)

Summary

❌ 13 regressions
✅ 114 untouched
⏩ 6 skipped1

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

Benchmarks breakdown

Benchmark BASE HEAD Efficiency
factor_multiple_u64s[2] 212.5 ms 254.5 ms -16.5%
sort_ascii_c_locale 21.5 ms 22.6 ms -4.73%
split_bytes 509 µs 529.4 µs -3.85%
split_number_chunks 293.8 µs 316.3 µs -7.13%
wc_bytes_synthetic[500] 181.5 µs 205.2 µs -11.56%
b64_encode_synthetic 161.9 µs 184.6 µs -12.28%
b64_decode_synthetic 166.4 µs 188.8 µs -11.88%
b64_decode_ignore_garbage_synthetic 163.3 µs 185 µs -11.75%
mv_force_overwrite 144.3 ms 160.2 ms -9.93%
mv_single_file 135.5 ms 160 ms -15.31%
cksum_blake3 206.4 µs 236.9 µs -12.85%
rm_single_file 119.4 ms 135 ms -11.57%
cp_large_file[16] 361.5 µs 387.4 µs -6.69%

Footnotes

  1. 6 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.

@RenjiSann
Copy link
Collaborator Author

Do you want to avoid ln -s cksum md5sum (and dropping ln -s coreutils md5sum)?

No, in my mind, we have separate binaries for cksum, b2sum, md5sum, etc...
That being said, I thought about having a single directory in src/uu, with a single main file which we could make several binaries from it (and its behavior would change depending on its name, but that would play poorly with our current testing system)

basename \
cat \
cksum \
b2sum \
Copy link
Contributor

Choose a reason for hiding this comment

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

Why b2sum only? Just forgot other bins?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No no, it's the reason it's still in draft

Copy link
Contributor

Choose a reason for hiding this comment

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

hasusum.rs is still remain. But we remove it later and totail size of source code is reduced. Right?

@oech3
Copy link
Contributor

oech3 commented Dec 22, 2025

We have

# Create *sum binaries
for sum in b2sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum; do
sum_path="${UU_BUILD_DIR}/${sum}"
test -f "${sum_path}" || (cd ${UU_BUILD_DIR} && ln -s "hashsum" "${sum}")
done
test -f "${UU_BUILD_DIR}/[" || (cd ${UU_BUILD_DIR} && ln -s "test" "[")
too.
(I have a PR to use coreutils for faster CI at GnuTests to remove it, but not merged.)

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/tty/tty-eof. tests/tty/tty-eof is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/tail/inotify-dir-recreate is now passing!

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this .png needed (instead of symlink to logo)?

@oech3
Copy link
Contributor

oech3 commented Dec 23, 2025

#9797 OK before this?

@oech3
Copy link
Contributor

oech3 commented Dec 24, 2025

I think 6 PR (each bins) for this is OK.

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.

2 participants