From aaf65645f7b2da9dba2ccf1474a7e6d11713dd0b Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 21 Dec 2022 16:57:57 +0100 Subject: [PATCH 1/4] Improve error when checksum dict has no entry for a file When the dict didn't contain the filename EB will crash with > Invalid checksum spec 'None', should be a string (MD5) or 2-tuple (type, value). This will now raise a more descriptive error --- easybuild/tools/filetools.py | 9 +++------ test/framework/filetools.py | 10 ++++++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index daa143b46c..f3bc626b2b 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -1265,14 +1265,11 @@ def verify_checksum(path, checksums): for checksum in checksums: if isinstance(checksum, dict): - if filename in checksum: + try: # Set this to a string-type checksum checksum = checksum[filename] - elif build_option('enforce_checksums'): - raise EasyBuildError("Missing checksum for %s", filename) - else: - # Set to None and allow to fail elsewhere - checksum = None + except KeyError: + raise EasyBuildError("Missing checksum for %s in %s", filename, checksum) if isinstance(checksum, string_type): # if no checksum type is specified, it is assumed to be MD5 (32 characters) or SHA256 (64 characters) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 32d72c7b83..aa6374e965 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -348,6 +348,14 @@ def test_checksums(self): alt_checksums = ('7167b64b1ca062b9674ffef46f9325db7167b64b1ca062b9674ffef46f9325db', broken_checksums['sha256']) self.assertFalse(ft.verify_checksum(fp, alt_checksums)) + # Check dictionary + alt_checksums = (known_checksums['sha256'],) + self.assertTrue(ft.verify_checksum(fp, {os.path.basename(fp): known_checksums['sha256']})) + faulty_dict = {'wrong-name': known_checksums['sha256']} + self.assertErrorRegex(EasyBuildError, + "Missing checksum for " + os.path.basename(fp) + " in .*wrong-name.*", + ft.verify_checksum, fp, faulty_dict) + # check whether missing checksums are enforced build_options = { 'enforce_checksums': True, @@ -362,6 +370,8 @@ def test_checksums(self): for checksum in [known_checksums[x] for x in ('md5', 'sha256')]: dict_checksum = {os.path.basename(fp): checksum, 'foo': 'baa'} self.assertTrue(ft.verify_checksum(fp, dict_checksum)) + del dict_checksum[os.path.basename(fp)] + self.assertErrorRegex(EasyBuildError, "Missing checksum for", ft.verify_checksum, fp, dict_checksum) def test_common_path_prefix(self): """Test get common path prefix for a list of paths.""" From ea926f1e1a998534532058242e5f55b2b427ba85 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 21 Dec 2022 16:57:57 +0100 Subject: [PATCH 2/4] Improve error when checksum dict has no entry for a file When the dict didn't contain the filename EB will crash with > Invalid checksum spec 'None', should be a string (MD5) or 2-tuple (type, value). This will now raise a more descriptive error --- easybuild/tools/filetools.py | 9 +++------ test/framework/filetools.py | 10 ++++++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index daa143b46c..f3bc626b2b 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -1265,14 +1265,11 @@ def verify_checksum(path, checksums): for checksum in checksums: if isinstance(checksum, dict): - if filename in checksum: + try: # Set this to a string-type checksum checksum = checksum[filename] - elif build_option('enforce_checksums'): - raise EasyBuildError("Missing checksum for %s", filename) - else: - # Set to None and allow to fail elsewhere - checksum = None + except KeyError: + raise EasyBuildError("Missing checksum for %s in %s", filename, checksum) if isinstance(checksum, string_type): # if no checksum type is specified, it is assumed to be MD5 (32 characters) or SHA256 (64 characters) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 32d72c7b83..aa6374e965 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -348,6 +348,14 @@ def test_checksums(self): alt_checksums = ('7167b64b1ca062b9674ffef46f9325db7167b64b1ca062b9674ffef46f9325db', broken_checksums['sha256']) self.assertFalse(ft.verify_checksum(fp, alt_checksums)) + # Check dictionary + alt_checksums = (known_checksums['sha256'],) + self.assertTrue(ft.verify_checksum(fp, {os.path.basename(fp): known_checksums['sha256']})) + faulty_dict = {'wrong-name': known_checksums['sha256']} + self.assertErrorRegex(EasyBuildError, + "Missing checksum for " + os.path.basename(fp) + " in .*wrong-name.*", + ft.verify_checksum, fp, faulty_dict) + # check whether missing checksums are enforced build_options = { 'enforce_checksums': True, @@ -362,6 +370,8 @@ def test_checksums(self): for checksum in [known_checksums[x] for x in ('md5', 'sha256')]: dict_checksum = {os.path.basename(fp): checksum, 'foo': 'baa'} self.assertTrue(ft.verify_checksum(fp, dict_checksum)) + del dict_checksum[os.path.basename(fp)] + self.assertErrorRegex(EasyBuildError, "Missing checksum for", ft.verify_checksum, fp, dict_checksum) def test_common_path_prefix(self): """Test get common path prefix for a list of paths.""" From 95c788f3e7f239e5c18a7d24cc4a3a4f450c0c06 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 9 Aug 2023 09:28:15 +0200 Subject: [PATCH 3/4] Update checksum type error message --- easybuild/tools/filetools.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index f3bc626b2b..e7034eca5c 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -1299,7 +1299,8 @@ def verify_checksum(path, checksums): # no matching checksums return False else: - raise EasyBuildError("Invalid checksum spec '%s', should be a string (MD5) or 2-tuple (type, value).", + raise EasyBuildError("Invalid checksum spec '%s', should be a string (MD5 or SHA256) " + "2-tuple (type, value) or tuple of alternative checksum specs.", checksum) actual_checksum = compute_checksum(path, typ) From f60d2c6bd01e45facf01305d53444333194512f9 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Sat, 9 Sep 2023 19:07:41 +0200 Subject: [PATCH 4/4] improve error message produced by verify_checksum --- easybuild/tools/filetools.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index e66a532c10..6e4bd2d83e 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -1298,8 +1298,8 @@ def verify_checksum(path, checksums): # no matching checksums return False else: - raise EasyBuildError("Invalid checksum spec '%s', should be a string (MD5 or SHA256) " - "2-tuple (type, value) or tuple of alternative checksum specs.", + raise EasyBuildError("Invalid checksum spec '%s': should be a string (MD5 or SHA256), " + "2-tuple (type, value), or tuple of alternative checksum specs.", checksum) actual_checksum = compute_checksum(path, typ)