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

checksum dict with a checksum value of None doesn't work #4142

Open
akesandgren opened this issue Dec 15, 2022 · 4 comments · May be fixed by #4159
Open

checksum dict with a checksum value of None doesn't work #4142

akesandgren opened this issue Dec 15, 2022 · 4 comments · May be fixed by #4159

Comments

@akesandgren
Copy link
Contributor

akesandgren commented Dec 15, 2022

This failes with --check-contrib, -D, and --new-pr

checksums = [
    {'v2.12.4.tar.gz': '31c1f4f91efbdb74619cebb36f57f999d6f1a57bb6f87b13e60d21e670c38f68'},
    {'CCCoreLib-20220714.tar.gz': None},
]

--check-contrib and -D fails on the above with

ERROR: Failed to process easyconfig /afs/hpc2n.umu.se/home/a/ake/Projects/git-repos/easybuild/easybuild_common/custom/easyconfigs/c/CloudCompare/CloudCompare-2.12.4-foss-2021b.eb: Converting type of [{'v2.12.4.tar.gz': '31c1f4f91efbdb74619cebb36f57f999d6f1a57bb6f87b13e60d21e670c38f68'}, {'CCCoreLib-20220714.tar.gz': None}] (<class 'list'>) to (<class 'list'>, (('elem_types', (<class 'str'>, <class 'tuple'>, (<class 'dict'>, (('elem_types', (<class 'str'>,)), ('key_types', (<class 'str'>,)))), (<class 'list'>, (('elem_types', (<class 'str'>, <class 'tuple'>, (<class 'dict'>, (('elem_types', (<class 'str'>,)), ('key_types', (<class 'str'>,)))))),)))),)) using <function to_checksums at 0x7fb0e86b4ee0> failed: 'NoneType' object is not iterable
@boegel boegel added this to the 4.x milestone Dec 21, 2022
@boegel
Copy link
Member

boegel commented Jan 4, 2023

related to #4150

@boegel boegel modified the milestones: 4.x, next release (4.7.0) Jan 4, 2023
@Flamefire
Copy link
Contributor

Working on that and found more issues with that function

Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue Jan 4, 2023
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limitting the recursiveness.

Fixes easybuilders#4142
Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue Jan 4, 2023
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limitting the recursiveness.

Fixes easybuilders#4142
Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue Jan 4, 2023
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limiting the recursiveness.

Fixes easybuilders#4142
Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue Jan 4, 2023
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limiting the recursiveness.

Fixes easybuilders#4142
Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue Jan 10, 2023
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limiting the recursiveness.

Fixes easybuilders#4142
@Flamefire
Copy link
Contributor

Ok got 2 PRs ready which should fix this and related issues. Turned out it was way harder than anticipated due to support of YEB files that don't have tuples.

Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue Jan 12, 2023
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limiting the recursiveness.

Fixes easybuilders#4142
@Flamefire
Copy link
Contributor

Flamefire commented Jan 12, 2023

I think we should first discuss and settle on a specification of checksums. I started a PR at easybuilders/easybuild#853 to serve as a basis for discussion. E.g. currently it is not documented, that you can use a dict at all and hence the meaning of a None entry or a missing key/filename is even less clear.

FWIW: Not only the check fails but None as a value is currently not supported by the checksum check function and would yield a cryptic error. The same as not having the filename listed in the dict (before #4150 gets merged)

Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue May 11, 2023
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limiting the recursiveness.

Fixes easybuilders#4142
Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue May 11, 2023
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limiting the recursiveness.

Fixes easybuilders#4142
Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue May 24, 2023
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limiting the recursiveness.

Fixes easybuilders#4142
@boegel boegel modified the milestones: 4.7.3, release after 4.7.3 Jul 6, 2023
Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue Nov 16, 2023
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limiting the recursiveness.

Fixes easybuilders#4142
@boegel boegel modified the milestones: 4.9.1, release after 4.9.1 Apr 3, 2024
Flamefire added a commit to Flamefire/easybuild-framework that referenced this issue Apr 16, 2024
Having a `'src': None` entry in a dict for checksums is as valid as
having a `None` entry directly in the list. However the current function
didn't handle it and crashed.
Fix that as well as a few corner cases especially in the recursive case
by introducing a new function for handling checksum entries in the
checksum list and limiting the recursiveness.

Fixes easybuilders#4142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants