Skip to content

cksum: add CRC32B algorithm added in GNU 9.6#7169

Merged
sylvestre merged 1 commit intouutils:mainfrom
RenjiSann:cksum-9.6-fixes
Jan 20, 2025
Merged

cksum: add CRC32B algorithm added in GNU 9.6#7169
sylvestre merged 1 commit intouutils:mainfrom
RenjiSann:cksum-9.6-fixes

Conversation

@RenjiSann
Copy link
Collaborator

@RenjiSann RenjiSann commented Jan 19, 2025

This is still a draft, but I'll try to fix all the now failing tests of cksum

@github-actions
Copy link

github-actions bot commented Jan 19, 2025

GNU testsuite comparison:

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

@RenjiSann RenjiSann force-pushed the cksum-9.6-fixes branch 3 times, most recently from 3d9755d to f89eb8b Compare January 20, 2025 00:24
@RenjiSann
Copy link
Collaborator Author

RenjiSann commented Jan 20, 2025

I have added a dependency to crc32fast for handling the new algorithm. It is very popular (200M downloads on crates.io) so it is very unlikely that we will have to put effort into maintaining it.

@RenjiSann
Copy link
Collaborator Author

RenjiSann commented Jan 20, 2025

The other test (cksum/cksum-c.sh) will be less trivial to fix, as the test know reveals flaws in the way we handle CLI options.

@github-actions
Copy link

github-actions bot commented Jan 20, 2025

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 is no longer failing!
Congrats! The gnu test tests/cksum/cksum-base64 is no longer failing!

@sylvestre
Copy link
Contributor

sylvestre commented Jan 20, 2025

nice
we don't have to fix all the issues at once. maybe we can merge what you have done here

@RenjiSann
Copy link
Collaborator Author

RenjiSann commented Jan 20, 2025

fix format (diff)

@RenjiSann
Copy link
Collaborator Author

RenjiSann commented Jan 20, 2025

we don't have to fix all the issues at once. maybe we can merge what you have done here
Sure ! Let's merge this already :)

@RenjiSann RenjiSann changed the title Fix cksum for GNU 9.6 cksum: add CRC32B algorithm added in GNU 9.6 Jan 20, 2025
sylvestre
sylvestre reviewed Jan 20, 2025
View reviewed changes
tests/by-util/test_cksum.rs Outdated
let scene = make_scene();

for (algo, digest) in PAIRS {
log_info(format!("ALGORITHM"), algo);
Copy link
Contributor

@sylvestre sylvestre Jan 20, 2025

Choose a reason for hiding this comment

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

Suggested change
log_info(format!("ALGORITHM"), algo);
log_info("ALGORITHM".to_string());

Copy link
Collaborator Author

@RenjiSann RenjiSann Jan 20, 2025

Choose a reason for hiding this comment

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

Yup, missed this 👍

@RenjiSann
Copy link
Collaborator Author

RenjiSann commented Jan 20, 2025

rebase on main

@github-actions
Copy link

github-actions bot commented Jan 20, 2025

GNU testsuite comparison:

GNU test failed: tests/misc/stdbuf. tests/misc/stdbuf is passing on 'main'. Maybe you have to rebase?
Congrats! The gnu test tests/cksum/cksum is no longer failing!
Congrats! The gnu test tests/cksum/cksum-base64 is no longer failing!
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

sylvestre commented Jan 20, 2025

Is it ready? Asking as it is still marked as draft

@RenjiSann RenjiSann marked this pull request as ready for review January 20, 2025 14:12
@RenjiSann
Copy link
Collaborator Author

RenjiSann commented Jan 20, 2025

Is it ready? Asking as it is still marked as draft

Yes, I was waiting for the tests to finish

@sylvestre sylvestre merged commit 3513ee6 into uutils:main Jan 20, 2025
64 of 65 checks passed
@RenjiSann RenjiSann deleted the cksum-9.6-fixes branch January 29, 2025 15:22
@moonfruit moonfruit mentioned this pull request Mar 10, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sylvestre sylvestre sylvestre left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@RenjiSann @sylvestre