From 653a28e7ec047f26c84c6236026ebeb1db16b1b8 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sun, 20 Oct 2024 16:40:55 +0200 Subject: [PATCH 1/4] simplify code for determining the PYTHONPAH module entries The check whether the path(s) have already been added is redundant as that is done in the module_generator class. The whole code with all the checks is superflous if there are no python paths to add. To avoid excessive indentation return early with an empty list in that case and same for Python installations for consistency. --- easybuild/framework/easyblock.py | 55 +++++++++++++++----------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 5ac1ee8ca9..0c3de5f009 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1399,43 +1399,40 @@ def make_module_pythonpath(self): Add lines for module file to update $PYTHONPATH or $EBPYTHONPREFIXES, if they aren't already present and the standard lib/python*/site-packages subdirectory exists """ - lines = [] - if not os.path.isfile(os.path.join(self.installdir, 'bin', 'python')): # only needed when not a python install - python_subdir_pattern = os.path.join(self.installdir, 'lib', 'python*', 'site-packages') - candidate_paths = (os.path.relpath(path, self.installdir) for path in glob.glob(python_subdir_pattern)) - python_paths = [path for path in candidate_paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] + if os.path.isfile(os.path.join(self.installdir, 'bin', 'python')): # only needed when not a python install + return [] - # determine whether Python is a runtime dependency; - # if so, we assume it was installed with EasyBuild, and hence is aware of $EBPYTHONPREFIXES - runtime_deps = [dep['name'] for dep in self.cfg.dependencies(runtime_only=True)] + python_subdir_pattern = os.path.join(self.installdir, 'lib', 'python*', 'site-packages') + candidate_paths = (os.path.relpath(path, self.installdir) for path in glob.glob(python_subdir_pattern)) + python_paths = [path for path in candidate_paths if re.match(r'lib/python\d+\.\d+/site-packages', path)] + if not python_paths: + return [] - # don't use $EBPYTHONPREFIXES unless we can and it's preferred or necesary (due to use of multi_deps) - use_ebpythonprefixes = False - multi_deps = self.cfg['multi_deps'] + # determine whether Python is a runtime dependency; + # if so, we assume it was installed with EasyBuild, and hence is aware of $EBPYTHONPREFIXES + runtime_deps = [dep['name'] for dep in self.cfg.dependencies(runtime_only=True)] - if 'Python' in runtime_deps: - self.log.info("Found Python runtime dependency, so considering $EBPYTHONPREFIXES...") + # don't use $EBPYTHONPREFIXES unless we can and it's preferred or necesary (due to use of multi_deps) + use_ebpythonprefixes = False + multi_deps = self.cfg['multi_deps'] - if build_option('prefer_python_search_path') == EBPYTHONPREFIXES: - self.log.info("Preferred Python search path is $EBPYTHONPREFIXES, so using that") - use_ebpythonprefixes = True + if 'Python' in runtime_deps: + self.log.info("Found Python runtime dependency, so considering $EBPYTHONPREFIXES...") - elif multi_deps and 'Python' in multi_deps: - self.log.info("Python is listed in 'multi_deps', so using $EBPYTHONPREFIXES instead of $PYTHONPATH") + if build_option('prefer_python_search_path') == EBPYTHONPREFIXES: + self.log.info("Preferred Python search path is $EBPYTHONPREFIXES, so using that") use_ebpythonprefixes = True - if python_paths: - # add paths unless they were already added - if use_ebpythonprefixes: - path = '' # EBPYTHONPREFIXES are relative to the install dir - if path not in self.module_generator.added_paths_per_key[EBPYTHONPREFIXES]: - lines.append(self.module_generator.prepend_paths(EBPYTHONPREFIXES, path)) - else: - for python_path in python_paths: - if python_path not in self.module_generator.added_paths_per_key[PYTHONPATH]: - lines.append(self.module_generator.prepend_paths(PYTHONPATH, python_path)) + elif multi_deps and 'Python' in multi_deps: + self.log.info("Python is listed in 'multi_deps', so using $EBPYTHONPREFIXES instead of $PYTHONPATH") + use_ebpythonprefixes = True - return lines + if use_ebpythonprefixes: + path = '' # EBPYTHONPREFIXES are relative to the install dir + lines = self.module_generator.prepend_paths(EBPYTHONPREFIXES, path) + else: + lines = self.module_generator.prepend_paths(PYTHONPATH, python_paths) + return [lines] if lines else [] def make_module_extra(self, altroot=None, altversion=None): """ From faf549b07a59b747418d57bb9ec8db5a49296ec1 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 8 Nov 2024 11:36:13 +0100 Subject: [PATCH 2/4] Add parameter to supress the warning when adding a path twice to a module environment variable --- easybuild/tools/module_generator.py | 23 +++++++++++++++-------- test/framework/module_generator.py | 12 ++++++++---- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index c0bfcd95b3..c017f13edf 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -206,8 +206,12 @@ 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): - """Filter out paths already added to key and return the remaining ones""" + def _filter_paths(self, key, paths, warn_exists=True): + """ + Filter out paths already added to key and return the remaining ones + + :param warn_exists: Show a warning for paths already added to the key + """ 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!') @@ -227,10 +231,12 @@ def _filter_paths(self, key, paths): 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, removed_paths, - log=self.log) + if warn_exists: + 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, + log=self.log) if not filtered_paths: filtered_paths = None return filtered_paths @@ -251,7 +257,7 @@ def append_paths(self, key, paths, allow_abs=False, expand_relpaths=True, delim= return self.update_paths(key, paths, prepend=False, allow_abs=allow_abs, expand_relpaths=expand_relpaths, delim=delim) - def prepend_paths(self, key, paths, allow_abs=False, expand_relpaths=True, delim=':'): + def prepend_paths(self, key, paths, allow_abs=False, expand_relpaths=True, delim=':', warn_exists=True): """ Generate prepend-path statements for the given list of paths. @@ -260,8 +266,9 @@ def prepend_paths(self, key, paths, allow_abs=False, expand_relpaths=True, delim :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) :param delim: delimiter used between paths + :param warn_exists: Show a warning if any path was already added to the variable """ - paths = self._filter_paths(key, paths) + paths = self._filter_paths(key, paths, warn_exists=warn_exists) if paths is None: return '' return self.update_paths(key, paths, prepend=True, allow_abs=allow_abs, expand_relpaths=expand_relpaths, diff --git a/test/framework/module_generator.py b/test/framework/module_generator.py index a3b4a835f5..94c90a45a7 100644 --- a/test/framework/module_generator.py +++ b/test/framework/module_generator.py @@ -882,14 +882,18 @@ def prepend_paths(*args, **kwargs): # check for warning that is printed when same path is added multiple times with self.modgen.start_module_creation(): self.modgen.prepend_paths('TEST', 'path1') - self.mock_stderr(True) - self.modgen.prepend_paths('TEST', 'path1') - stderr = self.get_stderr() - self.mock_stderr(False) + with self.mocked_stdout_stderr(): + self.modgen.prepend_paths('TEST', 'path1') + stderr = self.get_stderr() expected_warning = "\nWARNING: Suppressed adding the following path(s) to $TEST of the module " expected_warning += "as they were already added: path1\n\n" self.assertEqual(stderr, expected_warning) + with self.mocked_stdout_stderr(): + self.modgen.prepend_paths('TEST', 'path1', warn_exists=False) + stderr = self.get_stderr() + self.assertEqual(stderr, '') + def test_det_user_modpath(self): """Test for generic det_user_modpath method.""" # None by default From beeceeda342e80192826af660242caf79394aa72 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 8 Nov 2024 11:37:38 +0100 Subject: [PATCH 3/4] Avoid warning in `make_module_pythonpath` --- easybuild/framework/easyblock.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/framework/easyblock.py b/easybuild/framework/easyblock.py index 0c3de5f009..5fcd22e3d2 100644 --- a/easybuild/framework/easyblock.py +++ b/easybuild/framework/easyblock.py @@ -1429,9 +1429,9 @@ def make_module_pythonpath(self): if use_ebpythonprefixes: path = '' # EBPYTHONPREFIXES are relative to the install dir - lines = self.module_generator.prepend_paths(EBPYTHONPREFIXES, path) + lines = self.module_generator.prepend_paths(EBPYTHONPREFIXES, path, warn_exists=False) else: - lines = self.module_generator.prepend_paths(PYTHONPATH, python_paths) + lines = self.module_generator.prepend_paths(PYTHONPATH, python_paths, warn_exists=False) return [lines] if lines else [] def make_module_extra(self, altroot=None, altversion=None): From 0c949e73245a43d5ccbdb99a87be7b61ad3482dc Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 18 Nov 2024 16:03:52 +0100 Subject: [PATCH 4/4] Add `warn_exists` parameter also to `append_paths` --- easybuild/tools/module_generator.py | 5 +++-- test/framework/module_generator.py | 11 +++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/easybuild/tools/module_generator.py b/easybuild/tools/module_generator.py index c017f13edf..83a870a77b 100644 --- a/easybuild/tools/module_generator.py +++ b/easybuild/tools/module_generator.py @@ -241,7 +241,7 @@ def _filter_paths(self, key, paths, warn_exists=True): filtered_paths = None return filtered_paths - def append_paths(self, key, paths, allow_abs=False, expand_relpaths=True, delim=':'): + def append_paths(self, key, paths, allow_abs=False, expand_relpaths=True, delim=':', warn_exists=True): """ Generate append-path statements for the given list of paths. @@ -250,8 +250,9 @@ def append_paths(self, key, paths, allow_abs=False, expand_relpaths=True, delim= :param allow_abs: allow providing of absolute paths :param expand_relpaths: expand relative paths into absolute paths (by prefixing install dir) :param delim: delimiter used between paths + :param warn_exists: Show a warning if any path was already added to the variable """ - paths = self._filter_paths(key, paths) + paths = self._filter_paths(key, paths, warn_exists=warn_exists) if paths is None: return '' return self.update_paths(key, paths, prepend=False, allow_abs=allow_abs, expand_relpaths=expand_relpaths, diff --git a/test/framework/module_generator.py b/test/framework/module_generator.py index 94c90a45a7..2d0c105e46 100644 --- a/test/framework/module_generator.py +++ b/test/framework/module_generator.py @@ -782,13 +782,16 @@ def append_paths(*args, **kwargs): # check for warning that is printed when same path is added multiple times with self.modgen.start_module_creation(): self.modgen.append_paths('TEST', 'path1') - self.mock_stderr(True) - self.modgen.append_paths('TEST', 'path1') - stderr = self.get_stderr() - self.mock_stderr(False) + with self.mocked_stdout_stderr(): + self.modgen.append_paths('TEST', 'path1') + stderr = self.get_stderr() expected_warning = "\nWARNING: Suppressed adding the following path(s) to $TEST of the module " expected_warning += "as they were already added: path1\n\n" self.assertEqual(stderr, expected_warning) + with self.mocked_stdout_stderr(): + self.modgen.append_paths('TEST', 'path1', warn_exists=False) + stderr = self.get_stderr() + self.assertEqual(stderr, '') def test_module_extensions(self): """test the extensions() for extensions"""