From 0ec2df5b3acbff6d74f2764bac8049b17921412c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Wed, 3 Apr 2024 21:15:29 +0000 Subject: [PATCH 01/17] Make new safe module abstraction for update_paths --- easybuild/tools/module_generator.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index c537003de5..3314ba475e 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -243,15 +243,23 @@ def append_paths(self, key, paths, allow_abs=False, expand_relpaths=True): :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ - paths = self._filter_paths(key, paths) - if paths is None: - return '' return self.update_paths(key, paths, prepend=False, allow_abs=allow_abs, expand_relpaths=expand_relpaths) def prepend_paths(self, key, paths, allow_abs=False, expand_relpaths=True): """ Generate prepend-path statements for the given list of paths. + :param key: environment variable to append paths to + :param paths: list of paths to append + :param allow_abs: allow providing of absolute paths + :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) + """ + return self.update_paths(key, paths, prepend=True, allow_abs=allow_abs, expand_relpaths=expand_relpaths) + + def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): + """ + Generate append/prepend-path statements for the given list of paths. + :param key: environment variable to append paths to :param paths: list of paths to append :param allow_abs: allow providing of absolute paths @@ -260,7 +268,7 @@ def prepend_paths(self, key, paths, allow_abs=False, expand_relpaths=True): paths = self._filter_paths(key, paths) if paths is None: return '' - return self.update_paths(key, paths, prepend=True, allow_abs=allow_abs, expand_relpaths=expand_relpaths) + return self._update_paths(key, paths, prepend, allow_abs, expand_relpaths) def _modulerc_check_module_version(self, module_version): """ @@ -551,7 +559,7 @@ def unload_module(self, mod_name): """ raise NotImplementedError - def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): + def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): """ Generate prepend-path or append-path statements for the given list of paths. @@ -955,7 +963,7 @@ def msg_on_unload(self, msg): print_cmd = "puts stderr %s" % quote_str(msg, tcl=True) return '\n'.join(['', self.conditional_statement("module-info mode unload", print_cmd, indent=False)]) - def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): + def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): """ Generate prepend-path or append-path statements for the given list of paths. @@ -981,7 +989,7 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath abspaths = [] for path in paths: if os.path.isabs(path) and not allow_abs: - raise EasyBuildError("Absolute path %s passed to update_paths which only expects relative paths.", + raise EasyBuildError("Absolute path %s passed to _update_paths which only expects relative paths.", path) elif not os.path.isabs(path): # prepend/append $root (= installdir) for (non-empty) relative paths @@ -1426,7 +1434,7 @@ def modulerc(self, module_version=None, filepath=None, modulerc_txt=None): return super(ModuleGeneratorLua, self).modulerc(module_version=module_version, filepath=filepath, modulerc_txt=modulerc_txt) - def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): + def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): """ Generate prepend_path or append_path statements for the given list of paths @@ -1455,7 +1463,7 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath if allow_abs: abspaths.append(quote_str(path)) else: - raise EasyBuildError("Absolute path %s passed to update_paths which only expects relative paths.", + raise EasyBuildError("Absolute path %s passed to _update_paths which only expects relative paths.", path) else: # use pathJoin for (non-empty) relative paths From 74a3cb364daf8c7d2d2560ab9d840dc3de30c388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Wed, 3 Apr 2024 21:58:39 +0000 Subject: [PATCH 02/17] Replace PYTHONPATHs with EBPYTHONPREFIX when possible Co-authored-by: Alexander Grund --- easybuild/tools/module_generator.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 3314ba475e..e41c0997bc 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -268,6 +268,17 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath paths = self._filter_paths(key, paths) if paths is None: return '' + if key == 'PYTHONPATH': + # Special condition for PYTHONPATHs that match the standard pattern, + # replace with EBPYTHONPREFIX which is added to python sys path at runtime + python_paths = {path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)} + if python_paths: + self.log.info("Replaced PYTHONPATH %s with EBPYTHONPREFIX", python_paths) + paths = [path for path in paths if path not in python_paths] + ret = self._update_paths('EBPYTHONPREFIX', [''], prepend, allow_abs, expand_relpaths) + if paths: + ret += self._update_paths(key, paths, prepend, allow_abs, expand_relpaths) + return ret return self._update_paths(key, paths, prepend, allow_abs, expand_relpaths) def _modulerc_check_module_version(self, module_version): From 03e304ece15e0009a82748da4bb2310dce714096 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 4 Apr 2024 19:22:02 +0200 Subject: [PATCH 03/17] Add option prefer-ebpythonprefix-over-pythonpath --- easybuild/tools/config.py | 1 + easybuild/tools/module_generator.py | 6 +++--- easybuild/tools/options.py | 2 ++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 9791eb3dcc..53bf21a384 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -330,6 +330,7 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX): 'map_toolchains', 'modules_tool_version_check', 'mpi_tests', + 'prefer_ebpythonprefix_over_pythonpath', 'pre_create_installdir', 'show_progress_bar', 'trace', diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index e41c0997bc..8e401a7c8f 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -268,7 +268,7 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath paths = self._filter_paths(key, paths) if paths is None: return '' - if key == 'PYTHONPATH': + if key == 'PYTHONPATH' and build_option('prefer_ebpythonprefix_over_pythonpath'): # Special condition for PYTHONPATHs that match the standard pattern, # replace with EBPYTHONPREFIX which is added to python sys path at runtime python_paths = {path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)} @@ -1000,7 +1000,7 @@ def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpat abspaths = [] for path in paths: if os.path.isabs(path) and not allow_abs: - raise EasyBuildError("Absolute path %s passed to _update_paths which only expects relative paths.", + raise EasyBuildError("Absolute path %s passed to update_paths which only expects relative paths.", path) elif not os.path.isabs(path): # prepend/append $root (= installdir) for (non-empty) relative paths @@ -1474,7 +1474,7 @@ def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpat if allow_abs: abspaths.append(quote_str(path)) else: - raise EasyBuildError("Absolute path %s passed to _update_paths which only expects relative paths.", + raise EasyBuildError("Absolute path %s passed to update_paths which only expects relative paths.", path) else: # use pathJoin for (non-empty) relative paths diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 54e9b81982..7004f21dc0 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -480,6 +480,8 @@ def override_options(self): 'int', 'store', None), 'parallel-extensions-install': ("Install list of extensions in parallel (if supported)", None, 'store_true', False), + 'prefer-ebpythonprefix-over-pythonpath': ("Replaces PYTHONPATH with EBPYTHONPREFIX when possible", + None, 'store_true', True), 'pre-create-installdir': ("Create installation directory before submitting build jobs", None, 'store_true', True), 'pretend': (("Does the build/installation in a test directory located in $HOME/easybuildinstall"), From b07ea3fd4670eebc06d7d73e63180edaa3c7c558 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Wed, 10 Apr 2024 01:38:46 +0200 Subject: [PATCH 04/17] Add warning for duplicated PYTHONPATH's that should not occur --- easybuild/tools/module_generator.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 8e401a7c8f..db9f9cfc58 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -271,11 +271,14 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath if key == 'PYTHONPATH' and build_option('prefer_ebpythonprefix_over_pythonpath'): # Special condition for PYTHONPATHs that match the standard pattern, # replace with EBPYTHONPREFIX which is added to python sys path at runtime - python_paths = {path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)} + python_paths = [path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] if python_paths: - self.log.info("Replaced PYTHONPATH %s with EBPYTHONPREFIX", python_paths) - paths = [path for path in paths if path not in python_paths] + if len(python_paths) > 1: + raise EasyBuildError('Found multiple paths for PYTHONPATH: ' + ', '.join(python_paths)) + python_path = python_paths[0] + self.log.info("Replaced PYTHONPATH %s with EBPYTHONPREFIX", python_path) ret = self._update_paths('EBPYTHONPREFIX', [''], prepend, allow_abs, expand_relpaths) + paths = [path for path in paths if path != python_path] if paths: ret += self._update_paths(key, paths, prepend, allow_abs, expand_relpaths) return ret From 8c66e1aa228f6416c17977a6d9baea6c547907b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Wed, 10 Apr 2024 02:33:54 +0200 Subject: [PATCH 05/17] Add tests for PYTHONPATH to EBPYTHONPREFIX rewrite --- test/framework/module_generator.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/framework/module_generator.py b/test/framework/module_generator.py index 075cc3adbd..88846a48e9 100644 --- a/test/framework/module_generator.py +++ b/test/framework/module_generator.py @@ -694,6 +694,9 @@ def append_paths(*args, **kwargs): res = append_paths('key', ['1234@example.com'], expand_relpaths=False) self.assertEqual("append-path\tkey\t\t1234@example.com\n", res) + expected = "append-path\tEBPYTHONPREFIX\t\t$root\nappend-path\tPYTHONPATH\t\t$root/foo\n" + res = append_paths('PYTHONPATH', ['lib/python3.12/site-packages', 'foo']) + self.assertEqual(expected, res) else: expected = ''.join([ 'append_path("key", pathJoin(root, "path1"))\n', @@ -714,6 +717,10 @@ def append_paths(*args, **kwargs): res = append_paths('key', ['1234@example.com'], expand_relpaths=False) self.assertEqual('append_path("key", "1234@example.com")\n', res) + expected = 'append_path("EBPYTHONPREFIX", root)\nappend_path("PYTHONPATH", pathJoin(root, "foo"))\n' + res = append_paths('PYTHONPATH', ['lib/python3.12/site-packages', 'foo']) + self.assertEqual(expected, res) + self.assertErrorRegex(EasyBuildError, "Absolute path %s/foo passed to update_paths " "which only expects relative paths." % self.modgen.app.installdir, append_paths, "key2", ["bar", "%s/foo" % self.modgen.app.installdir]) From ccd3d7e79af3630e0adf98cbda4766bac4695985 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 11 Apr 2024 13:12:37 +0000 Subject: [PATCH 06/17] Move check for faulty PYTHONPATHs to always run it It's even more important for those who don't use EBPYTHONPREFIX to ensure it doesn't add multiple mixed PYTHONPATH's --- easybuild/tools/module_generator.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index db9f9cfc58..cdd6213bd0 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -268,13 +268,14 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath paths = self._filter_paths(key, paths) if paths is None: return '' - if key == 'PYTHONPATH' and build_option('prefer_ebpythonprefix_over_pythonpath'): + if key == 'PYTHONPATH': + python_paths = [path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] + if len(python_paths) > 1: + raise EasyBuildError('Found multiple paths for PYTHONPATH: ' + ', '.join(python_paths)) + # Special condition for PYTHONPATHs that match the standard pattern, # replace with EBPYTHONPREFIX which is added to python sys path at runtime - python_paths = [path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] - if python_paths: - if len(python_paths) > 1: - raise EasyBuildError('Found multiple paths for PYTHONPATH: ' + ', '.join(python_paths)) + if python_paths and build_option('prefer_ebpythonprefix_over_pythonpath'): python_path = python_paths[0] self.log.info("Replaced PYTHONPATH %s with EBPYTHONPREFIX", python_path) ret = self._update_paths('EBPYTHONPREFIX', [''], prepend, allow_abs, expand_relpaths) From f76f3205f9f7a71e9cae41c35b80726be69a33a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 11 Apr 2024 15:05:46 +0000 Subject: [PATCH 07/17] Move conversion of paths as str to lists earlier removing duplicated code --- easybuild/tools/module_generator.py | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index cdd6213bd0..ed033a01c8 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -213,18 +213,10 @@ def _filter_paths(self, key, paths): return paths added_paths = self.added_paths_per_key.setdefault(key, set()) - # paths can be a string - if isinstance(paths, str): - if paths in added_paths: - filtered_paths = None - else: - added_paths.add(paths) - filtered_paths = paths - else: - # Coerce any iterable/generator into a list - if not isinstance(paths, list): - paths = list(paths) - filtered_paths = [x for x in paths if x not in added_paths and not added_paths.add(x)] + # Coerce any iterable/generator into a list + if not isinstance(paths, list): + paths = list(paths) + filtered_paths = [x for x in paths if x not in added_paths and not added_paths.add(x)] if filtered_paths != paths: removed_paths = paths if filtered_paths is None else [x for x in paths if x not in filtered_paths] print_warning("Suppressed adding the following path(s) to $%s of the module as they were already added: %s", @@ -265,9 +257,14 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ + if isinstance(paths, str): + self.log.debug("Wrapping %s into a list before using it for %s", paths, key) + paths = [paths] + paths = self._filter_paths(key, paths) if paths is None: return '' + if key == 'PYTHONPATH': python_paths = [path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] if len(python_paths) > 1: @@ -997,10 +994,6 @@ def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpat self.log.info("Not including statement to %s environment variable $%s, as specified", update_type, key) return '' - if isinstance(paths, str): - self.log.debug("Wrapping %s into a list before using it to %s path %s", paths, update_type, key) - paths = [paths] - abspaths = [] for path in paths: if os.path.isabs(path) and not allow_abs: @@ -1468,10 +1461,6 @@ def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpat self.log.info("Not including statement to %s environment variable $%s, as specified", update_type, key) return '' - if isinstance(paths, str): - self.log.debug("Wrapping %s into a list before using it to %s path %s", update_type, paths, key) - paths = [paths] - abspaths = [] for path in paths: if os.path.isabs(path): From 8b5389f06bc88b41a71a0e3c8e7001b4dd5368dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Thu, 11 Apr 2024 15:35:48 +0000 Subject: [PATCH 08/17] Fix name EBPYTHONPREFIX to EBPYTHONPREFIXES which is used in sitecustomize.py --- easybuild/tools/module_generator.py | 4 ++-- test/framework/module_generator.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index ed033a01c8..b3104962e9 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -274,8 +274,8 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath # replace with EBPYTHONPREFIX which is added to python sys path at runtime if python_paths and build_option('prefer_ebpythonprefix_over_pythonpath'): python_path = python_paths[0] - self.log.info("Replaced PYTHONPATH %s with EBPYTHONPREFIX", python_path) - ret = self._update_paths('EBPYTHONPREFIX', [''], prepend, allow_abs, expand_relpaths) + self.log.info("Replaced PYTHONPATH %s with EBPYTHONPREFIXES", python_path) + ret = self._update_paths('EBPYTHONPREFIXES', [''], prepend, allow_abs, expand_relpaths) paths = [path for path in paths if path != python_path] if paths: ret += self._update_paths(key, paths, prepend, allow_abs, expand_relpaths) diff --git a/test/framework/module_generator.py b/test/framework/module_generator.py index 88846a48e9..023b04ed66 100644 --- a/test/framework/module_generator.py +++ b/test/framework/module_generator.py @@ -694,7 +694,7 @@ def append_paths(*args, **kwargs): res = append_paths('key', ['1234@example.com'], expand_relpaths=False) self.assertEqual("append-path\tkey\t\t1234@example.com\n", res) - expected = "append-path\tEBPYTHONPREFIX\t\t$root\nappend-path\tPYTHONPATH\t\t$root/foo\n" + expected = "append-path\tEBPYTHONPREFIXES\t\t$root\nappend-path\tPYTHONPATH\t\t$root/foo\n" res = append_paths('PYTHONPATH', ['lib/python3.12/site-packages', 'foo']) self.assertEqual(expected, res) else: @@ -717,7 +717,7 @@ def append_paths(*args, **kwargs): res = append_paths('key', ['1234@example.com'], expand_relpaths=False) self.assertEqual('append_path("key", "1234@example.com")\n', res) - expected = 'append_path("EBPYTHONPREFIX", root)\nappend_path("PYTHONPATH", pathJoin(root, "foo"))\n' + expected = 'append_path("EBPYTHONPREFIXES", root)\nappend_path("PYTHONPATH", pathJoin(root, "foo"))\n' res = append_paths('PYTHONPATH', ['lib/python3.12/site-packages', 'foo']) self.assertEqual(expected, res) From cb6adb05d41b7a4db959b20d9274b093a9a71a95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Fri, 12 Apr 2024 02:34:02 +0200 Subject: [PATCH 09/17] Move _filter_paths check to _update_paths This will ensure duplicate paths are filtered even when EBPYTHONPREFIXES rewrite is in place. --- easybuild/tools/module_generator.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index b3104962e9..77f592c028 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -220,7 +220,7 @@ def _filter_paths(self, key, paths): if filtered_paths != paths: removed_paths = paths if filtered_paths is None else [x for x in paths if x not in filtered_paths] print_warning("Suppressed adding the following path(s) to $%s of the module as they were already added: %s", - key, removed_paths, + key, ', '.join(removed_paths), log=self.log) if not filtered_paths: filtered_paths = None @@ -261,10 +261,6 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath self.log.debug("Wrapping %s into a list before using it for %s", paths, key) paths = [paths] - paths = self._filter_paths(key, paths) - if paths is None: - return '' - if key == 'PYTHONPATH': python_paths = [path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] if len(python_paths) > 1: @@ -985,6 +981,10 @@ def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpat :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ + paths = self._filter_paths(key, paths) + if paths is None: + return '' + if prepend: update_type = 'prepend' else: @@ -1452,6 +1452,10 @@ def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpat :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ + paths = self._filter_paths(key, paths) + if paths is None: + return '' + if prepend: update_type = 'prepend' else: From 4a11f2fc2febcaec9c85b734c499d1e8d7020074 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sun, 14 Apr 2024 20:17:16 +0200 Subject: [PATCH 10/17] Simplify logic for _filter_paths --- easybuild/tools/module_generator.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 77f592c028..9c600ccbe5 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -213,17 +213,12 @@ def _filter_paths(self, key, paths): return paths added_paths = self.added_paths_per_key.setdefault(key, set()) - # Coerce any iterable/generator into a list - if not isinstance(paths, list): - paths = list(paths) filtered_paths = [x for x in paths if x not in added_paths and not added_paths.add(x)] if filtered_paths != paths: removed_paths = paths if filtered_paths is None else [x for x in paths if x not in filtered_paths] print_warning("Suppressed adding the following path(s) to $%s of the module as they were already added: %s", key, ', '.join(removed_paths), log=self.log) - if not filtered_paths: - filtered_paths = None return filtered_paths def append_paths(self, key, paths, allow_abs=False, expand_relpaths=True): @@ -231,7 +226,7 @@ def append_paths(self, key, paths, allow_abs=False, expand_relpaths=True): Generate append-path statements for the given list of paths. :param key: environment variable to append paths to - :param paths: list of paths to append + :param paths: single or list of paths to append :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ @@ -242,7 +237,7 @@ def prepend_paths(self, key, paths, allow_abs=False, expand_relpaths=True): Generate prepend-path statements for the given list of paths. :param key: environment variable to append paths to - :param paths: list of paths to append + :param paths: single or list of paths to append :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ @@ -253,13 +248,15 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath Generate append/prepend-path statements for the given list of paths. :param key: environment variable to append paths to - :param paths: list of paths to append + :param paths: single or list of paths to append :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ if isinstance(paths, str): self.log.debug("Wrapping %s into a list before using it for %s", paths, key) paths = [paths] + elif not isinstance(paths, list): + paths = list(paths) # coerce any iterator into a list if key == 'PYTHONPATH': python_paths = [path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] @@ -976,14 +973,12 @@ def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpat Generate prepend-path or append-path statements for the given list of paths. :param key: environment variable to prepend/append paths to - :param paths: list of paths to prepend + :param paths: list of paths to prepend/append :param prepend: whether to prepend (True) or append (False) paths :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ paths = self._filter_paths(key, paths) - if paths is None: - return '' if prepend: update_type = 'prepend' @@ -1453,8 +1448,6 @@ def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpat :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ paths = self._filter_paths(key, paths) - if paths is None: - return '' if prepend: update_type = 'prepend' From c471e03c0443ff4b741910d13cd8dede8a62e703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sun, 21 Apr 2024 17:10:55 +0200 Subject: [PATCH 11/17] Rename flag to replace_pythonpath --- easybuild/tools/config.py | 2 +- easybuild/tools/module_generator.py | 4 ++-- easybuild/tools/options.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/easybuild/tools/config.py b/easybuild/tools/config.py index 53bf21a384..9ac971d7ba 100644 --- a/easybuild/tools/config.py +++ b/easybuild/tools/config.py @@ -330,8 +330,8 @@ def mk_full_default_path(name, prefix=DEFAULT_PREFIX): 'map_toolchains', 'modules_tool_version_check', 'mpi_tests', - 'prefer_ebpythonprefix_over_pythonpath', 'pre_create_installdir', + 'replace_pythonpath', 'show_progress_bar', 'trace', ], diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 9c600ccbe5..698d4fcfa9 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -264,8 +264,8 @@ def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpath raise EasyBuildError('Found multiple paths for PYTHONPATH: ' + ', '.join(python_paths)) # Special condition for PYTHONPATHs that match the standard pattern, - # replace with EBPYTHONPREFIX which is added to python sys path at runtime - if python_paths and build_option('prefer_ebpythonprefix_over_pythonpath'): + # replace with EBPYTHONPREFIXES which is added to python sys path at runtime via sitecustomize.py + if python_paths and build_option('replace_pythonpath'): python_path = python_paths[0] self.log.info("Replaced PYTHONPATH %s with EBPYTHONPREFIXES", python_path) ret = self._update_paths('EBPYTHONPREFIXES', [''], prepend, allow_abs, expand_relpaths) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 7004f21dc0..419a1213b0 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -480,8 +480,6 @@ def override_options(self): 'int', 'store', None), 'parallel-extensions-install': ("Install list of extensions in parallel (if supported)", None, 'store_true', False), - 'prefer-ebpythonprefix-over-pythonpath': ("Replaces PYTHONPATH with EBPYTHONPREFIX when possible", - None, 'store_true', True), 'pre-create-installdir': ("Create installation directory before submitting build jobs", None, 'store_true', True), 'pretend': (("Does the build/installation in a test directory located in $HOME/easybuildinstall"), @@ -491,6 +489,8 @@ def override_options(self): 'remove-ghost-install-dirs': ("Remove ghost installation directories when --force or --rebuild is used, " "rather than just warning about them", None, 'store_true', False), + 'replace-pythonpath': ("Replaces PYTHONPATH with EBPYTHONPREFIXES in modules when possible", + None, 'store_true', True), 'required-linked-shared-libs': ("Comma-separated list of shared libraries (names, file names, or paths) " "which must be linked in all installed binaries/libraries", 'strlist', 'extend', None), From 2676e4eea8f849ce3b2d330e3ad4af0afa35c35e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sun, 21 Apr 2024 17:45:36 +0200 Subject: [PATCH 12/17] Add assertion for input argument to _filter_paths. --- easybuild/tools/module_generator.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 698d4fcfa9..fcc5ad7b81 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -42,6 +42,7 @@ from contextlib import contextmanager from easybuild.tools import LooseVersion from textwrap import wrap +from typing import Union, Sequence from easybuild.base import fancylogger from easybuild.tools.build_log import EasyBuildError, print_warning @@ -205,8 +206,10 @@ def get_modules_path(self, fake=False, mod_path_suffix=None): return os.path.join(mod_path, mod_path_suffix) - def _filter_paths(self, key, paths): + def _filter_paths(self, key: str, paths: list) -> list: """Filter out paths already added to key and return the remaining ones""" + assert isinstance(paths, list) + if self.added_paths_per_key is None: # For compatibility this is only a warning for now and we don't filter any paths print_warning('Module creation has not been started. Call start_module_creation first!') @@ -221,7 +224,7 @@ def _filter_paths(self, key, paths): log=self.log) return filtered_paths - def append_paths(self, key, paths, allow_abs=False, expand_relpaths=True): + def append_paths(self, key: str, paths: Union[str, Sequence[str]], allow_abs=False, expand_relpaths=True) -> str: """ Generate append-path statements for the given list of paths. @@ -232,23 +235,24 @@ def append_paths(self, key, paths, allow_abs=False, expand_relpaths=True): """ return self.update_paths(key, paths, prepend=False, allow_abs=allow_abs, expand_relpaths=expand_relpaths) - def prepend_paths(self, key, paths, allow_abs=False, expand_relpaths=True): + def prepend_paths(self, key: str, paths: Union[str, Sequence[str]], allow_abs=False, expand_relpaths=True) -> str: """ Generate prepend-path statements for the given list of paths. :param key: environment variable to append paths to - :param paths: single or list of paths to append + :param paths: single or list of paths to prepend :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ return self.update_paths(key, paths, prepend=True, allow_abs=allow_abs, expand_relpaths=expand_relpaths) - def update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): + def update_paths(self, key: str, paths: Union[str, Sequence[str]], prepend=True, allow_abs=False, + expand_relpaths=True) -> str: """ Generate append/prepend-path statements for the given list of paths. :param key: environment variable to append paths to - :param paths: single or list of paths to append + :param paths: single or list of paths to add :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) """ @@ -564,12 +568,12 @@ def unload_module(self, mod_name): """ raise NotImplementedError - def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): + def _update_paths(self, key: str, paths: list, prepend=True, allow_abs=False, expand_relpaths=True) -> str: """ Generate prepend-path or append-path statements for the given list of paths. :param key: environment variable to prepend/append paths to - :param paths: list of paths to prepend + :param paths: list of paths to add :param prepend: whether to prepend (True) or append (False) paths :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) @@ -968,12 +972,12 @@ def msg_on_unload(self, msg): print_cmd = "puts stderr %s" % quote_str(msg, tcl=True) return '\n'.join(['', self.conditional_statement("module-info mode unload", print_cmd, indent=False)]) - def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): + def _update_paths(self, key: str, paths: list, prepend=True, allow_abs=False, expand_relpaths=True) -> str: """ Generate prepend-path or append-path statements for the given list of paths. :param key: environment variable to prepend/append paths to - :param paths: list of paths to prepend/append + :param paths: list of paths to add :param prepend: whether to prepend (True) or append (False) paths :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) @@ -1437,12 +1441,12 @@ def modulerc(self, module_version=None, filepath=None, modulerc_txt=None): return super(ModuleGeneratorLua, self).modulerc(module_version=module_version, filepath=filepath, modulerc_txt=modulerc_txt) - def _update_paths(self, key, paths, prepend=True, allow_abs=False, expand_relpaths=True): + def _update_paths(self, key: str, paths: list, prepend=True, allow_abs=False, expand_relpaths=True) -> str: """ Generate prepend_path or append_path statements for the given list of paths :param key: environment variable to prepend/append paths to - :param paths: list of paths to prepend/append + :param paths: list of paths to add :param prepend: whether to prepend (True) or append (False) paths :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) From f4fbe76588a703f885b66f47812549202089e700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sun, 21 Apr 2024 19:53:34 +0200 Subject: [PATCH 13/17] Improve typing hints for list of str Co-authored-by: Alexander Grund --- easybuild/tools/module_generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index fcc5ad7b81..a278c42f55 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -206,7 +206,7 @@ def get_modules_path(self, fake=False, mod_path_suffix=None): return os.path.join(mod_path, mod_path_suffix) - def _filter_paths(self, key: str, paths: list) -> list: + def _filter_paths(self, key: str, paths: List[str]) -> List[str]: """Filter out paths already added to key and return the remaining ones""" assert isinstance(paths, list) From 92f0e62903161789cac26387baccc1b46abc6fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sun, 21 Apr 2024 19:53:59 +0200 Subject: [PATCH 14/17] Also use List from typing Co-authored-by: Alexander Grund --- easybuild/tools/module_generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index a278c42f55..92f229a653 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -42,7 +42,7 @@ from contextlib import contextmanager from easybuild.tools import LooseVersion from textwrap import wrap -from typing import Union, Sequence +from typing import List, Union, Sequence from easybuild.base import fancylogger from easybuild.tools.build_log import EasyBuildError, print_warning From e91186e7c3aab4238757736b3dcaac987540d956 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sun, 21 Apr 2024 19:54:26 +0200 Subject: [PATCH 15/17] Improve typing hints for list of str Co-authored-by: Alexander Grund --- easybuild/tools/module_generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 92f229a653..ad88a63f87 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -1441,7 +1441,7 @@ def modulerc(self, module_version=None, filepath=None, modulerc_txt=None): return super(ModuleGeneratorLua, self).modulerc(module_version=module_version, filepath=filepath, modulerc_txt=modulerc_txt) - def _update_paths(self, key: str, paths: list, prepend=True, allow_abs=False, expand_relpaths=True) -> str: + def _update_paths(self, key: str, paths: List[str], prepend=True, allow_abs=False, expand_relpaths=True) -> str: """ Generate prepend_path or append_path statements for the given list of paths From 236c67f128ea0a2d54bd7684a60157420ae8ebed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sun, 21 Apr 2024 19:55:08 +0200 Subject: [PATCH 16/17] Improve typing hints for list of str Co-authored-by: Alexander Grund --- easybuild/tools/module_generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index ad88a63f87..7d14cc3458 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -568,7 +568,7 @@ def unload_module(self, mod_name): """ raise NotImplementedError - def _update_paths(self, key: str, paths: list, prepend=True, allow_abs=False, expand_relpaths=True) -> str: + def _update_paths(self, key: str, paths: List[str], prepend=True, allow_abs=False, expand_relpaths=True) -> str: """ Generate prepend-path or append-path statements for the given list of paths. From ca304fb4b1c5389f3785dbf7aabf1cadd1d58b2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikael=20=C3=96hman?= Date: Sun, 28 Apr 2024 00:02:42 +0200 Subject: [PATCH 17/17] Update easybuild/tools/module_generator.py Co-authored-by: Alexander Grund --- easybuild/tools/module_generator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index 7d14cc3458..03e17b8af2 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -263,7 +263,7 @@ def update_paths(self, key: str, paths: Union[str, Sequence[str]], prepend=True, paths = list(paths) # coerce any iterator into a list if key == 'PYTHONPATH': - python_paths = [path for path in paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] + python_paths = [path for path in paths if re.match(r'lib(64)?/python\d+\.\d+/site-packages', path)] if len(python_paths) > 1: raise EasyBuildError('Found multiple paths for PYTHONPATH: ' + ', '.join(python_paths))