Avoid duplicate paths in make_module_req

Expand all globs and iterate over combined list of paths per key
This allows elimination of duplicate paths and greatly simplified code.

Author: Flamefire

easybuild/framework/easyblock.py

@@ -1291,58 +1291,57 @@ def make_module_req(self):
             note = "note: glob patterns are not expanded and existence checks "
             note += "for paths are skipped for the statements below due to dry run"
             lines.append(self.module_generator.comment(note))
-        else:
-            lib64_is_symlink = (all(os.path.isdir(path) for path in ['lib', 'lib64'])
-                                and os.path.samefile('lib', 'lib64'))
 
         # for these environment variables, the corresponding subdirectory must include at least one file
-        keys_requiring_files = ('CPATH', 'LD_LIBRARY_PATH', 'LIBRARY_PATH', 'PATH', 'CMAKE_LIBRARY_PATH')
+        keys_requiring_files = {'CPATH', 'LD_LIBRARY_PATH', 'LIBRARY_PATH', 'PATH', 'CMAKE_LIBRARY_PATH'}
 
         for key, reqs in sorted(requirements.items()):
             if isinstance(reqs, string_type):
                 self.log.warning("Hoisting string value %s into a list before iterating over it", reqs)
                 reqs = [reqs]
             if self.dry_run:
                 self.dry_run_msg(" $%s: %s" % (key, ', '.join(reqs)))
-
-            for path in reqs:
-                # only use glob if the string is non-empty
-                if path and not self.dry_run:
-                    paths = glob.glob(path)
-                    # If lib64 is just a symlink to lib we fixup the paths to avoid duplicates
-                    if lib64_is_symlink:
-                        fixed_paths = []
-                        for path in paths:
-                            if (path + os.path.sep).startswith('lib64' + os.path.sep):
-                                # We only need CMAKE_LIBRARY_PATH if there is a separate lib64 path
-                                if key == 'CMAKE_LIBRARY_PATH':
-                                    continue
-                                path = path.replace('lib64', 'lib', 1)
-                            fixed_paths.append(path)
-                        if fixed_paths != paths:
-                            self.log.info("Fixed symlink lib64 in paths for %s: %s -> %s",
-                                            key, paths, fixed_paths)
-                            paths = fixed_paths
-                    # Use a set to remove duplicates
-                    paths = sorted(set(paths))
-                    if paths and key in keys_requiring_files:
-                        # only retain paths that contain at least one file
-                        retained_paths = [
-                            path for path in paths
-                            if os.path.isdir(os.path.join(self.installdir, path))
-                            and dir_contains_files(os.path.join(self.installdir, path))
-                        ]
+                # Don't expand globs or do any filtering below for dry run
+                paths = sorted(reqs)
+            else:
+                # Expand globs but only if the string is non-empty
+                # empty string is a valid value here (i.e. to prepend the installation prefix, cfr $CUDA_HOME)
+                paths = sorted(sum((glob.glob(path) if path else [path] for path in reqs), []))  # sum flattens to list
+
+                # If lib64 is just a symlink to lib we fixup the paths to avoid duplicates
+                lib64_is_symlink = (all(os.path.isdir(path) for path in ['lib', 'lib64'])
+                                    and os.path.samefile('lib', 'lib64'))
+                if lib64_is_symlink:
+                    fixed_paths = []
+                    for path in paths:
+                        if (path + os.path.sep).startswith('lib64' + os.path.sep):
+                            # We only need CMAKE_LIBRARY_PATH if there is a separate lib64 path, so skip symlink
+                            if key == 'CMAKE_LIBRARY_PATH':
+                                continue
+                            path = path.replace('lib64', 'lib', 1)
+                        fixed_paths.append(path)
+                    if fixed_paths != paths:
+                        self.log.info("Fixed symlink lib64 in paths for %s: %s -> %s", key, paths, fixed_paths)
+                        paths = fixed_paths
+                # Use a set to remove duplicates, e.g. by having lib64 and lib which get fixed to lib and lib above
+                paths = sorted(set(paths))
+                if key in keys_requiring_files:
+                    # only retain paths that contain at least one file
+                    retained_paths = [
+                        path for path in paths
+                        if os.path.isdir(os.path.join(self.installdir, path))
+                        and dir_contains_files(os.path.join(self.installdir, path))
+                    ]
+                    if retained_paths != paths:
                         self.log.info("Only retaining paths for %s that contain at least one file: %s -> %s",
-                                        key, paths, retained_paths)
+                                      key, paths, retained_paths)
                         paths = retained_paths
-                else:
-                    # empty string is a valid value here (i.e. to prepend the installation prefix, cfr $CUDA_HOME)
-                    paths = [path]
 
+            if paths:
                 lines.append(self.module_generator.prepend_paths(key, paths))
         if self.dry_run:
             self.dry_run_msg('')
-        
+
         if old_dir is not None:
             change_dir(old_dir)
 

test/framework/easyblock.py

@@ -386,6 +386,17 @@ def test_make_module_req(self):
         elif get_module_syntax() == 'Lua':
             self.assertFalse('prepend_path("CMAKE_LIBRARY_PATH", pathJoin(root, "lib64"))' in guess)
 
+        # With files in /lib and /lib64 symlinked to /lib there should be exactly 1 entry for (LD_)LIBRARY_PATH
+        # pointing to /lib
+        for var in ('LIBRARY_PATH', 'LD_LIBRARY_PATH'):
+            if get_module_syntax() == 'Tcl':
+                self.assertFalse(re.search(r"^prepend-path\s+%s\s+\$root/lib64$" % var, guess, re.M))
+                self.assertEqual(len(re.findall(r"^prepend-path\s+%s\s+\$root/lib$" % var, guess, re.M)), 1)
+            elif get_module_syntax() == 'Lua':
+                self.assertFalse(re.search(r'^prepend_path\("%s", pathJoin\(root, "lib64"\)\)$' % var, guess, re.M))
+                self.assertEqual(len(re.findall(r'^prepend_path\("%s", pathJoin\(root, "lib"\)\)$' % var,
+                                                guess, re.M)), 1)
+
         # check for behavior when a string value is used as dict value by make_module_req_guesses
         eb.make_module_req_guess = lambda: {'PATH': 'bin'}
         txt = eb.make_module_req()