diff --git a/lib/spack/spack/bootstrap/_common.py b/lib/spack/spack/bootstrap/_common.py index e0120777415..6a30869f272 100644 --- a/lib/spack/spack/bootstrap/_common.py +++ b/lib/spack/spack/bootstrap/_common.py @@ -10,7 +10,9 @@ import sys import sysconfig import warnings -from typing import Dict, Optional, Sequence, Union +from typing import Optional, Sequence, Union + +from typing_extensions import TypedDict import archspec.cpu @@ -18,13 +20,17 @@ from llnl.util import tty import spack.platforms +import spack.spec import spack.store import spack.util.environment import spack.util.executable from .config import spec_for_current_python -QueryInfo = Dict[str, "spack.spec.Spec"] + +class QueryInfo(TypedDict, total=False): + spec: spack.spec.Spec + command: spack.util.executable.Executable def _python_import(module: str) -> bool: @@ -211,7 +217,9 @@ def _executables_in_store( ): spack.util.environment.path_put_first("PATH", [bin_dir]) if query_info is not None: - query_info["command"] = spack.util.executable.which(*executables, path=bin_dir) + query_info["command"] = spack.util.executable.which( + *executables, path=bin_dir, required=True + ) query_info["spec"] = concrete_spec return True return False diff --git a/lib/spack/spack/bootstrap/core.py b/lib/spack/spack/bootstrap/core.py index 5b144e4c824..725cc671a09 100644 --- a/lib/spack/spack/bootstrap/core.py +++ b/lib/spack/spack/bootstrap/core.py @@ -48,7 +48,13 @@ import spack.version from spack.installer import PackageInstaller -from ._common import _executables_in_store, _python_import, _root_spec, _try_import_from_store +from ._common import ( + QueryInfo, + _executables_in_store, + _python_import, + _root_spec, + _try_import_from_store, +) from .clingo import ClingoBootstrapConcretizer from .config import spack_python_interpreter, spec_for_current_python @@ -135,7 +141,7 @@ class BuildcacheBootstrapper(Bootstrapper): def __init__(self, conf) -> None: super().__init__(conf) - self.last_search: Optional[ConfigDictionary] = None + self.last_search: Optional[QueryInfo] = None self.config_scope_name = f"bootstrap_buildcache-{uuid.uuid4()}" @staticmethod @@ -212,14 +218,14 @@ def _install_and_test( for _, pkg_hash, pkg_sha256 in item["binaries"]: self._install_by_hash(pkg_hash, pkg_sha256, bincache_platform) - info: ConfigDictionary = {} + info: QueryInfo = {} if test_fn(query_spec=abstract_spec, query_info=info): self.last_search = info return True return False def try_import(self, module: str, abstract_spec_str: str) -> bool: - info: ConfigDictionary + info: QueryInfo test_fn, info = functools.partial(_try_import_from_store, module), {} if test_fn(query_spec=abstract_spec_str, query_info=info): return True @@ -232,7 +238,7 @@ def try_import(self, module: str, abstract_spec_str: str) -> bool: return self._install_and_test(abstract_spec, bincache_platform, data, test_fn) def try_search_path(self, executables: Tuple[str], abstract_spec_str: str) -> bool: - info: ConfigDictionary + info: QueryInfo test_fn, info = functools.partial(_executables_in_store, executables), {} if test_fn(query_spec=abstract_spec_str, query_info=info): self.last_search = info @@ -250,11 +256,11 @@ class SourceBootstrapper(Bootstrapper): def __init__(self, conf) -> None: super().__init__(conf) - self.last_search: Optional[ConfigDictionary] = None + self.last_search: Optional[QueryInfo] = None self.config_scope_name = f"bootstrap_source-{uuid.uuid4()}" def try_import(self, module: str, abstract_spec_str: str) -> bool: - info: ConfigDictionary = {} + info: QueryInfo = {} if _try_import_from_store(module, abstract_spec_str, query_info=info): self.last_search = info return True @@ -289,7 +295,7 @@ def try_import(self, module: str, abstract_spec_str: str) -> bool: return False def try_search_path(self, executables: Tuple[str], abstract_spec_str: str) -> bool: - info: ConfigDictionary = {} + info: QueryInfo = {} if _executables_in_store(executables, abstract_spec_str, query_info=info): self.last_search = info return True @@ -415,6 +421,7 @@ def ensure_executables_in_path_or_raise( current_bootstrapper.last_search["spec"], current_bootstrapper.last_search["command"], ) + assert cmd is not None, "expected an Executable" cmd.add_default_envmod( spack.user_environment.environment_modifications_for_specs( concrete_spec, set_package_py_globals=False diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 4fc0b734772..5e136eb23dc 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -275,7 +275,7 @@ def _do_fake_install(pkg: "spack.package_base.PackageBase") -> None: fs.mkdirp(pkg.prefix.bin) fs.touch(os.path.join(pkg.prefix.bin, command)) if sys.platform != "win32": - chmod = which("chmod") + chmod = which("chmod", required=True) chmod("+x", os.path.join(pkg.prefix.bin, command)) # Install fake header file diff --git a/lib/spack/spack/test/util/executable.py b/lib/spack/spack/test/util/executable.py index 510b7261c6c..641ef993753 100644 --- a/lib/spack/spack/test/util/executable.py +++ b/lib/spack/spack/test/util/executable.py @@ -110,3 +110,11 @@ def test_which(tmpdir, monkeypatch): exe = ex.which("spack-test-exe") assert exe is not None assert exe.path == path + + +def test_construct_from_pathlib(mock_executable): + """Tests that we can construct an executable from a pathlib.Path object""" + expected = "Hello world!" + path = mock_executable("hello", output=f"echo {expected}\n") + hello = ex.Executable(path) + assert expected in hello(output=str) diff --git a/lib/spack/spack/util/executable.py b/lib/spack/spack/util/executable.py index 9502b0e63f0..e61a1af7ec5 100644 --- a/lib/spack/spack/util/executable.py +++ b/lib/spack/spack/util/executable.py @@ -7,7 +7,9 @@ import subprocess import sys from pathlib import Path, PurePath -from typing import Callable, Dict, Optional, Sequence, TextIO, Type, Union, overload +from typing import Callable, Dict, List, Optional, Sequence, TextIO, Type, Union, overload + +from typing_extensions import Literal import llnl.util.tty as tty @@ -20,9 +22,9 @@ class Executable: """Class representing a program that can be run on the command line.""" - def __init__(self, name: str) -> None: + def __init__(self, name: Union[str, Path]) -> None: file_path = str(Path(name)) - if sys.platform != "win32" and name.startswith("."): + if sys.platform != "win32" and isinstance(name, str) and name.startswith("."): # pathlib strips the ./ from relative paths so it must be added back file_path = os.path.join(".", file_path) @@ -338,10 +340,24 @@ def __str__(self): return " ".join(self.exe) -def which_string(*args, **kwargs): - """Like ``which()``, but return a string instead of an ``Executable``.""" - path = kwargs.get("path", os.environ.get("PATH", "")) - required = kwargs.get("required", False) +@overload +def which_string( + *args: str, path: Optional[Union[List[str], str]] = ..., required: Literal[True] +) -> str: ... + + +@overload +def which_string( + *args: str, path: Optional[Union[List[str], str]] = ..., required: bool = ... +) -> Optional[str]: ... + + +def which_string( + *args: str, path: Optional[Union[List[str], str]] = None, required: bool = False +) -> Optional[str]: + """Like ``which()``, but returns a string instead of an ``Executable``.""" + if path is None: + path = os.environ.get("PATH", "") if isinstance(path, list): paths = [Path(str(x)) for x in path] @@ -372,7 +388,6 @@ def add_extra_search_paths(paths): search_paths.insert(0, Path.cwd()) search_paths = add_extra_search_paths(search_paths) - search_item = Path(search_item) candidate_items = get_candidate_items(Path(search_item)) for candidate_item in candidate_items: @@ -385,29 +400,41 @@ def add_extra_search_paths(paths): pass if required: - raise CommandNotFoundError("spack requires '%s'. Make sure it is in your path." % args[0]) + raise CommandNotFoundError(f"spack requires '{args[0]}'. Make sure it is in your path.") return None -def which(*args, **kwargs): +@overload +def which( + *args: str, path: Optional[Union[List[str], str]] = ..., required: Literal[True] +) -> Executable: ... + + +@overload +def which( + *args: str, path: Optional[Union[List[str], str]] = ..., required: bool = ... +) -> Optional[Executable]: ... + + +def which( + *args: str, path: Optional[Union[List[str], str]] = None, required: bool = False +) -> Optional[Executable]: """Finds an executable in the path like command-line which. If given multiple executables, returns the first one that is found. If no executables are found, returns None. Parameters: - *args (str): One or more executables to search for - - Keyword Arguments: - path (list or str): The path to search. Defaults to ``PATH`` - required (bool): If set to True, raise an error if executable not found + *args: one or more executables to search for + path: the path to search. Defaults to ``PATH`` + required: if set to True, raise an error if executable not found Returns: Executable: The first executable that is found in the path """ - exe = which_string(*args, **kwargs) - return Executable(exe) if exe else None + exe = which_string(*args, path=path, required=required) + return Executable(exe) if exe is not None else None class ProcessError(spack.error.SpackError):