From 818c9aeb5a3cbf3e83ddc98f21f2ee7405e2fa8d Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 5 Sep 2023 14:19:57 +0200 Subject: Add type-hints to the `spack.detection` package (#39803) --- lib/spack/spack/detection/common.py | 119 ++++++++++++++++++++++-------------- lib/spack/spack/detection/path.py | 47 ++++++++------ 2 files changed, 99 insertions(+), 67 deletions(-) diff --git a/lib/spack/spack/detection/common.py b/lib/spack/spack/detection/common.py index 41455e43ae..5bed78711c 100644 --- a/lib/spack/spack/detection/common.py +++ b/lib/spack/spack/detection/common.py @@ -13,13 +13,13 @@ provided here. The module also contains other functions that might be useful across different detection mechanisms. """ -import collections import glob import itertools import os import os.path import re import sys +from typing import Dict, List, NamedTuple, Optional, Set, Tuple, Union import llnl.util.tty @@ -29,12 +29,18 @@ import spack.spec import spack.util.spack_yaml import spack.util.windows_registry -#: Information on a package that has been detected -DetectedPackage = collections.namedtuple("DetectedPackage", ["spec", "prefix"]) +class DetectedPackage(NamedTuple): + """Information on a package that has been detected.""" -def _externals_in_packages_yaml(): - """Return all the specs mentioned as externals in packages.yaml""" + #: Spec that was detected + spec: spack.spec.Spec + #: Prefix of the spec + prefix: str + + +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") already_defined_specs = set() for pkg_name, package_configuration in packages_yaml.items(): @@ -43,7 +49,12 @@ def _externals_in_packages_yaml(): return already_defined_specs -def _pkg_config_dict(external_pkg_entries): +ExternalEntryType = Union[str, Dict[str, str]] + + +def _pkg_config_dict( + external_pkg_entries: List[DetectedPackage], +) -> Dict[str, Union[bool, List[Dict[str, ExternalEntryType]]]]: """Generate a package specific config dict according to the packages.yaml schema. This does not generate the entire packages.yaml. For example, given some @@ -65,7 +76,10 @@ def _pkg_config_dict(external_pkg_entries): if not _spec_is_valid(e.spec): continue - external_items = [("spec", str(e.spec)), ("prefix", e.prefix)] + external_items: List[Tuple[str, ExternalEntryType]] = [ + ("spec", str(e.spec)), + ("prefix", e.prefix), + ] if e.spec.external_modules: external_items.append(("modules", e.spec.external_modules)) @@ -83,15 +97,14 @@ def _pkg_config_dict(external_pkg_entries): return pkg_dict -def _spec_is_valid(spec): +def _spec_is_valid(spec: spack.spec.Spec) -> bool: try: str(spec) except spack.error.SpackError: - # It is assumed here that we can at least extract the package name from - # the spec so we can look up the implementation of - # determine_spec_details - msg = "Constructed spec for {0} does not have a string representation" - llnl.util.tty.warn(msg.format(spec.name)) + # It is assumed here that we can at least extract the package name from the spec so we + # can look up the implementation of determine_spec_details + msg = f"Constructed spec for {spec.name} does not have a string representation" + llnl.util.tty.warn(msg) return False try: @@ -106,7 +119,7 @@ def _spec_is_valid(spec): return True -def path_to_dict(search_paths): +def path_to_dict(search_paths: List[str]): """Return dictionary[fullpath]: basename from list of paths""" path_to_lib = {} # Reverse order of search directories so that a lib in the first @@ -124,7 +137,7 @@ def path_to_dict(search_paths): return path_to_lib -def is_executable(file_path): +def is_executable(file_path: str) -> bool: """Return True if the path passed as argument is that of an executable""" return os.path.isfile(file_path) and os.access(file_path, os.X_OK) @@ -146,7 +159,7 @@ def _convert_to_iterable(single_val_or_multiple): return [x] -def executable_prefix(executable_dir): +def executable_prefix(executable_dir: str) -> str: """Given a directory where an executable is found, guess the prefix (i.e. the "root" directory of that installation) and return it. @@ -167,12 +180,12 @@ def executable_prefix(executable_dir): return os.sep.join(components[:idx]) -def library_prefix(library_dir): - """Given a directory where an library is found, guess the prefix +def library_prefix(library_dir: str) -> str: + """Given a directory where a library is found, guess the prefix (i.e. the "root" directory of that installation) and return it. Args: - library_dir: directory where an library is found + library_dir: directory where a library is found """ # Given a prefix where an library is found, assuming that prefix # contains /lib/ or /lib64/, strip off the 'lib' or 'lib64' directory @@ -195,13 +208,17 @@ def library_prefix(library_dir): return library_dir -def update_configuration(detected_packages, scope=None, buildable=True): +def update_configuration( + detected_packages: Dict[str, List[DetectedPackage]], + scope: Optional[str] = None, + buildable: bool = True, +) -> List[spack.spec.Spec]: """Add the packages passed as arguments to packages.yaml Args: - detected_packages (list): list of DetectedPackage objects to be added - scope (str): configuration scope where to add the detected packages - buildable (bool): whether the detected packages are buildable or not + detected_packages: list of DetectedPackage objects 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 = {}, [] @@ -209,7 +226,10 @@ def update_configuration(detected_packages, scope=None, buildable=True): new_entries = [e for e in entries if (e.spec not in predefined_external_specs)] pkg_config = _pkg_config_dict(new_entries) - all_new_specs.extend([spack.spec.Spec(x["spec"]) for x in pkg_config.get("externals", [])]) + external_entries = pkg_config.get("externals", []) + assert not isinstance(external_entries, bool), "unexpected value for external entry" + + all_new_specs.extend([spack.spec.Spec(x["spec"]) for x in external_entries]) if buildable is False: pkg_config["buildable"] = False pkg_to_cfg[package_name] = pkg_config @@ -222,16 +242,19 @@ def update_configuration(detected_packages, scope=None, buildable=True): return all_new_specs -def _windows_drive(): - """Return Windows drive string extracted from PROGRAMFILES - env var, which is garunteed to be defined for all logins""" - drive = re.match(r"([a-zA-Z]:)", os.environ["PROGRAMFILES"]).group(1) - return drive +def _windows_drive() -> str: + """Return Windows drive string extracted from the PROGRAMFILES environment variable, + which is guaranteed to be defined for all logins. + """ + match = re.match(r"([a-zA-Z]:)", os.environ["PROGRAMFILES"]) + if match is None: + raise RuntimeError("cannot read the PROGRAMFILES environment variable") + return match.group(1) class WindowsCompilerExternalPaths: @staticmethod - def find_windows_compiler_root_paths(): + def find_windows_compiler_root_paths() -> List[str]: """Helper for Windows compiler installation root discovery At the moment simply returns location of VS install paths from VSWhere @@ -239,7 +262,7 @@ class WindowsCompilerExternalPaths: return list(winOs.WindowsOs.vs_install_paths) @staticmethod - def find_windows_compiler_cmake_paths(): + def find_windows_compiler_cmake_paths() -> List[str]: """Semi hard-coded search path for cmake bundled with MSVC""" return [ os.path.join( @@ -249,7 +272,7 @@ class WindowsCompilerExternalPaths: ] @staticmethod - def find_windows_compiler_ninja_paths(): + def find_windows_compiler_ninja_paths() -> List[str]: """Semi hard-coded search heuristic for locating ninja bundled with MSVC""" return [ os.path.join(path, "Common7", "IDE", "CommonExtensions", "Microsoft", "CMake", "Ninja") @@ -257,7 +280,7 @@ class WindowsCompilerExternalPaths: ] @staticmethod - def find_windows_compiler_bundled_packages(): + def find_windows_compiler_bundled_packages() -> List[str]: """Return all MSVC compiler bundled packages""" return ( WindowsCompilerExternalPaths.find_windows_compiler_cmake_paths() @@ -266,14 +289,15 @@ class WindowsCompilerExternalPaths: class WindowsKitExternalPaths: + plat_major_ver = None if sys.platform == "win32": plat_major_ver = str(winOs.windows_version()[0]) @staticmethod - def find_windows_kit_roots(): + def find_windows_kit_roots() -> Optional[str]: """Return Windows kit root, typically %programfiles%\\Windows Kits\\10|11\\""" if sys.platform != "win32": - return [] + return None program_files = os.environ["PROGRAMFILES(x86)"] kit_base = os.path.join( program_files, "Windows Kits", WindowsKitExternalPaths.plat_major_ver @@ -281,21 +305,23 @@ class WindowsKitExternalPaths: return kit_base @staticmethod - def find_windows_kit_bin_paths(kit_base=None): + def find_windows_kit_bin_paths(kit_base: Optional[str] = None) -> List[str]: """Returns Windows kit bin directory per version""" kit_base = WindowsKitExternalPaths.find_windows_kit_roots() if not kit_base else kit_base + assert kit_base is not None, "unexpected value for kit_base" kit_bin = os.path.join(kit_base, "bin") return glob.glob(os.path.join(kit_bin, "[0-9]*", "*\\")) @staticmethod - def find_windows_kit_lib_paths(kit_base=None): + def find_windows_kit_lib_paths(kit_base: Optional[str] = None) -> List[str]: """Returns Windows kit lib directory per version""" kit_base = WindowsKitExternalPaths.find_windows_kit_roots() if not kit_base else kit_base + assert kit_base is not None, "unexpected value for kit_base" kit_lib = os.path.join(kit_base, "Lib") return glob.glob(os.path.join(kit_lib, "[0-9]*", "*", "*\\")) @staticmethod - def find_windows_driver_development_kit_paths(): + def find_windows_driver_development_kit_paths() -> List[str]: """Provides a list of all installation paths for the WDK by version and architecture """ @@ -303,7 +329,7 @@ class WindowsKitExternalPaths: return WindowsKitExternalPaths.find_windows_kit_lib_paths(wdk_content_root) @staticmethod - def find_windows_kit_reg_installed_roots_paths(): + def find_windows_kit_reg_installed_roots_paths() -> List[str]: reg = spack.util.windows_registry.WindowsRegistryView( "SOFTWARE\\Microsoft\\Windows Kits\\Installed Roots", root_key=spack.util.windows_registry.HKEY.HKEY_LOCAL_MACHINE, @@ -316,7 +342,7 @@ class WindowsKitExternalPaths: ) @staticmethod - def find_windows_kit_reg_sdk_paths(): + def find_windows_kit_reg_sdk_paths() -> List[str]: reg = spack.util.windows_registry.WindowsRegistryView( "SOFTWARE\\WOW6432Node\\Microsoft\\Microsoft SDKs\\Windows\\v%s.0" % WindowsKitExternalPaths.plat_major_ver, @@ -330,7 +356,7 @@ class WindowsKitExternalPaths: ) -def find_win32_additional_install_paths(): +def find_win32_additional_install_paths() -> List[str]: """Not all programs on Windows live on the PATH Return a list of other potential install locations. """ @@ -357,13 +383,12 @@ def find_win32_additional_install_paths(): return windows_search_ext -def compute_windows_program_path_for_package(pkg): - """Given a package, attempt to compute its Windows - program files location, return list of best guesses +def compute_windows_program_path_for_package(pkg: spack.package_base.PackageBase) -> List[str]: + """Given a package, attempts to compute its Windows program files location, + and returns the list of best guesses. Args: - pkg (spack.package_base.PackageBase): package for which - Program Files location is to be computed + pkg: package for which Program Files location is to be computed """ if sys.platform != "win32": return [] @@ -378,7 +403,7 @@ def compute_windows_program_path_for_package(pkg): ] -def compute_windows_user_path_for_package(pkg): +def compute_windows_user_path_for_package(pkg: spack.package_base.PackageBase) -> List[str]: """Given a package attempt to compute its user scoped install location, return list of potential locations based on common heuristics. For more info on Windows user specific diff --git a/lib/spack/spack/detection/path.py b/lib/spack/spack/detection/path.py index 04cf682f5c..ba6a0c153b 100644 --- a/lib/spack/spack/detection/path.py +++ b/lib/spack/spack/detection/path.py @@ -11,6 +11,7 @@ import os.path import re import sys import warnings +from typing import Dict, List, Optional, Set import llnl.util.filesystem import llnl.util.tty @@ -18,7 +19,7 @@ import llnl.util.tty import spack.util.environment import spack.util.ld_so_conf -from .common import ( # find_windows_compiler_bundled_packages, +from .common import ( DetectedPackage, WindowsCompilerExternalPaths, WindowsKitExternalPaths, @@ -32,7 +33,7 @@ from .common import ( # find_windows_compiler_bundled_packages, ) -def common_windows_package_paths(): +def common_windows_package_paths() -> List[str]: paths = WindowsCompilerExternalPaths.find_windows_compiler_bundled_packages() paths.extend(find_win32_additional_install_paths()) paths.extend(WindowsKitExternalPaths.find_windows_kit_bin_paths()) @@ -41,7 +42,7 @@ def common_windows_package_paths(): return paths -def executables_in_path(path_hints): +def executables_in_path(path_hints: List[str]) -> Dict[str, str]: """Get the paths of all executables available from the current PATH. For convenience, this is constructed as a dictionary where the keys are @@ -52,7 +53,7 @@ def executables_in_path(path_hints): assumed there are two different instances of the executable. Args: - path_hints (list): list of paths to be searched. If None the list will be + path_hints: list of paths to be searched. If None the list will be constructed based on the PATH environment variable. """ if sys.platform == "win32": @@ -61,7 +62,9 @@ def executables_in_path(path_hints): return path_to_dict(search_paths) -def libraries_in_ld_and_system_library_path(path_hints=None): +def libraries_in_ld_and_system_library_path( + path_hints: Optional[List[str]] = None, +) -> Dict[str, str]: """Get the paths of all libraries available from LD_LIBRARY_PATH, LIBRARY_PATH, DYLD_LIBRARY_PATH, DYLD_FALLBACK_LIBRARY_PATH, and standard system library paths. @@ -74,7 +77,7 @@ def libraries_in_ld_and_system_library_path(path_hints=None): assumed there are two different instances of the library. Args: - path_hints (list): list of paths to be searched. If None the list will be + path_hints: list of paths to be searched. If None the list will be constructed based on the set of LD_LIBRARY_PATH, LIBRARY_PATH, DYLD_LIBRARY_PATH, and DYLD_FALLBACK_LIBRARY_PATH environment variables as well as the standard system library paths. @@ -90,7 +93,7 @@ def libraries_in_ld_and_system_library_path(path_hints=None): return path_to_dict(search_paths) -def libraries_in_windows_paths(path_hints): +def libraries_in_windows_paths(path_hints: List[str]) -> Dict[str, str]: path_hints.extend(spack.util.environment.get_path("PATH")) search_paths = llnl.util.filesystem.search_paths_for_libraries(*path_hints) # on Windows, some libraries (.dlls) are found in the bin directory or sometimes @@ -106,17 +109,19 @@ def libraries_in_windows_paths(path_hints): return path_to_dict(search_paths) -def _group_by_prefix(paths): +def _group_by_prefix(paths: Set[str]) -> Dict[str, Set[str]]: groups = collections.defaultdict(set) for p in paths: groups[os.path.dirname(p)].add(p) - return groups.items() + return groups # TODO consolidate this with by_executable # Packages should be able to define both .libraries and .executables in the future # determine_spec_details should get all relevant libraries and executables in one call -def by_library(packages_to_check, path_hints=None): +def by_library( + packages_to_check: List[spack.package_base.PackageBase], path_hints: Optional[List[str]] = None +) -> Dict[str, List[DetectedPackage]]: # Techniques for finding libraries is determined on a per recipe basis in # the determine_version class method. Some packages will extract the # version number from a shared libraries filename. @@ -127,8 +132,8 @@ def by_library(packages_to_check, path_hints=None): DYLD_FALLBACK_LIBRARY_PATH, and standard system library paths. Args: - packages_to_check (list): list of packages to be detected - path_hints (list): list of paths to be searched. If None the list will be + packages_to_check: list of packages to be detected + path_hints: list of paths to be searched. If None the list will be constructed based on the LD_LIBRARY_PATH, LIBRARY_PATH, DYLD_LIBRARY_PATH, DYLD_FALLBACK_LIBRARY_PATH environment variables and standard system library paths. @@ -160,7 +165,7 @@ def by_library(packages_to_check, path_hints=None): pkg_to_found_libs[pkg].add(path) pkg_to_entries = collections.defaultdict(list) - resolved_specs = {} # spec -> lib found for the spec + resolved_specs: Dict[spack.spec.Spec, str] = {} # spec -> lib found for the spec for pkg, libs in pkg_to_found_libs.items(): if not hasattr(pkg, "determine_spec_details"): @@ -171,7 +176,7 @@ def by_library(packages_to_check, path_hints=None): ) continue - for prefix, libs_in_prefix in sorted(_group_by_prefix(libs)): + for prefix, libs_in_prefix in sorted(_group_by_prefix(libs).items()): try: specs = _convert_to_iterable(pkg.determine_spec_details(prefix, libs_in_prefix)) except Exception as e: @@ -225,19 +230,21 @@ def by_library(packages_to_check, path_hints=None): return pkg_to_entries -def by_executable(packages_to_check, path_hints=None): +def by_executable( + packages_to_check: List[spack.package_base.PackageBase], path_hints: Optional[List[str]] = None +) -> Dict[str, List[DetectedPackage]]: """Return the list of packages that have been detected on the system, searching by path. Args: - packages_to_check (list): list of package classes to be detected - path_hints (list): list of paths to be searched. If None the list will be + packages_to_check: list of package classes to be detected + path_hints: list of paths to be searched. If None the list will be constructed based on the PATH environment variable. """ path_hints = spack.util.environment.get_path("PATH") if path_hints is None else path_hints exe_pattern_to_pkgs = collections.defaultdict(list) for pkg in packages_to_check: - if hasattr(pkg, "executables"): + if hasattr(pkg, "executables") and hasattr(pkg, "platform_executables"): for exe in pkg.platform_executables(): exe_pattern_to_pkgs[exe].append(pkg) # Add Windows specific, package related paths to the search paths @@ -254,7 +261,7 @@ def by_executable(packages_to_check, path_hints=None): pkg_to_found_exes[pkg].add(path) pkg_to_entries = collections.defaultdict(list) - resolved_specs = {} # spec -> exe found for the spec + resolved_specs: Dict[spack.spec.Spec, str] = {} # spec -> exe found for the spec for pkg, exes in pkg_to_found_exes.items(): if not hasattr(pkg, "determine_spec_details"): @@ -265,7 +272,7 @@ def by_executable(packages_to_check, path_hints=None): ) continue - for prefix, exes_in_prefix in sorted(_group_by_prefix(exes)): + for prefix, exes_in_prefix in sorted(_group_by_prefix(exes).items()): # TODO: multiple instances of a package can live in the same # prefix, and a package implementation can return multiple specs # for one prefix, but without additional details (e.g. about the -- cgit v1.2.3-60-g2f50