PackageBase: revamp libs and headers API

Add `find_libs`, `find_headers`, `query_libs`, and `query_headers` to
`PackageBase` and implement `SpecBuildInterface` in terms of those.

The old style `libs` and `headers` properties are deprecated but take
priority over the new style `find_libs` and `find_headers` methods.

lib/spack/spack/package_base.py

@@ -22,9 +22,22 @@
 import textwrap
 import time
 import traceback
-from typing import Any, Callable, Dict, Iterable, List, Optional, Set, Tuple, Type, TypeVar, Union
+from typing import (
+    Any,
+    Callable,
+    Dict,
+    Iterable,
+    List,
+    Optional,
+    Sequence,
+    Set,
+    Tuple,
+    Type,
+    TypeVar,
+    Union,
+)
 
-from typing_extensions import Literal
+from typing_extensions import Literal, final
 
 import llnl.util.filesystem as fsys
 import llnl.util.tty as tty
@@ -1381,6 +1394,75 @@ def command(self) -> spack.util.executable.Executable:
             return spack.util.executable.Executable(path)
         raise RuntimeError(f"Unable to locate {self.spec.name} command in {self.home.bin}")
 
+    def find_headers(
+        self, *, features: Sequence[str] = (), virtual: Optional[str] = None
+    ) -> fsys.HeaderList:
+        """Return the header list for this package based on the query. This method can be
+        overridden by individual packages to return package specific headers.
+
+        Args:
+            features: query argument to filter or extend the header list.
+            virtual: when set, return headers relevant for the virtual provided by this package.
+        Raises:
+            spack.error.NoHeadersError: if there was an error locating the headers.
+        """
+        spec = self.spec
+        home = self.home
+        headers = fsys.find_headers("*", root=home.include, recursive=True)
+
+        if headers:
+            return headers
+        raise spack.error.NoHeadersError(f"Unable to locate {spec.name} headers in {home}")
+
+    def find_libs(
+        self, *, features: Sequence[str] = (), virtual: Optional[str] = None
+    ) -> fsys.LibraryList:
+        """Return the library list for this package based on the query. This method can be
+        overridden by individual packages to return package specific libraries.
+
+        Args:
+            features: query argument to filter or extend the library list.
+            virtual: when set, return libraries relevant for the virtual provided by this package.
+        Raises:
+            spack.error.NoLibrariesError: if there was an error locating the libraries.
+        """
+        spec = self.spec
+        home = self.home
+        name = self.spec.name.replace("-", "?")
+
+        # Avoid double 'lib' for packages whose names already start with lib
+        if not name.startswith("lib") and not spec.satisfies("platform=windows"):
+            name = "lib" + name
+
+        # If '+shared' search only for shared library; if '~shared' search only for
+        # static library; otherwise, first search for shared and then for static.
+        search_shared = (
+            [True] if ("+shared" in spec) else ([False] if ("~shared" in spec) else [True, False])
+        )
+
+        for shared in search_shared:
+            # Since we are searching for link libraries, on Windows search only for
+            # ".Lib" extensions by default as those represent import libraries for implicit links.
+            libs = fsys.find_libraries(name, home, shared=shared, recursive=True, runtime=False)
+            if libs:
+                return libs
+
+        raise spack.error.NoLibrariesError(
+            f"Unable to recursively locate {spec.name} libraries in {home}"
+        )
+
+    @final
+    def query_headers(self, name: str, *, features: Sequence[str] = ()) -> fsys.HeaderList:
+        """Returns the header list for a dependency ``name``."""
+        spec, is_virtual = self.spec._get_dependency_by_name(name)
+        return spec.package.find_headers(features=features, virtual=name if is_virtual else None)
+
+    @final
+    def query_libs(self, name: str, *, features: Sequence[str] = ()) -> fsys.LibraryList:
+        """Returns the library list for a dependency ``name``."""
+        spec, is_virtual = self.spec._get_dependency_by_name(name)
+        return spec.package.find_libs(features=features, virtual=name if is_virtual else None)
+
     def url_version(self, version):
         """
         Given a version, this returns a string that should be substituted

lib/spack/spack/spec.py

@@ -1070,123 +1070,26 @@ def clear(self):
         self.edges.clear()
 
 
-def _headers_default_handler(spec: "Spec"):
-    """Default handler when looking for the 'headers' attribute.
-
-    Tries to search for ``*.h`` files recursively starting from
-    ``spec.package.home.include``.
-
-    Parameters:
-        spec: spec that is being queried
-
-    Returns:
-        HeaderList: The headers in ``prefix.include``
-
-    Raises:
-        NoHeadersError: If no headers are found
-    """
-    home = getattr(spec.package, "home")
-    headers = fs.find_headers("*", root=home.include, recursive=True)
-
-    if headers:
-        return headers
-    raise spack.error.NoHeadersError(f"Unable to locate {spec.name} headers in {home}")
-
-
-def _libs_default_handler(spec: "Spec"):
-    """Default handler when looking for the 'libs' attribute.
-
-    Tries to search for ``lib{spec.name}`` recursively starting from
-    ``spec.package.home``. If ``spec.name`` starts with ``lib``, searches for
-    ``{spec.name}`` instead.
-
-    Parameters:
-        spec: spec that is being queried
-
-    Returns:
-        LibraryList: The libraries found
-
-    Raises:
-        NoLibrariesError: If no libraries are found
-    """
-
-    # Variable 'name' is passed to function 'find_libraries', which supports
-    # glob characters. For example, we have a package with a name 'abc-abc'.
-    # Now, we don't know if the original name of the package is 'abc_abc'
-    # (and it generates a library 'libabc_abc.so') or 'abc-abc' (and it
-    # generates a library 'libabc-abc.so'). So, we tell the function
-    # 'find_libraries' to give us anything that matches 'libabc?abc' and it
-    # gives us either 'libabc-abc.so' or 'libabc_abc.so' (or an error)
-    # depending on which one exists (there is a possibility, of course, to
-    # get something like 'libabcXabc.so, but for now we consider this
-    # unlikely).
-    name = spec.name.replace("-", "?")
-    home = getattr(spec.package, "home")
-
-    # Avoid double 'lib' for packages whose names already start with lib
-    if not name.startswith("lib") and not spec.satisfies("platform=windows"):
-        name = "lib" + name
-
-    # If '+shared' search only for shared library; if '~shared' search only for
-    # static library; otherwise, first search for shared and then for static.
-    search_shared = (
-        [True] if ("+shared" in spec) else ([False] if ("~shared" in spec) else [True, False])
-    )
-
-    for shared in search_shared:
-        # Since we are searching for link libraries, on Windows search only for
-        # ".Lib" extensions by default as those represent import libraries for implicit links.
-        libs = fs.find_libraries(name, home, shared=shared, recursive=True, runtime=False)
-        if libs:
-            return libs
-
-    raise spack.error.NoLibrariesError(
-        f"Unable to recursively locate {spec.name} libraries in {home}"
-    )
-
-
 class ForwardQueryToPackage:
     """Descriptor used to forward queries from Spec to Package"""
 
-    def __init__(
-        self,
-        attribute_name: str,
-        default_handler: Optional[Callable[["Spec"], Any]] = None,
-        _indirect: bool = False,
-    ) -> None:
+    def __init__(self, attribute_name: str, _indirect: bool = False) -> None:
         """Create a new descriptor.
 
         Parameters:
             attribute_name: name of the attribute to be searched for in the Package instance
-            default_handler: default function to be called if the attribute was not found in the
-                Package instance
             _indirect: temporarily added to redirect a query to another package.
         """
         self.attribute_name = attribute_name
-        self.default = default_handler
         self.indirect = _indirect
 
     def __get__(self, instance: "SpecBuildInterface", cls):
-        """Retrieves the property from Package using a well defined chain
-        of responsibility.
+        """Retrieves the property from Package using a well defined chain of responsibility.
 
-        The order of call is:
+        The call order is:
 
-        1. if the query was through the name of a virtual package try to
-            search for the attribute `{virtual_name}_{attribute_name}`
-            in Package
-
-        2. try to search for attribute `{attribute_name}` in Package
-
-        3. try to call the default handler
-
-        The first call that produces a value will stop the chain.
-
-        If no call can handle the request then AttributeError is raised with a
-        message indicating that no relevant attribute exists.
-        If a call returns None, an AttributeError is raised with a message
-        indicating a query failure, e.g. that library files were not found in a
-        'libs' query.
+        1. `pkg.{virtual_name}_{attribute_name}` if the query is for a virtual package
+        2. `pkg.{attribute_name}` otherwise
         """
         # TODO: this indirection exist solely for `spec["python"].command` to actually return
         # spec["python-venv"].command. It should be removed when `python` is a virtual.
@@ -1202,61 +1105,36 @@ def __get__(self, instance: "SpecBuildInterface", cls):
             _ = instance.wrapped_obj[instance.wrapped_obj.name]  # NOQA: ignore=F841
             query = instance.last_query
 
-        callbacks_chain = []
-        # First in the chain : specialized attribute for virtual packages
+        # First try the deprecated attributes (e.g. `<virtual>_libs` and `libs`)
         if query.isvirtual:
-            specialized_name = "{0}_{1}".format(query.name, self.attribute_name)
-            callbacks_chain.append(lambda: getattr(pkg, specialized_name))
-        # Try to get the generic method from Package
-        callbacks_chain.append(lambda: getattr(pkg, self.attribute_name))
-        # Final resort : default callback
-        if self.default is not None:
-            _default = self.default  # make mypy happy
-            callbacks_chain.append(lambda: _default(instance.wrapped_obj))
-
-        # Trigger the callbacks in order, the first one producing a
-        # value wins
-        value = None
-        message = None
-        for f in callbacks_chain:
-            try:
-                value = f()
-                # A callback can return None to trigger an error indicating
-                # that the query failed.
-                if value is None:
-                    msg = "Query of package '{name}' for '{attrib}' failed\n"
-                    msg += "\tprefix : {spec.prefix}\n"
-                    msg += "\tspec : {spec}\n"
-                    msg += "\tqueried as : {query.name}\n"
-                    msg += "\textra parameters : {query.extra_parameters}"
-                    message = msg.format(
-                        name=pkg.name,
-                        attrib=self.attribute_name,
-                        spec=instance,
-                        query=instance.last_query,
-                    )
+            deprecated_attrs = [f"{query.name}_{self.attribute_name}", self.attribute_name]
         else:
+            deprecated_attrs = [self.attribute_name]
+
+        for attr in deprecated_attrs:
+            if not hasattr(pkg, attr):
+                continue
+            value = getattr(pkg, attr)
+            # Deprecated properties can return None to indicate the query failed.
+            if value is None:
+                raise AttributeError(
+                    f"Query of package '{pkg.name}' for '{self.attribute_name}' failed\n"
+                    f"\tprefix : {instance.prefix}\n"  # type: ignore[attr-defined]
+                    f"\tspec : {instance}\n"
+                    f"\tqueried as : {query.name}\n"
+                    f"\textra parameters : {query.extra_parameters}"
+                )
             return value
-                break
-            except AttributeError:
-                pass
-        # value is 'None'
-        if message is not None:
-            # Here we can use another type of exception. If we do that, the
-            # unit test 'test_getitem_exceptional_paths' in the file
-            # lib/spack/spack/test/spec_dag.py will need to be updated to match
-            # the type.
-            raise AttributeError(message)
-        # 'None' value at this point means that there are no appropriate
-        # properties defined and no default handler, or that all callbacks
-        # raised AttributeError. In this case, we raise AttributeError with an
-        # appropriate message.
-        fmt = "'{name}' package has no relevant attribute '{query}'\n"
-        fmt += "\tspec : '{spec}'\n"
-        fmt += "\tqueried as : '{spec.last_query.name}'\n"
-        fmt += "\textra parameters : '{spec.last_query.extra_parameters}'\n"
-        message = fmt.format(name=pkg.name, query=self.attribute_name, spec=instance)
-        raise AttributeError(message)
+
+        # Then try the new functions (e.g. `find_libs`).
+        features = query.extra_parameters
+        virtual = query.name if query.isvirtual else None
+        if self.attribute_name == "libs":
+            return pkg.find_libs(features=features, virtual=virtual)
+        elif self.attribute_name == "headers":
+            return pkg.find_headers(features=features, virtual=virtual)
+
+        raise AttributeError(f"Package {pkg.name} has no attribute {self.attribute_name}")
 
     def __set__(self, instance, value):
         cls_name = type(instance).__name__
@@ -1270,10 +1148,10 @@ def __set__(self, instance, value):
 
 class SpecBuildInterface(lang.ObjectWrapper):
     # home is available in the base Package so no default is needed
-    home = ForwardQueryToPackage("home", default_handler=None)
-    headers = ForwardQueryToPackage("headers", default_handler=_headers_default_handler)
-    libs = ForwardQueryToPackage("libs", default_handler=_libs_default_handler)
-    command = ForwardQueryToPackage("command", default_handler=None, _indirect=True)
+    home = ForwardQueryToPackage("home")
+    headers = ForwardQueryToPackage("headers")
+    libs = ForwardQueryToPackage("libs")
+    command = ForwardQueryToPackage("command", _indirect=True)
 
     def __init__(
         self,
@@ -3642,6 +3520,21 @@ def version(self):
             raise spack.error.SpecError("Spec version is not concrete: " + str(self))
         return self.versions[0]
 
+    def _get_dependency_by_name(self, name: str) -> Tuple["Spec", bool]:
+        """Get a dependency by package name or virtual. Returns a tuple with the matching spec
+        and a boolean indicating if the spec is a virtual dependency. Raises a KeyError if the
+        dependency is not found."""
+        # Consider all direct dependencies and transitive runtime dependencies
+        order = itertools.chain(
+            self.edges_to_dependencies(depflag=dt.BUILD | dt.TEST),
+            self.traverse_edges(deptype=dt.LINK | dt.RUN, order="breadth", cover="edges"),
+        )
+
+        edge = next((e for e in order if e.spec.name == name or name in e.virtuals), None)
+        if edge is None:
+            raise KeyError(f"No spec with name {name} in {self}")
+        return edge.spec, name in edge.virtuals
+
     def __getitem__(self, name: str):
         """Get a dependency from the spec by its name. This call implicitly
         sets a query state in the package being retrieved. The behavior of
@@ -3662,23 +3555,14 @@ def __getitem__(self, name: str):
             csv = query_parameters.pop().strip()
             query_parameters = re.split(r"\s*,\s*", csv)
 
-        # Consider all direct dependencies and transitive runtime dependencies
-        order = itertools.chain(
-            self.edges_to_dependencies(depflag=dt.BUILD | dt.TEST),
-            self.traverse_edges(deptype=dt.LINK | dt.RUN, order="breadth", cover="edges"),
-        )
-
-        try:
-            edge = next((e for e in order if e.spec.name == name or name in e.virtuals))
-        except StopIteration as e:
-            raise KeyError(f"No spec with name {name} in {self}") from e
+        spec, is_virtual = self._get_dependency_by_name(name)
 
         if self._concrete:
             return SpecBuildInterface(
-                edge.spec, name, query_parameters, _parent=self, is_virtual=name in edge.virtuals
+                spec, name, query_parameters, _parent=self, is_virtual=is_virtual
             )
 
-        return edge.spec
+        return spec
 
     def __contains__(self, spec):
         """True if this spec or some dependency satisfies the spec.