detection: prefer dir instead of symlink in case of duplicate search paths (#46957)

lib/spack/spack/detection/path.py

@@ -66,6 +66,21 @@ def file_identifier(path):
     return s.st_dev, s.st_ino
 
 
+def dedupe_paths(paths: List[str]) -> List[str]:
+    """Deduplicate paths based on inode and device number. In case the list contains first a
+    symlink and then the directory it points to, the symlink is replaced with the directory path.
+    This ensures that we pick for example ``/usr/bin`` over ``/bin`` if the latter is a symlink to
+    the former`."""
+    seen: Dict[Tuple[int, int], str] = {}
+    for path in paths:
+        identifier = file_identifier(path)
+        if identifier not in seen:
+            seen[identifier] = path
+        elif not os.path.islink(path):
+            seen[identifier] = path
+    return list(seen.values())
+
+
 def executables_in_path(path_hints: List[str]) -> Dict[str, str]:
     """Get the paths of all executables available from the current PATH.
 
@@ -82,8 +97,7 @@ def executables_in_path(path_hints: List[str]) -> Dict[str, str]:
     """
     search_paths = llnl.util.filesystem.search_paths_for_executables(*path_hints)
     # Make use we don't doubly list /usr/lib and /lib etc
-    search_paths = list(llnl.util.lang.dedupe(search_paths, key=file_identifier))
+    return path_to_dict(dedupe_paths(search_paths))
-    return path_to_dict(search_paths)
 
 
 def accept_elf(path, host_compat):
@@ -144,7 +158,7 @@ def libraries_in_ld_and_system_library_path(
         search_paths = list(filter(os.path.isdir, search_paths))
 
     # Make use we don't doubly list /usr/lib and /lib etc
-    search_paths = list(llnl.util.lang.dedupe(search_paths, key=file_identifier))
+    search_paths = dedupe_paths(search_paths)
 
     try:
         host_compat = elf_utils.get_elf_compat(sys.executable)

lib/spack/spack/test/detection.py

@@ -7,6 +7,7 @@
 import spack.config
 import spack.detection
 import spack.detection.common
+import spack.detection.path
 import spack.spec
 
 
@@ -26,3 +27,28 @@ def test_detection_update_config(mutable_config):
     external_gcc = externals[0]
     assert external_gcc["spec"] == "cmake@3.27.5"
     assert external_gcc["prefix"] == "/usr/bin"
+
+
+def test_dedupe_paths(tmp_path):
+    """Test that ``dedupe_paths`` deals with symlinked directories, retaining the target"""
+    x = tmp_path / "x"
+    y = tmp_path / "y"
+    z = tmp_path / "z"
+
+    x.mkdir()
+    y.mkdir()
+    z.symlink_to("x", target_is_directory=True)
+
+    # dedupe repeated dirs, should preserve order
+    assert spack.detection.path.dedupe_paths([str(x), str(y), str(x)]) == [str(x), str(y)]
+    assert spack.detection.path.dedupe_paths([str(y), str(x), str(y)]) == [str(y), str(x)]
+
+    # dedupe repeated symlinks
+    assert spack.detection.path.dedupe_paths([str(z), str(y), str(z)]) == [str(z), str(y)]
+    assert spack.detection.path.dedupe_paths([str(y), str(z), str(y)]) == [str(y), str(z)]
+
+    # when both symlink and target are present, only target is retained, and it comes at the
+    # priority of the first occurrence.
+    assert spack.detection.path.dedupe_paths([str(x), str(y), str(z)]) == [str(x), str(y)]
+    assert spack.detection.path.dedupe_paths([str(z), str(y), str(x)]) == [str(x), str(y)]
+    assert spack.detection.path.dedupe_paths([str(y), str(z), str(x)]) == [str(y), str(x)]