Remove `DetectedPackage` class (#46071)

This PR simplifies the code doing external spec detection by removing 
the `DetectedPackage` class. Now, functions accepting or returning lists 
of `DetectedPackage`, will accept or return list of specs.

Performance doesn't seem to change if we use `Spec.__reduce__` instead 
of `DetectionPackage.__reduce__`.

9 files changed, 55 insertions, 95 deletions

diff --git a/lib/spack/spack/build_systems/python.py b/lib/spack/spack/build_systems/python.py
index 3504406253..969d5f5b03 100644
--- a/lib/spack/spack/build_systems/python.py
+++ b/lib/spack/spack/build_systems/python.py
@@ -315,9 +315,9 @@ class PythonExtension(spack.package_base.PackageBase):
         )
 
         python_externals_detected = [
-            d.spec
-            for d in python_externals_detection.get("python", [])
-            if d.prefix == self.spec.external_path
+            spec
+            for spec in python_externals_detection.get("python", [])
+            if spec.external_path == self.spec.external_path
         ]
         if python_externals_detected:
             return python_externals_detected[0]
diff --git a/lib/spack/spack/compilers/__init__.py b/lib/spack/spack/compilers/__init__.py
index 1d140b2b6b..aec829cd01 100644
--- a/lib/spack/spack/compilers/__init__.py
+++ b/lib/spack/spack/compilers/__init__.py
@@ -273,24 +273,24 @@ def find_compilers(
 
     valid_compilers = {}
     for name, detected in detected_packages.items():
-        compilers = [x for x in detected if CompilerConfigFactory.from_external_spec(x.spec)]
+        compilers = [x for x in detected if CompilerConfigFactory.from_external_spec(x)]
         if not compilers:
             continue
         valid_compilers[name] = compilers
 
     def _has_fortran_compilers(x):
-        if "compilers" not in x.spec.extra_attributes:
+        if "compilers" not in x.extra_attributes:
             return False
 
-        return "fortran" in x.spec.extra_attributes["compilers"]
+        return "fortran" in x.extra_attributes["compilers"]
 
     if mixed_toolchain:
         gccs = [x for x in valid_compilers.get("gcc", []) if _has_fortran_compilers(x)]
         if gccs:
             best_gcc = sorted(
-                gccs, key=lambda x: spack.spec.parse_with_version_concrete(x.spec).version
+                gccs, key=lambda x: spack.spec.parse_with_version_concrete(x).version
             )[-1]
-            gfortran = best_gcc.spec.extra_attributes["compilers"]["fortran"]
+            gfortran = best_gcc.extra_attributes["compilers"]["fortran"]
             for name in ("llvm", "apple-clang"):
                 if name not in valid_compilers:
                     continue
@@ -298,11 +298,11 @@ def find_compilers(
                 for candidate in candidates:
                     if _has_fortran_compilers(candidate):
                         continue
-                    candidate.spec.extra_attributes["compilers"]["fortran"] = gfortran
+                    candidate.extra_attributes["compilers"]["fortran"] = gfortran
 
     new_compilers = []
     for name, detected in valid_compilers.items():
-        for config in CompilerConfigFactory.from_specs([x.spec for x in detected]):
+        for config in CompilerConfigFactory.from_specs(detected):
             c = _compiler_from_config_entry(config["compiler"])
             if c in known_compilers:
                 continue
diff --git a/lib/spack/spack/detection/__init__.py b/lib/spack/spack/detection/__init__.py
index 1cff789c26..e2909d5c14 100644
--- a/lib/spack/spack/detection/__init__.py
+++ b/lib/spack/spack/detection/__init__.py
@@ -2,17 +2,11 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-from .common import (
-    DetectedPackage,
-    executable_prefix,
-    set_virtuals_nonbuildable,
-    update_configuration,
-)
+from .common import executable_prefix, set_virtuals_nonbuildable, update_configuration
 from .path import by_path, executables_in_path
 from .test import detection_tests
 
 __all__ = [
-    "DetectedPackage",
     "by_path",
     "executables_in_path",
     "executable_prefix",
diff --git a/lib/spack/spack/detection/common.py b/lib/spack/spack/detection/common.py
index eda7fda528..addcd1797d 100644
--- a/lib/spack/spack/detection/common.py
+++ b/lib/spack/spack/detection/common.py
@@ -6,9 +6,9 @@
 function to update packages.yaml given a list of detected packages.
 
 Ideally, each detection method should be placed in a specific subpackage
-and implement at least a function that returns a list of DetectedPackage
-objects. The update in packages.yaml can then be done using the function
-provided here.
+and implement at least a function that returns a list of specs.
+
+The update in packages.yaml can then be done using the function provided here.
 
 The module also contains other functions that might be useful across different
 detection mechanisms.
@@ -17,9 +17,10 @@ import glob
 import itertools
 import os
 import os.path
+import pathlib
 import re
 import sys
-from typing import Dict, List, NamedTuple, Optional, Set, Tuple, Union
+from typing import Dict, List, Optional, Set, Tuple, Union
 
 import llnl.util.tty
 
@@ -30,27 +31,6 @@ import spack.util.spack_yaml
 import spack.util.windows_registry
 
 
-class DetectedPackage(NamedTuple):
-    """Information on a package that has been detected."""
-
-    #: Spec that was detected
-    spec: spack.spec.Spec
-    #: Prefix of the spec
-    prefix: str
-
-    def __reduce__(self):
-        return DetectedPackage.restore, (str(self.spec), self.prefix, self.spec.extra_attributes)
-
-    @staticmethod
-    def restore(
-        spec_str: str, prefix: str, extra_attributes: Optional[Dict[str, str]]
-    ) -> "DetectedPackage":
-        spec = spack.spec.Spec.from_detection(
-            spec_str=spec_str, external_path=prefix, extra_attributes=extra_attributes
-        )
-        return DetectedPackage(spec=spec, prefix=prefix)
-
-
 def _externals_in_packages_yaml() -> Set[spack.spec.Spec]:
     """Returns all the specs mentioned as externals in packages.yaml"""
     packages_yaml = spack.config.get("packages")
@@ -65,7 +45,7 @@ ExternalEntryType = Union[str, Dict[str, str]]
 
 
 def _pkg_config_dict(
-    external_pkg_entries: List[DetectedPackage],
+    external_pkg_entries: List["spack.spec.Spec"],
 ) -> Dict[str, Union[bool, List[Dict[str, ExternalEntryType]]]]:
     """Generate a package specific config dict according to the packages.yaml schema.
 
@@ -85,22 +65,19 @@ def _pkg_config_dict(
     pkg_dict = spack.util.spack_yaml.syaml_dict()
     pkg_dict["externals"] = []
     for e in external_pkg_entries:
-        if not _spec_is_valid(e.spec):
+        if not _spec_is_valid(e):
             continue
 
         external_items: List[Tuple[str, ExternalEntryType]] = [
-            ("spec", str(e.spec)),
-            ("prefix", e.prefix),
+            ("spec", str(e)),
+            ("prefix", pathlib.Path(e.external_path).as_posix()),
         ]
-        if e.spec.external_modules:
-            external_items.append(("modules", e.spec.external_modules))
+        if e.external_modules:
+            external_items.append(("modules", e.external_modules))
 
-        if e.spec.extra_attributes:
+        if e.extra_attributes:
             external_items.append(
-                (
-                    "extra_attributes",
-                    spack.util.spack_yaml.syaml_dict(e.spec.extra_attributes.items()),
-                )
+                ("extra_attributes", spack.util.spack_yaml.syaml_dict(e.extra_attributes.items()))
             )
 
         # external_items.extend(e.spec.extra_attributes.items())
@@ -221,33 +198,32 @@ def library_prefix(library_dir: str) -> str:
 
 
 def update_configuration(
-    detected_packages: Dict[str, List[DetectedPackage]],
+    detected_packages: Dict[str, List["spack.spec.Spec"]],
     scope: Optional[str] = None,
     buildable: bool = True,
 ) -> List[spack.spec.Spec]:
     """Add the packages passed as arguments to packages.yaml
 
     Args:
-        detected_packages: list of DetectedPackage objects to be added
+        detected_packages: list of specs to be added
         scope: configuration scope where to add the detected packages
         buildable: whether the detected packages are buildable or not
     """
     predefined_external_specs = _externals_in_packages_yaml()
     pkg_to_cfg, all_new_specs = {}, []
     for package_name, entries in detected_packages.items():
-        new_entries = [e for e in entries if (e.spec not in predefined_external_specs)]
+        new_entries = [s for s in entries if s not in predefined_external_specs]
 
         pkg_config = _pkg_config_dict(new_entries)
         external_entries = pkg_config.get("externals", [])
         assert not isinstance(external_entries, bool), "unexpected value for external entry"
 
-        all_new_specs.extend([x.spec for x in new_entries])
+        all_new_specs.extend(new_entries)
         if buildable is False:
             pkg_config["buildable"] = False
         pkg_to_cfg[package_name] = pkg_config
 
     pkgs_cfg = spack.config.get("packages", scope=scope)
-
     pkgs_cfg = spack.config.merge_yaml(pkgs_cfg, pkg_to_cfg)
     spack.config.set("packages", pkgs_cfg, scope=scope)
 
diff --git a/lib/spack/spack/detection/path.py b/lib/spack/spack/detection/path.py
index 75d29fe1db..ec94b71f14 100644
--- a/lib/spack/spack/detection/path.py
+++ b/lib/spack/spack/detection/path.py
@@ -24,7 +24,6 @@ import spack.util.environment as environment
 import spack.util.ld_so_conf
 
 from .common import (
-    DetectedPackage,
     WindowsCompilerExternalPaths,
     WindowsKitExternalPaths,
     _convert_to_iterable,
@@ -229,7 +228,7 @@ class Finder:
 
     def detect_specs(
         self, *, pkg: Type["spack.package_base.PackageBase"], paths: List[str]
-    ) -> List[DetectedPackage]:
+    ) -> List["spack.spec.Spec"]:
         """Given a list of files matching the search patterns, returns a list of detected specs.
 
         Args:
@@ -295,16 +294,16 @@ class Finder:
                     warnings.warn(msg)
                     continue
 
-                if spec.external_path:
-                    prefix = spec.external_path
+                if not spec.external_path:
+                    spec.external_path = prefix
 
-                result.append(DetectedPackage(spec=spec, prefix=prefix))
+                result.append(spec)
 
         return result
 
     def find(
         self, *, pkg_name: str, repository, initial_guess: Optional[List[str]] = None
-    ) -> List[DetectedPackage]:
+    ) -> List["spack.spec.Spec"]:
         """For a given package, returns a list of detected specs.
 
         Args:
@@ -388,7 +387,7 @@ def by_path(
     *,
     path_hints: Optional[List[str]] = None,
     max_workers: Optional[int] = None,
-) -> Dict[str, List[DetectedPackage]]:
+) -> Dict[str, List["spack.spec.Spec"]]:
     """Return the list of packages that have been detected on the system, keyed by
     unqualified package name.
 
diff --git a/lib/spack/spack/detection/test.py b/lib/spack/spack/detection/test.py
index c2f0a5394b..1d56e8522c 100644
--- a/lib/spack/spack/detection/test.py
+++ b/lib/spack/spack/detection/test.py
@@ -68,7 +68,7 @@ class Runner:
         with self._mock_layout() as path_hints:
             entries = by_path([self.test.pkg_name], path_hints=path_hints)
             _, unqualified_name = spack.repo.partition_package_name(self.test.pkg_name)
-            specs = set(x.spec for x in entries[unqualified_name])
+            specs = set(entries[unqualified_name])
         return list(specs)
 
     @contextlib.contextmanager
diff --git a/lib/spack/spack/test/cmd/external.py b/lib/spack/spack/test/cmd/external.py
index a55b621de7..a186552987 100644
--- a/lib/spack/spack/test/cmd/external.py
+++ b/lib/spack/spack/test/cmd/external.py
@@ -44,7 +44,7 @@ def test_find_external_single_package(mock_executable):
 
     assert len(specs_by_package) == 1 and "cmake" in specs_by_package
     detected_spec = specs_by_package["cmake"]
-    assert len(detected_spec) == 1 and detected_spec[0].spec == Spec("cmake@1.foo")
+    assert len(detected_spec) == 1 and detected_spec[0] == Spec("cmake@1.foo")
 
 
 def test_find_external_two_instances_same_package(mock_executable):
@@ -61,10 +61,10 @@ def test_find_external_two_instances_same_package(mock_executable):
     )
 
     assert len(detected_specs) == 2
-    spec_to_path = {e.spec: e.prefix for e in detected_specs}
+    spec_to_path = {s: s.external_path for s in detected_specs}
     assert spec_to_path[Spec("cmake@1.foo")] == (
         spack.detection.executable_prefix(str(cmake1.parent))
-    )
+    ), spec_to_path
     assert spec_to_path[Spec("cmake@3.17.2")] == (
         spack.detection.executable_prefix(str(cmake2.parent))
     )
@@ -72,12 +72,8 @@ def test_find_external_two_instances_same_package(mock_executable):
 
 def test_find_external_update_config(mutable_config):
     entries = [
-        spack.detection.DetectedPackage(
-            Spec.from_detection("cmake@1.foo", external_path="/x/y1/"), "/x/y1/"
-        ),
-        spack.detection.DetectedPackage(
-            Spec.from_detection("cmake@3.17.2", external_path="/x/y2/"), "/x/y2/"
-        ),
+        Spec.from_detection("cmake@1.foo", external_path="/x/y1"),
+        Spec.from_detection("cmake@3.17.2", external_path="/x/y2"),
     ]
     pkg_to_entries = {"cmake": entries}
 
@@ -88,8 +84,8 @@ def test_find_external_update_config(mutable_config):
     cmake_cfg = pkgs_cfg["cmake"]
     cmake_externals = cmake_cfg["externals"]
 
-    assert {"spec": "cmake@1.foo", "prefix": "/x/y1/"} in cmake_externals
-    assert {"spec": "cmake@3.17.2", "prefix": "/x/y2/"} in cmake_externals
+    assert {"spec": "cmake@1.foo", "prefix": "/x/y1"} in cmake_externals
+    assert {"spec": "cmake@3.17.2", "prefix": "/x/y2"} in cmake_externals
 
 
 def test_get_executables(working_env, mock_executable):
@@ -229,19 +225,15 @@ def test_find_external_merge(mutable_config, mutable_mock_repo, tmp_path):
     """Checks that 'spack find external' doesn't overwrite an existing spec in packages.yaml."""
     pkgs_cfg_init = {
         "find-externals1": {
-            "externals": [{"spec": "find-externals1@1.1", "prefix": "/preexisting-prefix/"}],
+            "externals": [{"spec": "find-externals1@1.1", "prefix": "/preexisting-prefix"}],
             "buildable": False,
         }
     }
 
     mutable_config.update_config("packages", pkgs_cfg_init)
     entries = [
-        spack.detection.DetectedPackage(
-            Spec.from_detection("find-externals1@1.1", external_path="/x/y1/"), "/x/y1/"
-        ),
-        spack.detection.DetectedPackage(
-            Spec.from_detection("find-externals1@1.2", external_path="/x/y2/"), "/x/y2/"
-        ),
+        Spec.from_detection("find-externals1@1.1", external_path="/x/y1"),
+        Spec.from_detection("find-externals1@1.2", external_path="/x/y2"),
     ]
     pkg_to_entries = {"find-externals1": entries}
     scope = spack.config.default_modify_scope("packages")
@@ -251,8 +243,8 @@ def test_find_external_merge(mutable_config, mutable_mock_repo, tmp_path):
     pkg_cfg = pkgs_cfg["find-externals1"]
     pkg_externals = pkg_cfg["externals"]
 
-    assert {"spec": "find-externals1@1.1", "prefix": "/preexisting-prefix/"} in pkg_externals
-    assert {"spec": "find-externals1@1.2", "prefix": "/x/y2/"} in pkg_externals
+    assert {"spec": "find-externals1@1.1", "prefix": "/preexisting-prefix"} in pkg_externals
+    assert {"spec": "find-externals1@1.2", "prefix": "/x/y2"} in pkg_externals
 
 
 def test_list_detectable_packages(mutable_config, mutable_mock_repo):
@@ -278,7 +270,7 @@ def test_overriding_prefix(mock_executable, mutable_config, monkeypatch):
 
     assert len(detected_specs) == 1
 
-    gcc = detected_specs[0].spec
+    gcc = detected_specs[0]
     assert gcc.name == "gcc"
     assert gcc.external_path == os.path.sep + os.path.join("opt", "gcc", "bin")
 
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 6961605959..66ae350bcd 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -2109,11 +2109,13 @@ class TestConcretize:
         """Test that python extensions have access to a python dependency
 
         when python isn't otherwise in the DAG"""
-        python_spec = Spec("python@=detected")
         prefix = os.path.sep + "fake"
+        python_spec = Spec.from_detection("python@=detected", external_path=prefix)
 
         def find_fake_python(classes, path_hints):
-            return {"python": [spack.detection.DetectedPackage(python_spec, prefix=path_hints[0])]}
+            return {
+                "python": [Spec.from_detection("python@=detected", external_path=path_hints[0])]
+            }
 
         monkeypatch.setattr(spack.detection, "by_path", find_fake_python)
         external_conf = {
@@ -2128,7 +2130,8 @@ class TestConcretize:
 
         assert "python" in spec["py-extension1"]
         assert spec["python"].prefix == prefix
-        assert spec["python"] == python_spec
+        assert spec["python"].external
+        assert spec["python"].satisfies(python_spec)
 
     def test_external_python_extension_find_unified_python(self):
         """Test that python extensions use the same python as other specs in unified env"""
diff --git a/lib/spack/spack/test/detection.py b/lib/spack/spack/test/detection.py
index b83562e4e5..3fbc52fcbd 100644
--- a/lib/spack/spack/test/detection.py
+++ b/lib/spack/spack/test/detection.py
@@ -11,11 +11,7 @@ import spack.spec
 def test_detection_update_config(mutable_config):
     # mock detected package
     detected_packages = collections.defaultdict(list)
-    detected_packages["cmake"] = [
-        spack.detection.common.DetectedPackage(
-            spec=spack.spec.Spec("cmake@3.27.5"), prefix="/usr/bin"
-        )
-    ]
+    detected_packages["cmake"] = [spack.spec.Spec("cmake@3.27.5", external_path="/usr/bin")]
 
     # update config for new package
     spack.detection.common.update_configuration(detected_packages)