Skip to content

Commit 597dca8

Browse files
committed
Fix the checksum type check
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.
1 parent 8ba299f commit 597dca8

2 files changed

Lines changed: 35 additions & 7 deletions

File tree

easybuild/framework/easyconfig/types.py

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -565,19 +565,33 @@ def ensure_iterable_license_specs(specs):
565565
}))
566566
# checksums is a list of checksums, one entry per file (source/patch)
567567
# each entry can be:
568+
# None
568569
# a single checksum value (string)
569570
# a single checksum value of a specified type (2-tuple, 1st element is checksum type, 2nd element is checksum)
570571
# a list of checksums (of different types, perhaps different formats), which should *all* be valid
571-
# a dictionary with a mapping from filename to checksum value
572-
CHECKSUM_LIST = (list, as_hashable({'elem_types': [str, tuple, STRING_DICT]}))
573-
CHECKSUMS = (list, as_hashable({'elem_types': [str, tuple, STRING_DICT, CHECKSUM_LIST]}))
572+
# a tuple of checksums (of different types, perhaps different formats), where one should be valid
573+
# a dictionary with a mapping from filename to checksum (None, value, type&value, alternatives)
574574

575-
CHECKABLE_TYPES = [CHECKSUM_LIST, CHECKSUMS, DEPENDENCIES, DEPENDENCY_DICT, LIST_OF_STRINGS,
575+
# Type & value, value may be an int for type "size"
576+
# This is a bit too permissive as it allows the first element to be an int and doesn't restrict the number of elements
577+
CHECKSUM_TUPLE = (tuple, as_hashable({'elem_types': [str, int]}))
578+
CHECKSUM_DICT = (dict, as_hashable(
579+
{
580+
'elem_types': [type(None), str, CHECKSUM_TUPLE],
581+
'key_types': [str],
582+
}
583+
))
584+
CHECKSUM_LIST = (list, as_hashable({'elem_types': [str, CHECKSUM_TUPLE, CHECKSUM_DICT]}))
585+
586+
CHECKSUMS = (list, as_hashable({'elem_types': [type(None), str, CHECKSUM_LIST, CHECKSUM_TUPLE, CHECKSUM_DICT]}))
587+
588+
CHECKABLE_TYPES = [CHECKSUM_DICT, CHECKSUM_LIST, CHECKSUM_TUPLE, CHECKSUMS,
589+
DEPENDENCIES, DEPENDENCY_DICT, LIST_OF_STRINGS,
576590
SANITY_CHECK_PATHS_DICT, STRING_DICT, STRING_OR_TUPLE_LIST, STRING_OR_TUPLE_OR_DICT_LIST,
577591
TOOLCHAIN_DICT, TUPLE_OF_STRINGS]
578592

579593
# easy types, that can be verified with isinstance
580-
EASY_TYPES = [string_type, bool, dict, int, list, str, tuple]
594+
EASY_TYPES = [string_type, bool, dict, int, list, str, tuple, type(None)]
581595

582596
# type checking is skipped for easyconfig parameters names not listed in PARAMETER_TYPES
583597
PARAMETER_TYPES = {

test/framework/type_checking.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,24 @@ def test_check_type_of_param_value_checksums(self):
221221
{'foo.txt': sha256_checksum1, 'bar.txt': sha256_checksum2},
222222
# 3 alternative checksums for a single file, one match is sufficient
223223
(sha256_checksum1, sha256_checksum2, sha256_checksum3),
224-
]
224+
# two alternative checksums for a single file (not to be confused by checksum-type & -value tuple)
225+
(sha256_checksum1, md5_checksum),
226+
# three alternative checksums for a single file of different types
227+
(sha256_checksum1, ('md5', md5_checksum), {'foo.txt': sha256_checksum1}),
228+
# alternative checksums in dicts are also allowed
229+
{'foo.txt': (sha256_checksum2, sha256_checksum3), 'bar.txt': (sha256_checksum1, md5_checksum)},
230+
# Same but with lists -> all must match for each file
231+
{'foo.txt': [sha256_checksum2, sha256_checksum3], 'bar.txt': [sha256_checksum1, md5_checksum]},
232+
],
233+
# None is allowed, meaning skip the checksum
234+
[
235+
None,
236+
# Also in mappings
237+
{'foo.txt': sha256_checksum1, 'bar.txt': None},
238+
],
225239
]
226240
for inp in inputs:
227-
self.assertEqual(check_type_of_param_value('checksums', inp), (True, inp))
241+
self.assertTrue(check_type_of_param_value('checksums', inp), 'Failed for ' + str(inp))
228242

229243
def test_check_type_of_param_value_patches(self):
230244
"""Test check_type_of_param_value function for patches."""

0 commit comments

Comments
 (0)