Skip to content
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.

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

cksum: Update error and flags handling to improver GNU's match #7192

Merged
merged 5 commits into from
Jan 23, 2025

Conversation

RenjiSann
Copy link
Contributor

This PR does 4 things:

  • Move the printing of improperly formatted inline warning to the process_checksum_line loop instead of only printing in the case of a problem of algorithm parsing (necessary for passing tests for --warn)
  • Introduce another rather shitty variable for "caching" the algorithm that was used in the previous checksum line, so we can use it to print a warning message for a potentially incorrect line after.

Example:

CHECKSUM.txt:
  SM3 (foo.txt) = xxxxxxxxxxx
  an invalid line
cksum -c CHECKSUM.txt --warn
foo.txt: OK
CHECKSUM.txt: 2: improperly formatted SM3 checksum line      <-- SM3 is from the previous line
  • Change the way the --status, --quiet and --warn flags interact together. In GNU, these flags supersedes each other to set a specific verbose level, while our implementation was just setting flags. Now, our implementation is using a verbose logic as well (STATUS, QUIET, NORMAL, WARN), and the last flag passed on CLi will override the others.

  • Fix a bug which prevented cksum to return with an error exit code upon a failing checksum in case --ignore-missing was passed.

In addition to this, I have ported the gnu test cksum/cksum-c.sh directly in the rust testsuite

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/cksum/cksum-c is no longer failing!
Congrats! The gnu test tests/misc/stdbuf is no longer failing!

@RenjiSann RenjiSann force-pushed the cksum-fix-error-handling branch from 8b92671 to 3120b16 Compare January 22, 2025 09:39
@RenjiSann
Copy link
Contributor Author

Force pushed style fixes (diff)


let verbose = {
use ChecksumVerbose::*;
match (status, quiet, warn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is great and fun :)


let verbose = {
use ChecksumVerbose::*;
match (status, quiet, warn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe create a function for this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed ! Done 👍

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/cksum/cksum-c is no longer failing!
Congrats! The gnu test tests/misc/stdbuf is no longer failing!

@RenjiSann RenjiSann force-pushed the cksum-fix-error-handling branch from 3120b16 to ac72268 Compare January 22, 2025 10:26
@RenjiSann
Copy link
Contributor Author

Move ChecksumVerrbose variable creation to a function (diff)

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/cksum/cksum-c is no longer failing!
Congrats! The gnu test tests/misc/stdbuf is no longer failing!

@RenjiSann RenjiSann force-pushed the cksum-fix-error-handling branch from ac72268 to 1900339 Compare January 22, 2025 21:48
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cksum/cksum-c is no longer failing!
Skipping an intermittent issue tests/misc/usage_vs_getopt (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@RenjiSann RenjiSann force-pushed the cksum-fix-error-handling branch from 1900339 to 84bbd05 Compare January 23, 2025 16:04
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/cksum/cksum-c is no longer failing!
Skipping an intermittent issue tests/misc/usage_vs_getopt (passes in this run but fails in the 'main' branch)

@RenjiSann
Copy link
Contributor Author

@sylvestre ready for review when you have some time 👍

@sylvestre sylvestre merged commit 5129aba into uutils:main Jan 23, 2025
65 checks passed
@sylvestre
Copy link
Contributor

thanks :)

@RenjiSann
Copy link
Contributor Author

And that's 100% passed GNU tests for cksum 🥳

@cakebaker
Copy link
Contributor

@RenjiSann well done :)

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.

3 participants