Merge pull request #4686 from Flamefire/patch-1

simplify code for determining the PYTHONPATH module entries

Author: Micket

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, warn_exists=False)
+        else:
+            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):
         """

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,15 +231,17 @@ 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
 
-    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.
 
@@ -244,14 +250,15 @@ 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,
                                  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 +267,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,

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"""
@@ -882,14 +885,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