From d29c326fe9cfd42b00e2fa1ebd4bc36a13e47a51 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Tue, 21 May 2019 11:58:39 +0200 Subject: [PATCH 1/5] Fix crash for TMod4 versions less than 4.1.0 * Throw `ConfigError` for TMod4 versions less than 4.1.0. * Change the corresponding docs. --- docs/configure.rst | 2 +- reframe/core/modules.py | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/docs/configure.rst b/docs/configure.rst index 7defce4d44..954ed7a845 100644 --- a/docs/configure.rst +++ b/docs/configure.rst @@ -98,7 +98,7 @@ The valid attributes of a system are the following: Three types of modules systems are currently supported: - ``tmod``: The classic Tcl implementation of the `environment modules `__. - - ``tmod4``: The version 4 of the Tcl implementation of the `environment modules `__. + - ``tmod4``: The version 4 of the Tcl implementation of the `environment modules `__ (versions less than 4.1.0 are not supported). - ``lmod``: The Lua implementation of the `environment modules `__. * ``prefix``: Default regression prefix for this system (default ``.``). * ``stagedir``: Default stage directory for this system (default :class:`None`). diff --git a/reframe/core/modules.py b/reframe/core/modules.py index 1fb206a95b..e64f4803bb 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -218,7 +218,8 @@ def is_module_loaded(self, name): If module ``name`` refers to multiple real modules, this method will return :class:`True` only if all the referees are loaded. """ - return all(self._is_module_loaded(m) for m in self.resolve_module(name)) + return all(self._is_module_loaded(m) + for m in self.resolve_module(name)) def _is_module_loaded(self, name): return self._backend.is_module_loaded(Module(name)) @@ -484,22 +485,30 @@ def emit_unload_instr(self, module): class TMod4Impl(TModImpl): """Module system for TMod 4.""" + MIN_VERSION = ('4', '1', '0') + def __init__(self): self._command = 'modulecmd python' try: completed = os_ext.run_command(self._command + ' -V', check=True) except OSError as e: raise ConfigError( - 'could not find a sane Tmod4 installation') from e + 'could not find a sane TMod4 installation') from e except SpawnedProcessError as e: raise ConfigError( - 'could not get the Python bindings for Tmod4') from e + 'could not get the Python bindings for TMod4') from e - version_match = re.match('^Modules Release (\S+)\s+', completed.stderr) + version_match = re.match(r'^Modules Release (\S+)\s+', + completed.stderr) if not version_match: raise ConfigError('could not retrieve the TMod4 version') - self._version = version_match.group(1) + version = version_match.group(1) + if tuple(version.split('.')) < self.MIN_VERSION: + raise ConfigError( + 'version %s of TMod4 is not supported' % version) + + self._version = version def name(self): return 'tmod4' From 89909600933cce8be196524a34761d78ec42463e Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 22 May 2019 09:25:11 +0200 Subject: [PATCH 2/5] Address PR comments --- docs/configure.rst | 4 ++-- reframe/core/modules.py | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/docs/configure.rst b/docs/configure.rst index 954ed7a845..bc3216feae 100644 --- a/docs/configure.rst +++ b/docs/configure.rst @@ -97,8 +97,8 @@ The valid attributes of a system are the following: * ``modules_system``: The modules system that should be used for loading environment modules on this system (default :class:`None`). Three types of modules systems are currently supported: - - ``tmod``: The classic Tcl implementation of the `environment modules `__. - - ``tmod4``: The version 4 of the Tcl implementation of the `environment modules `__ (versions less than 4.1.0 are not supported). + - ``tmod``: The classic Tcl implementation of the `environment modules `__ (versions less than 3.2 are not supported). + - ``tmod4``: The version 4 of the Tcl implementation of the `environment modules `__ (versions less than 4.1 are not supported). - ``lmod``: The Lua implementation of the `environment modules `__. * ``prefix``: Default regression prefix for this system (default ``.``). * ``stagedir``: Default stage directory for this system (default :class:`None`). diff --git a/reframe/core/modules.py b/reframe/core/modules.py index e64f4803bb..61aee3d17c 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -376,6 +376,8 @@ def __str__(self): class TModImpl(ModulesSystemImpl): """Module system for TMod (Tcl).""" + MIN_VERSION = (3, 2) + def __init__(self): # Try to figure out if we are indeed using the TCL version try: @@ -392,7 +394,14 @@ def __init__(self): if version_match is None or tcl_version_match is None: raise ConfigError('could not find a sane Tmod installation') - self._version = version_match.group(1) + version = version_match.group(1) + ver_major, ver_minor, *_ = [int(v) for v in version.split('.')] + if (ver_major, ver_minor) < self.MIN_VERSION: + raise ConfigError( + 'unsupported TMod version: %s (required >= %s)' % + (version, self.MIN_VERSION)) + + self._version = version self._command = 'modulecmd python' try: # Try the Python bindings now @@ -485,7 +494,7 @@ def emit_unload_instr(self, module): class TMod4Impl(TModImpl): """Module system for TMod 4.""" - MIN_VERSION = ('4', '1', '0') + MIN_VERSION = (4, 1) def __init__(self): self._command = 'modulecmd python' @@ -504,9 +513,11 @@ def __init__(self): raise ConfigError('could not retrieve the TMod4 version') version = version_match.group(1) - if tuple(version.split('.')) < self.MIN_VERSION: + ver_major, ver_minor, *_ = [int(v) for v in version.split('.')] + if (ver_major, ver_minor) < self.MIN_VERSION: raise ConfigError( - 'version %s of TMod4 is not supported' % version) + 'unsupported TMod4 version: %s (required >= %s)' % + (version, self.MIN_VERSION)) self._version = version From 762bfe15a77dccbdaa187ad64e718ef2e5c0cc91 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 22 May 2019 12:02:23 +0200 Subject: [PATCH 3/5] Address PR comments (version 2) --- docs/configure.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/configure.rst b/docs/configure.rst index bc3216feae..31d8e4b067 100644 --- a/docs/configure.rst +++ b/docs/configure.rst @@ -97,8 +97,8 @@ The valid attributes of a system are the following: * ``modules_system``: The modules system that should be used for loading environment modules on this system (default :class:`None`). Three types of modules systems are currently supported: - - ``tmod``: The classic Tcl implementation of the `environment modules `__ (versions less than 3.2 are not supported). - - ``tmod4``: The version 4 of the Tcl implementation of the `environment modules `__ (versions less than 4.1 are not supported). + - ``tmod``: The classic Tcl implementation of the `environment modules `__ (versions older than 3.2 are not supported). + - ``tmod4``: The version 4 of the Tcl implementation of the `environment modules `__ (versions older than 4.1 are not supported). - ``lmod``: The Lua implementation of the `environment modules `__. * ``prefix``: Default regression prefix for this system (default ``.``). * ``stagedir``: Default stage directory for this system (default :class:`None`). From 5f3c9a696ac9ca3a33aa3db994aaf3d6d0878c5a Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 22 May 2019 13:08:46 +0200 Subject: [PATCH 4/5] Address PR comments (version 3) --- reframe/core/modules.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/reframe/core/modules.py b/reframe/core/modules.py index 61aee3d17c..1f031ad843 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -384,7 +384,7 @@ def __init__(self): completed = os_ext.run_command('modulecmd -V') except OSError as e: raise ConfigError( - 'could not find a sane Tmod installation: %s' % e) from e + 'could not find a sane TMod installation: %s' % e) from e version_match = re.search(r'^VERSION=(\S+)', completed.stdout, re.MULTILINE) @@ -392,10 +392,14 @@ def __init__(self): re.MULTILINE) if version_match is None or tcl_version_match is None: - raise ConfigError('could not find a sane Tmod installation') + raise ConfigError('could not find a sane TMod installation') version = version_match.group(1) - ver_major, ver_minor, *_ = [int(v) for v in version.split('.')] + try: + ver_major, ver_minor, *_ = [int(v) for v in version.split('.')] + except ValueError as e: + raise ConfigError('could not parse TMod version') from e + if (ver_major, ver_minor) < self.MIN_VERSION: raise ConfigError( 'unsupported TMod version: %s (required >= %s)' % @@ -408,11 +412,11 @@ def __init__(self): completed = os_ext.run_command(self._command) except OSError as e: raise ConfigError( - 'could not get the Python bindings for Tmod: ' % e) from e + 'could not get the Python bindings for TMod: ' % e) from e if re.search(r'Unknown shell type', completed.stderr): raise ConfigError( - 'Python is not supported by this Tmod installation') + 'Python is not supported by this TMod installation') def name(self): return 'tmod' @@ -513,7 +517,11 @@ def __init__(self): raise ConfigError('could not retrieve the TMod4 version') version = version_match.group(1) - ver_major, ver_minor, *_ = [int(v) for v in version.split('.')] + try: + ver_major, ver_minor, *_ = [int(v) for v in version.split('.')] + except ValueError as e: + raise ConfigError('could not parse TMod4 version') from e + if (ver_major, ver_minor) < self.MIN_VERSION: raise ConfigError( 'unsupported TMod4 version: %s (required >= %s)' % From 9d3e4493125bd0abca12362d538af5e6d90e40a6 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Wed, 22 May 2019 13:23:20 +0200 Subject: [PATCH 5/5] Print version string in case of parse errors --- reframe/core/modules.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/reframe/core/modules.py b/reframe/core/modules.py index 1f031ad843..5242c921be 100644 --- a/reframe/core/modules.py +++ b/reframe/core/modules.py @@ -397,8 +397,9 @@ def __init__(self): version = version_match.group(1) try: ver_major, ver_minor, *_ = [int(v) for v in version.split('.')] - except ValueError as e: - raise ConfigError('could not parse TMod version') from e + except ValueError: + raise ConfigError( + 'could not parse TMod version string: ' + version) from None if (ver_major, ver_minor) < self.MIN_VERSION: raise ConfigError( @@ -519,8 +520,9 @@ def __init__(self): version = version_match.group(1) try: ver_major, ver_minor, *_ = [int(v) for v in version.split('.')] - except ValueError as e: - raise ConfigError('could not parse TMod4 version') from e + except ValueError: + raise ConfigError( + 'could not parse TMod4 version string: ' + version) from None if (ver_major, ver_minor) < self.MIN_VERSION: raise ConfigError(