From 5c6b7f7f1a60f548b4fd7a69a6360695c93e21cb Mon Sep 17 00:00:00 2001 From: Bart Oldeman Date: Wed, 6 Mar 2024 15:27:09 -0500 Subject: [PATCH 1/3] Eliminate `distutils.util.strtobool`. It's only used once so I open-coded it. A stricter but not backwards compatible check would be to only allow 'True' and 'False'. --- easybuild/framework/easyconfig/types.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyconfig/types.py b/easybuild/framework/easyconfig/types.py index fa9829e312..2cf9b77058 100644 --- a/easybuild/framework/easyconfig/types.py +++ b/easybuild/framework/easyconfig/types.py @@ -31,7 +31,6 @@ * Caroline De Brouwer (Ghent University) * Kenneth Hoste (Ghent University) """ -from distutils.util import strtobool from easybuild.base import fancylogger from easybuild.framework.easyconfig.format.format import DEPENDENCY_PARAMETERS @@ -280,7 +279,14 @@ def to_toolchain_dict(spec): res = {'name': spec[0].strip(), 'version': spec[1].strip()} # 3-element list elif len(spec) == 3: - res = {'name': spec[0].strip(), 'version': spec[1].strip(), 'hidden': strtobool(spec[2].strip())} + hidden = spec[2].strip().lower() + if hidden in {'yes', 'true', 't', 'y', '1'}: + hidden = True + elif hidden in {'no', 'false', 'f', 'n', '0'}: + hidden = False + else: + raise EasyBuildError("Invalid truth value %s", hidden) + res = {'name': spec[0].strip(), 'version': spec[1].strip(), 'hidden': hidden} else: raise EasyBuildError("Can not convert list %s to toolchain dict. Expected 2 or 3 elements", spec) From 9cd91f0ff5fd9321545c987484985d93ea08bfba Mon Sep 17 00:00:00 2001 From: Bart Oldeman Date: Wed, 6 Mar 2024 16:00:00 -0500 Subject: [PATCH 2/3] Check for EasyBuildError for strtobool replacement --- test/framework/type_checking.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/framework/type_checking.py b/test/framework/type_checking.py index 1eb2f19fd3..e52c3da603 100644 --- a/test/framework/type_checking.py +++ b/test/framework/type_checking.py @@ -317,9 +317,9 @@ def test_to_toolchain_dict(self): self.assertErrorRegex(EasyBuildError, errstr, to_toolchain_dict, ['gcc', '4', 'False', '7']) # invalid truth value - errstr = "invalid truth value .*" - self.assertErrorRegex(ValueError, errstr, to_toolchain_dict, "intel, 2015, foo") - self.assertErrorRegex(ValueError, errstr, to_toolchain_dict, ['gcc', '4', '7']) + errstr = "Invalid truth value .*" + self.assertErrorRegex(EasyBuildError, errstr, to_toolchain_dict, "intel, 2015, foo") + self.assertErrorRegex(EasyBuildError, errstr, to_toolchain_dict, ['gcc', '4', '7']) # missing keys self.assertErrorRegex(EasyBuildError, "Incorrect set of keys", to_toolchain_dict, {'name': 'intel'}) From bce95ecea3f5b5bcbc244dfb2c39a3227bd5e2d8 Mon Sep 17 00:00:00 2001 From: Bart Oldeman Date: Thu, 7 Mar 2024 09:56:02 -0500 Subject: [PATCH 3/3] Apply suggestions from code review Allow `on` and `off` as well for compatibility with `distutils.strtobool` Co-authored-by: Simon Branford <4967+branfosj@users.noreply.github.com> --- easybuild/framework/easyconfig/types.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyconfig/types.py b/easybuild/framework/easyconfig/types.py index 2cf9b77058..451d22af03 100644 --- a/easybuild/framework/easyconfig/types.py +++ b/easybuild/framework/easyconfig/types.py @@ -280,9 +280,9 @@ def to_toolchain_dict(spec): # 3-element list elif len(spec) == 3: hidden = spec[2].strip().lower() - if hidden in {'yes', 'true', 't', 'y', '1'}: + if hidden in {'yes', 'true', 't', 'y', '1', 'on'}: hidden = True - elif hidden in {'no', 'false', 'f', 'n', '0'}: + elif hidden in {'no', 'false', 'f', 'n', '0', 'off'}: hidden = False else: raise EasyBuildError("Invalid truth value %s", hidden)