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

Fix the checksum type check #4164

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Jan 6, 2023

The None case was missed and due to the unrestricted tuple elem_type it may return valid for actually invalid entries.
So restrict that beeing overly cautious so it may wrongly return invalid.
But in that case the conversion function will be called which can do more elaborate verification.

Add test checking for None in checksums.

Extracted from #4159, see #4159 (comment)

Also required for #4142 as the check now correctly handles a None value in the dict

Requires resolution of

@Flamefire
Copy link
Contributor Author

Converted to draft to first specify how checksums especially "None" should work

@Flamefire Flamefire force-pushed the fix-checksums-verification branch 2 times, most recently from 54d9628 to 9b018f3 Compare May 12, 2023 07:20
@boegel boegel modified the milestones: 4.7.3, release after 4.7.3 Jul 5, 2023
@Flamefire Flamefire marked this pull request as ready for review November 15, 2023 19:00
@Flamefire
Copy link
Contributor Author

Specifying as ready as the inability to specify alternate checksums in a dict blocks e.g. easybuilders/easybuild-easyconfigs#19231

@easybuilders easybuilders deleted a comment from boegelbot Mar 11, 2024
@boegel boegel modified the milestones: 4.9.1, release after 4.9.1 Apr 3, 2024
The `None` case was missed and due to the unrestricted `tuple` elem_type
it may return valid for actually invalid entries.
So restrict that beeing overly cautious so it may wrongly return invalid.
But in that case the conversion function will be called which can do more
elaborate verification.

Add test checking for None in checksums.
…bility

Not sure if that makes sense but at least for EB 4.x we need this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants