diff options
author | Harmen Stoppels <me@harmenstoppels.nl> | 2024-05-29 08:25:34 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-29 08:25:34 +0200 |
commit | 70412612c79af495fb2b2223edac4bd5a70a813a (patch) | |
tree | ada06069c44fdf26dbdf839f5c525f919abf66f3 /lib | |
parent | cd741c368ca3f0df4297d46092107a49e7698828 (diff) | |
download | spack-70412612c79af495fb2b2223edac4bd5a70a813a.tar.gz spack-70412612c79af495fb2b2223edac4bd5a70a813a.tar.bz2 spack-70412612c79af495fb2b2223edac4bd5a70a813a.tar.xz spack-70412612c79af495fb2b2223edac4bd5a70a813a.zip |
installer: improve init signature and explicits (#44374)
Change the installer to take `([pkg], args)` in the constructor instead
of `[(pkg, args)]`. The reason is that certain arguments are global
settings, and the new API ensures that those arguments cannot be
different across different "build requests".
The `explicit` install arg is now a list of hashes, and the installer is
no longer responsible for determining what package is installed
explicitly. This way environment installs can simply pass the list of
environment roots, without them necessarily being explicit build
requests. For example an env with two roots [a, b], where b depends on
a, would not always cause spack install to mark b as explicit.
Notice that `overwrite` already took a list of hashes, this makes
`explicit` consistent.
`package.do_install(explicit=True)` continues to take a boolean.
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/spack/cmd/install.py | 6 | ||||
-rw-r--r-- | lib/spack/spack/environment/environment.py | 16 | ||||
-rw-r--r-- | lib/spack/spack/installer.py | 76 | ||||
-rw-r--r-- | lib/spack/spack/package_base.py | 5 | ||||
-rw-r--r-- | lib/spack/spack/test/buildtask.py | 14 | ||||
-rw-r--r-- | lib/spack/spack/test/installer.py | 240 |
6 files changed, 132 insertions, 225 deletions
diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index 755fab7fa6..69de44df57 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -61,7 +61,6 @@ def install_kwargs_from_args(args): "dependencies_use_cache": cache_opt(args.use_cache, dep_use_bc), "dependencies_cache_only": cache_opt(args.cache_only, dep_use_bc), "include_build_deps": args.include_build_deps, - "explicit": True, # Use true as a default for install command "stop_at": args.until, "unsigned": args.unsigned, "install_deps": ("dependencies" in args.things_to_install), @@ -473,6 +472,7 @@ def install_without_active_env(args, install_kwargs, reporter_factory): require_user_confirmation_for_overwrite(concrete_specs, args) install_kwargs["overwrite"] = [spec.dag_hash() for spec in concrete_specs] - installs = [(s.package, install_kwargs) for s in concrete_specs] - builder = PackageInstaller(installs) + installs = [s.package for s in concrete_specs] + install_kwargs["explicit"] = [s.dag_hash() for s in concrete_specs] + builder = PackageInstaller(installs, install_kwargs) builder.install() diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index bc31f820b0..22a1a0d322 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -1948,13 +1948,19 @@ class Environment: specs = specs if specs is not None else roots # Extend the set of specs to overwrite with modified dev specs and their parents - install_args["overwrite"] = ( - install_args.get("overwrite", []) + self._dev_specs_that_need_overwrite() + overwrite: Set[str] = set() + overwrite.update(install_args.get("overwrite", []), self._dev_specs_that_need_overwrite()) + install_args["overwrite"] = overwrite + + explicit: Set[str] = set() + explicit.update( + install_args.get("explicit", []), + (s.dag_hash() for s in specs), + (s.dag_hash() for s in roots), ) + install_args["explicit"] = explicit - installs = [(spec.package, {**install_args, "explicit": spec in roots}) for spec in specs] - - PackageInstaller(installs).install() + PackageInstaller([spec.package for spec in specs], install_args).install() def all_specs_generator(self) -> Iterable[Spec]: """Returns a generator for all concrete specs""" diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index dcfa0c2269..c37fac186e 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -759,12 +759,8 @@ class BuildRequest: if not self.pkg.spec.concrete: raise ValueError(f"{self.pkg.name} must have a concrete spec") - # Cache the package phase options with the explicit package, - # popping the options to ensure installation of associated - # dependencies is NOT affected by these options. - - self.pkg.stop_before_phase = install_args.pop("stop_before", None) # type: ignore[attr-defined] # noqa: E501 - self.pkg.last_phase = install_args.pop("stop_at", None) # type: ignore[attr-defined] + self.pkg.stop_before_phase = install_args.get("stop_before") # type: ignore[attr-defined] # noqa: E501 + self.pkg.last_phase = install_args.get("stop_at") # type: ignore[attr-defined] # Cache the package id for convenience self.pkg_id = package_id(pkg.spec) @@ -1074,19 +1070,17 @@ class BuildTask: @property def explicit(self) -> bool: - """The package was explicitly requested by the user.""" - return self.is_root and self.request.install_args.get("explicit", True) + return self.pkg.spec.dag_hash() in self.request.install_args.get("explicit", []) @property - def is_root(self) -> bool: - """The package was requested directly, but may or may not be explicit - in an environment.""" + def is_build_request(self) -> bool: + """The package was requested directly""" return self.pkg == self.request.pkg @property def use_cache(self) -> bool: _use_cache = True - if self.is_root: + if self.is_build_request: return self.request.install_args.get("package_use_cache", _use_cache) else: return self.request.install_args.get("dependencies_use_cache", _use_cache) @@ -1094,7 +1088,7 @@ class BuildTask: @property def cache_only(self) -> bool: _cache_only = False - if self.is_root: + if self.is_build_request: return self.request.install_args.get("package_cache_only", _cache_only) else: return self.request.install_args.get("dependencies_cache_only", _cache_only) @@ -1120,24 +1114,17 @@ class BuildTask: class PackageInstaller: """ - Class for managing the install process for a Spack instance based on a - bottom-up DAG approach. + Class for managing the install process for a Spack instance based on a bottom-up DAG approach. - This installer can coordinate concurrent batch and interactive, local - and distributed (on a shared file system) builds for the same Spack - instance. + This installer can coordinate concurrent batch and interactive, local and distributed (on a + shared file system) builds for the same Spack instance. """ - def __init__(self, installs: List[Tuple["spack.package_base.PackageBase", dict]] = []) -> None: - """Initialize the installer. - - Args: - installs (list): list of tuples, where each - tuple consists of a package (PackageBase) and its associated - install arguments (dict) - """ + def __init__( + self, packages: List["spack.package_base.PackageBase"], install_args: dict + ) -> None: # List of build requests - self.build_requests = [BuildRequest(pkg, install_args) for pkg, install_args in installs] + self.build_requests = [BuildRequest(pkg, install_args) for pkg in packages] # Priority queue of build tasks self.build_pq: List[Tuple[Tuple[int, int], BuildTask]] = [] @@ -1560,7 +1547,7 @@ class PackageInstaller: # # External and upstream packages need to get flagged as installed to # ensure proper status tracking for environment build. - explicit = request.install_args.get("explicit", True) + explicit = request.pkg.spec.dag_hash() in request.install_args.get("explicit", []) not_local = _handle_external_and_upstream(request.pkg, explicit) if not_local: self._flag_installed(request.pkg) @@ -1681,10 +1668,6 @@ class PackageInstaller: if not pkg.unit_test_check(): return - # Injecting information to know if this installation request is the root one - # to determine in BuildProcessInstaller whether installation is explicit or not - install_args["is_root"] = task.is_root - try: self._setup_install_dir(pkg) @@ -1996,8 +1979,8 @@ class PackageInstaller: self._init_queue() fail_fast_err = "Terminating after first install failure" - single_explicit_spec = len(self.build_requests) == 1 - failed_explicits = [] + single_requested_spec = len(self.build_requests) == 1 + failed_build_requests = [] install_status = InstallStatus(len(self.build_pq)) @@ -2195,14 +2178,11 @@ class PackageInstaller: if self.fail_fast: raise InstallError(f"{fail_fast_err}: {str(exc)}", pkg=pkg) - # Terminate at this point if the single explicit spec has - # failed to install. - if single_explicit_spec and task.explicit: - raise - - # Track explicit spec id and error to summarize when done - if task.explicit: - failed_explicits.append((pkg, pkg_id, str(exc))) + # Terminate when a single build request has failed, or summarize errors later. + if task.is_build_request: + if single_requested_spec: + raise + failed_build_requests.append((pkg, pkg_id, str(exc))) finally: # Remove the install prefix if anything went wrong during @@ -2225,16 +2205,16 @@ class PackageInstaller: if request.install_args.get("install_package") and request.pkg_id not in self.installed ] - if failed_explicits or missing: - for _, pkg_id, err in failed_explicits: + if failed_build_requests or missing: + for _, pkg_id, err in failed_build_requests: tty.error(f"{pkg_id}: {err}") for _, pkg_id in missing: tty.error(f"{pkg_id}: Package was not installed") - if len(failed_explicits) > 0: - pkg = failed_explicits[0][0] - ids = [pkg_id for _, pkg_id, _ in failed_explicits] + if len(failed_build_requests) > 0: + pkg = failed_build_requests[0][0] + ids = [pkg_id for _, pkg_id, _ in failed_build_requests] tty.debug( "Associating installation failure with first failed " f"explicit package ({ids[0]}) from {', '.join(ids)}" @@ -2293,7 +2273,7 @@ class BuildProcessInstaller: self.verbose = bool(install_args.get("verbose", False)) # whether installation was explicitly requested by the user - self.explicit = install_args.get("is_root", False) and install_args.get("explicit", True) + self.explicit = pkg.spec.dag_hash() in install_args.get("explicit", []) # env before starting installation self.unmodified_env = install_args.get("unmodified_env", {}) diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py index 4726635750..a2b3e16f28 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -1881,7 +1881,10 @@ class PackageBase(WindowsRPath, PackageViewMixin, RedistributionMixin, metaclass verbose (bool): Display verbose build output (by default, suppresses it) """ - PackageInstaller([(self, kwargs)]).install() + explicit = kwargs.get("explicit", True) + if isinstance(explicit, bool): + kwargs["explicit"] = {self.spec.dag_hash()} if explicit else set() + PackageInstaller([self], kwargs).install() # TODO (post-34236): Update tests and all packages that use this as a # TODO (post-34236): package method to the routine made available to diff --git a/lib/spack/spack/test/buildtask.py b/lib/spack/spack/test/buildtask.py index a571e1dccd..569bfc56d8 100644 --- a/lib/spack/spack/test/buildtask.py +++ b/lib/spack/spack/test/buildtask.py @@ -12,21 +12,21 @@ import spack.spec def test_build_task_errors(install_mockery): with pytest.raises(ValueError, match="must be a package"): - inst.BuildTask("abc", None, False, 0, 0, 0, []) + inst.BuildTask("abc", None, False, 0, 0, 0, set()) spec = spack.spec.Spec("trivial-install-test-package") pkg_cls = spack.repo.PATH.get_pkg_class(spec.name) with pytest.raises(ValueError, match="must have a concrete spec"): - inst.BuildTask(pkg_cls(spec), None, False, 0, 0, 0, []) + inst.BuildTask(pkg_cls(spec), None, False, 0, 0, 0, set()) spec.concretize() assert spec.concrete with pytest.raises(ValueError, match="must have a build request"): - inst.BuildTask(spec.package, None, False, 0, 0, 0, []) + inst.BuildTask(spec.package, None, False, 0, 0, 0, set()) request = inst.BuildRequest(spec.package, {}) with pytest.raises(inst.InstallError, match="Cannot create a build task"): - inst.BuildTask(spec.package, request, False, 0, 0, inst.STATUS_REMOVED, []) + inst.BuildTask(spec.package, request, False, 0, 0, inst.STATUS_REMOVED, set()) def test_build_task_basics(install_mockery): @@ -36,8 +36,8 @@ def test_build_task_basics(install_mockery): # Ensure key properties match expectations request = inst.BuildRequest(spec.package, {}) - task = inst.BuildTask(spec.package, request, False, 0, 0, inst.STATUS_ADDED, []) - assert task.explicit # package was "explicitly" requested + task = inst.BuildTask(spec.package, request, False, 0, 0, inst.STATUS_ADDED, set()) + assert not task.explicit assert task.priority == len(task.uninstalled_deps) assert task.key == (task.priority, task.sequence) @@ -58,7 +58,7 @@ def test_build_task_strings(install_mockery): # Ensure key properties match expectations request = inst.BuildRequest(spec.package, {}) - task = inst.BuildTask(spec.package, request, False, 0, 0, inst.STATUS_ADDED, []) + task = inst.BuildTask(spec.package, request, False, 0, 0, inst.STATUS_ADDED, set()) # Cover __repr__ irep = task.__repr__() diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index 97b95a487f..53ce7314eb 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -7,6 +7,7 @@ import glob import os import shutil import sys +from typing import List, Optional, Union import py import pytest @@ -44,12 +45,10 @@ def _mock_repo(root, namespace): repodir.ensure(spack.repo.packages_dir_name, dir=True) yaml = repodir.join("repo.yaml") yaml.write( - """ + f""" repo: - namespace: {0} -""".format( - namespace - ) + namespace: {namespace} +""" ) @@ -73,53 +72,21 @@ def _true(*args, **kwargs): return True -def create_build_task(pkg, install_args={}): - """ - Create a built task for the given (concretized) package - - Args: - pkg (spack.package_base.PackageBase): concretized package associated with - the task - install_args (dict): dictionary of kwargs (or install args) - - Return: - (BuildTask) A basic package build task - """ - request = inst.BuildRequest(pkg, install_args) - return inst.BuildTask(pkg, request, False, 0, 0, inst.STATUS_ADDED, []) - - -def create_installer(installer_args): - """ - Create an installer using the concretized spec for each arg - - Args: - installer_args (list): the list of (spec name, kwargs) tuples +def create_build_task( + pkg: spack.package_base.PackageBase, install_args: Optional[dict] = None +) -> inst.BuildTask: + request = inst.BuildRequest(pkg, {} if install_args is None else install_args) + return inst.BuildTask(pkg, request, False, 0, 0, inst.STATUS_ADDED, set()) - Return: - spack.installer.PackageInstaller: the associated package installer - """ - const_arg = [(spec.package, kwargs) for spec, kwargs in installer_args] - return inst.PackageInstaller(const_arg) - -def installer_args(spec_names, kwargs={}): - """Return a the installer argument with each spec paired with kwargs - - Args: - spec_names (list): list of spec names - kwargs (dict or None): install arguments to apply to all of the specs - - Returns: - list: list of (spec, kwargs), the installer constructor argument - """ - arg = [] - for name in spec_names: - spec = spack.spec.Spec(name) - spec.concretize() - assert spec.concrete - arg.append((spec, kwargs)) - return arg +def create_installer( + specs: Union[List[str], List[spack.spec.Spec]], install_args: Optional[dict] = None +) -> inst.PackageInstaller: + """Create an installer instance for a list of specs or package names that will be + concretized.""" + _specs = [spack.spec.Spec(s).concretized() if isinstance(s, str) else s for s in specs] + _install_args = {} if install_args is None else install_args + return inst.PackageInstaller([spec.package for spec in _specs], _install_args) @pytest.mark.parametrize( @@ -240,8 +207,7 @@ def test_try_install_from_binary_cache(install_mockery, mock_packages, monkeypat def test_installer_repr(install_mockery): - const_arg = installer_args(["trivial-install-test-package"], {}) - installer = create_installer(const_arg) + installer = create_installer(["trivial-install-test-package"]) irep = installer.__repr__() assert irep.startswith(installer.__class__.__name__) @@ -250,8 +216,7 @@ def test_installer_repr(install_mockery): def test_installer_str(install_mockery): - const_arg = installer_args(["trivial-install-test-package"], {}) - installer = create_installer(const_arg) + installer = create_installer(["trivial-install-test-package"]) istr = str(installer) assert "#tasks=0" in istr @@ -296,8 +261,7 @@ def test_installer_prune_built_build_deps(install_mockery, monkeypatch, tmpdir): builder.add_package("f") with spack.repo.use_repositories(builder.root): - const_arg = installer_args(["a"], {}) - installer = create_installer(const_arg) + installer = create_installer(["a"]) installer._init_queue() @@ -331,8 +295,7 @@ def test_check_last_phase_error(install_mockery): def test_installer_ensure_ready_errors(install_mockery, monkeypatch): - const_arg = installer_args(["trivial-install-test-package"], {}) - installer = create_installer(const_arg) + installer = create_installer(["trivial-install-test-package"]) spec = installer.build_requests[0].pkg.spec fmt = r"cannot be installed locally.*{0}" @@ -366,8 +329,7 @@ def test_ensure_locked_err(install_mockery, monkeypatch, tmpdir, capsys): def _raise(lock, timeout=None): raise RuntimeError(mock_err_msg) - const_arg = installer_args(["trivial-install-test-package"], {}) - installer = create_installer(const_arg) + installer = create_installer(["trivial-install-test-package"]) spec = installer.build_requests[0].pkg.spec monkeypatch.setattr(ulk.Lock, "acquire_read", _raise) @@ -382,8 +344,7 @@ def test_ensure_locked_err(install_mockery, monkeypatch, tmpdir, capsys): def test_ensure_locked_have(install_mockery, tmpdir, capsys): """Test _ensure_locked when already have lock.""" - const_arg = installer_args(["trivial-install-test-package"], {}) - installer = create_installer(const_arg) + installer = create_installer(["trivial-install-test-package"], {}) spec = installer.build_requests[0].pkg.spec pkg_id = inst.package_id(spec) @@ -419,8 +380,7 @@ def test_ensure_locked_have(install_mockery, tmpdir, capsys): @pytest.mark.parametrize("lock_type,reads,writes", [("read", 1, 0), ("write", 0, 1)]) def test_ensure_locked_new_lock(install_mockery, tmpdir, lock_type, reads, writes): pkg_id = "a" - const_arg = installer_args([pkg_id], {}) - installer = create_installer(const_arg) + installer = create_installer([pkg_id], {}) spec = installer.build_requests[0].pkg.spec with tmpdir.as_cwd(): ltype, lock = installer._ensure_locked(lock_type, spec.package) @@ -439,8 +399,7 @@ def test_ensure_locked_new_warn(install_mockery, monkeypatch, tmpdir, capsys): return lock pkg_id = "a" - const_arg = installer_args([pkg_id], {}) - installer = create_installer(const_arg) + installer = create_installer([pkg_id], {}) spec = installer.build_requests[0].pkg.spec monkeypatch.setattr(spack.database.SpecLocker, "lock", _pl) @@ -510,7 +469,7 @@ def test_packages_needed_to_bootstrap_compiler_packages(install_mockery, monkeyp def test_update_tasks_for_compiler_packages_as_compiler(mock_packages, config, monkeypatch): spec = spack.spec.Spec("trivial-install-test-package").concretized() - installer = inst.PackageInstaller([(spec.package, {})]) + installer = inst.PackageInstaller([spec.package], {}) # Add a task to the queue installer._add_init_task(spec.package, installer.build_requests[0], False, {}) @@ -694,8 +653,7 @@ def test_check_deps_status_install_failure(install_mockery): for dep in s.traverse(root=False): spack.store.STORE.failure_tracker.mark(dep) - const_arg = installer_args(["a"], {}) - installer = create_installer(const_arg) + installer = create_installer(["a"], {}) request = installer.build_requests[0] with pytest.raises(inst.InstallError, match="install failure"): @@ -703,8 +661,7 @@ def test_check_deps_status_install_failure(install_mockery): def test_check_deps_status_write_locked(install_mockery, monkeypatch): - const_arg = installer_args(["a"], {}) - installer = create_installer(const_arg) + installer = create_installer(["a"], {}) request = installer.build_requests[0] # Ensure the lock is not acquired @@ -715,8 +672,7 @@ def test_check_deps_status_write_locked(install_mockery, monkeypatch): def test_check_deps_status_external(install_mockery, monkeypatch): - const_arg = installer_args(["a"], {}) - installer = create_installer(const_arg) + installer = create_installer(["a"], {}) request = installer.build_requests[0] # Mock the dependencies as external so assumed to be installed @@ -728,8 +684,7 @@ def test_check_deps_status_external(install_mockery, monkeypatch): def test_check_deps_status_upstream(install_mockery, monkeypatch): - const_arg = installer_args(["a"], {}) - installer = create_installer(const_arg) + installer = create_installer(["a"], {}) request = installer.build_requests[0] # Mock the known dependencies as installed upstream @@ -747,8 +702,7 @@ def test_add_bootstrap_compilers(install_mockery, monkeypatch): spec = spack.spec.Spec("mpi").concretized() return [(spec.package, True)] - const_arg = installer_args(["trivial-install-test-package"], {}) - installer = create_installer(const_arg) + installer = create_installer(["trivial-install-test-package"], {}) request = installer.build_requests[0] all_deps = defaultdict(set) @@ -763,8 +717,7 @@ def test_add_bootstrap_compilers(install_mockery, monkeypatch): def test_prepare_for_install_on_installed(install_mockery, monkeypatch): """Test of _prepare_for_install's early return for installed task path.""" - const_arg = installer_args(["dependent-install"], {}) - installer = create_installer(const_arg) + installer = create_installer(["dependent-install"], {}) request = installer.build_requests[0] install_args = {"keep_prefix": True, "keep_stage": True, "restage": False} @@ -779,8 +732,7 @@ def test_installer_init_requests(install_mockery): """Test of installer initial requests.""" spec_name = "dependent-install" with spack.config.override("config:install_missing_compilers", True): - const_arg = installer_args([spec_name], {}) - installer = create_installer(const_arg) + installer = create_installer([spec_name], {}) # There is only one explicit request in this case assert len(installer.build_requests) == 1 @@ -789,8 +741,7 @@ def test_installer_init_requests(install_mockery): def test_install_task_use_cache(install_mockery, monkeypatch): - const_arg = installer_args(["trivial-install-test-package"], {}) - installer = create_installer(const_arg) + installer = create_installer(["trivial-install-test-package"], {}) request = installer.build_requests[0] task = create_build_task(request.pkg) @@ -805,8 +756,7 @@ def test_install_task_add_compiler(install_mockery, monkeypatch, capfd): def _add(_compilers): tty.msg(config_msg) - const_arg = installer_args(["a"], {}) - installer = create_installer(const_arg) + installer = create_installer(["a"], {}) task = create_build_task(installer.build_requests[0].pkg) task.compiler = True @@ -825,8 +775,7 @@ def test_install_task_add_compiler(install_mockery, monkeypatch, capfd): def test_release_lock_write_n_exception(install_mockery, tmpdir, capsys): """Test _release_lock for supposed write lock with exception.""" - const_arg = installer_args(["trivial-install-test-package"], {}) - installer = create_installer(const_arg) + installer = create_installer(["trivial-install-test-package"], {}) pkg_id = "test" with tmpdir.as_cwd(): @@ -843,8 +792,7 @@ def test_release_lock_write_n_exception(install_mockery, tmpdir, capsys): @pytest.mark.parametrize("installed", [True, False]) def test_push_task_skip_processed(install_mockery, installed): """Test to ensure skip re-queueing a processed package.""" - const_arg = installer_args(["a"], {}) - installer = create_installer(const_arg) + installer = create_installer(["a"], {}) assert len(list(installer.build_tasks)) == 0 # Mark the package as installed OR failed @@ -861,8 +809,7 @@ def test_push_task_skip_processed(install_mockery, installed): def test_requeue_task(install_mockery, capfd): """Test to ensure cover _requeue_task.""" - const_arg = installer_args(["a"], {}) - installer = create_installer(const_arg) + installer = create_installer(["a"], {}) task = create_build_task(installer.build_requests[0].pkg) # temporarily set tty debug messages on so we can test output @@ -892,8 +839,7 @@ def test_cleanup_all_tasks(install_mockery, monkeypatch): def _rmtask(installer, pkg_id): raise RuntimeError("Raise an exception to test except path") - const_arg = installer_args(["a"], {}) - installer = create_installer(const_arg) + installer = create_installer(["a"], {}) spec = installer.build_requests[0].pkg.spec # Cover task removal happy path @@ -922,8 +868,7 @@ def test_setup_install_dir_grp(install_mockery, monkeypatch, capfd): monkeypatch.setattr(prefs, "get_package_group", _get_group) monkeypatch.setattr(fs, "chgrp", _chgrp) - const_arg = installer_args(["trivial-install-test-package"], {}) - installer = create_installer(const_arg) + installer = create_installer(["trivial-install-test-package"], {}) spec = installer.build_requests[0].pkg.spec fs.touchp(spec.prefix) @@ -949,8 +894,7 @@ def test_cleanup_failed_err(install_mockery, tmpdir, monkeypatch, capsys): def _raise_except(lock): raise RuntimeError(msg) - const_arg = installer_args(["trivial-install-test-package"], {}) - installer = create_installer(const_arg) + installer = create_installer(["trivial-install-test-package"], {}) monkeypatch.setattr(lk.Lock, "release_write", _raise_except) pkg_id = "test" @@ -966,8 +910,7 @@ def test_cleanup_failed_err(install_mockery, tmpdir, monkeypatch, capsys): def test_update_failed_no_dependent_task(install_mockery): """Test _update_failed with missing dependent build tasks.""" - const_arg = installer_args(["dependent-install"], {}) - installer = create_installer(const_arg) + installer = create_installer(["dependent-install"], {}) spec = installer.build_requests[0].pkg.spec for dep in spec.traverse(root=False): @@ -978,8 +921,7 @@ def test_update_failed_no_dependent_task(install_mockery): def test_install_uninstalled_deps(install_mockery, monkeypatch, capsys): """Test install with uninstalled dependencies.""" - const_arg = installer_args(["dependent-install"], {}) - installer = create_installer(const_arg) + installer = create_installer(["dependent-install"], {}) # Skip the actual installation and any status updates monkeypatch.setattr(inst.PackageInstaller, "_install_task", _noop) @@ -996,8 +938,7 @@ def test_install_uninstalled_deps(install_mockery, monkeypatch, capsys): def test_install_failed(install_mockery, monkeypatch, capsys): """Test install with failed install.""" - const_arg = installer_args(["b"], {}) - installer = create_installer(const_arg) + installer = create_installer(["b"], {}) # Make sure the package is identified as failed monkeypatch.setattr(spack.database.FailureTracker, "has_failed", _true) @@ -1012,8 +953,7 @@ def test_install_failed(install_mockery, monkeypatch, capsys): def test_install_failed_not_fast(install_mockery, monkeypatch, capsys): """Test install with failed install.""" - const_arg = installer_args(["a"], {"fail_fast": False}) - installer = create_installer(const_arg) + installer = create_installer(["a"], {"fail_fast": False}) # Make sure the package is identified as failed monkeypatch.setattr(spack.database.FailureTracker, "has_failed", _true) @@ -1037,8 +977,7 @@ def test_install_fail_on_interrupt(install_mockery, monkeypatch): else: installer.installed.add(task.pkg.name) - const_arg = installer_args([spec_name], {}) - installer = create_installer(const_arg) + installer = create_installer([spec_name], {}) # Raise a KeyboardInterrupt error to trigger early termination monkeypatch.setattr(inst.PackageInstaller, "_install_task", _interrupt) @@ -1064,8 +1003,7 @@ def test_install_fail_single(install_mockery, monkeypatch): else: installer.installed.add(task.pkg.name) - const_arg = installer_args([spec_name], {}) - installer = create_installer(const_arg) + installer = create_installer([spec_name], {}) # Raise a KeyboardInterrupt error to trigger early termination monkeypatch.setattr(inst.PackageInstaller, "_install_task", _install) @@ -1091,8 +1029,7 @@ def test_install_fail_multi(install_mockery, monkeypatch): else: installer.installed.add(task.pkg.name) - const_arg = installer_args([spec_name, "a"], {}) - installer = create_installer(const_arg) + installer = create_installer([spec_name, "a"], {}) # Raise a KeyboardInterrupt error to trigger early termination monkeypatch.setattr(inst.PackageInstaller, "_install_task", _install) @@ -1106,25 +1043,21 @@ def test_install_fail_multi(install_mockery, monkeypatch): def test_install_fail_fast_on_detect(install_mockery, monkeypatch, capsys): """Test fail_fast install when an install failure is detected.""" - const_arg = installer_args(["b"], {"fail_fast": False}) - const_arg.extend(installer_args(["c"], {"fail_fast": True})) - installer = create_installer(const_arg) - pkg_ids = [inst.package_id(spec) for spec, _ in const_arg] + b, c = spack.spec.Spec("b").concretized(), spack.spec.Spec("c").concretized() + b_id, c_id = inst.package_id(b), inst.package_id(c) + + installer = create_installer([b, c], {"fail_fast": True}) # Make sure all packages are identified as failed - # - # This will prevent b from installing, which will cause the build of a - # to be skipped. + # This will prevent b from installing, which will cause the build of c to be skipped. monkeypatch.setattr(spack.database.FailureTracker, "has_failed", _true) with pytest.raises(inst.InstallError, match="after first install failure"): installer.install() - assert pkg_ids[0] in installer.failed, "Expected b to be marked as failed" - assert pkg_ids[1] not in installer.failed, "Expected no attempt to install c" - - out = capsys.readouterr()[1] - assert "{0} failed to install".format(pkg_ids[0]) in out + assert b_id in installer.failed, "Expected b to be marked as failed" + assert c_id not in installer.failed, "Expected no attempt to install c" + assert f"{b_id} failed to install" in capsys.readouterr().err def _test_install_fail_fast_on_except_patch(installer, **kwargs): @@ -1137,8 +1070,7 @@ def _test_install_fail_fast_on_except_patch(installer, **kwargs): @pytest.mark.disable_clean_stage_check def test_install_fail_fast_on_except(install_mockery, monkeypatch, capsys): """Test fail_fast install when an install failure results from an error.""" - const_arg = installer_args(["a"], {"fail_fast": True}) - installer = create_installer(const_arg) + installer = create_installer(["a"], {"fail_fast": True}) # Raise a non-KeyboardInterrupt exception to trigger fast failure. # @@ -1161,8 +1093,7 @@ def test_install_lock_failures(install_mockery, monkeypatch, capfd): def _requeued(installer, task, install_status): tty.msg("requeued {0}".format(task.pkg.spec.name)) - const_arg = installer_args(["b"], {}) - installer = create_installer(const_arg) + installer = create_installer(["b"], {}) # Ensure never acquire a lock monkeypatch.setattr(inst.PackageInstaller, "_ensure_locked", _not_locked) @@ -1181,20 +1112,19 @@ def test_install_lock_failures(install_mockery, monkeypatch, capfd): def test_install_lock_installed_requeue(install_mockery, monkeypatch, capfd): """Cover basic install handling for installed package.""" - const_arg = installer_args(["b"], {}) - b, _ = const_arg[0] - installer = create_installer(const_arg) + b = spack.spec.Spec("b").concretized() b_pkg_id = inst.package_id(b) + installer = create_installer([b]) def _prep(installer, task): installer.installed.add(b_pkg_id) - tty.msg("{0} is installed".format(b_pkg_id)) + tty.msg(f"{b_pkg_id} is installed") # also do not allow the package to be locked again monkeypatch.setattr(inst.PackageInstaller, "_ensure_locked", _not_locked) def _requeued(installer, task, install_status): - tty.msg("requeued {0}".format(inst.package_id(task.pkg.spec))) + tty.msg(f"requeued {inst.package_id(task.pkg.spec)}") # Flag the package as installed monkeypatch.setattr(inst.PackageInstaller, "_prepare_for_install", _prep) @@ -1207,9 +1137,8 @@ def test_install_lock_installed_requeue(install_mockery, monkeypatch, capfd): assert b_pkg_id not in installer.installed - out = capfd.readouterr()[0] expected = ["is installed", "read locked", "requeued"] - for exp, ln in zip(expected, out.split("\n")): + for exp, ln in zip(expected, capfd.readouterr().out.splitlines()): assert exp in ln @@ -1237,8 +1166,7 @@ def test_install_read_locked_requeue(install_mockery, monkeypatch, capfd): # Ensure don't continually requeue the task monkeypatch.setattr(inst.PackageInstaller, "_requeue_task", _requeued) - const_arg = installer_args(["b"], {}) - installer = create_installer(const_arg) + installer = create_installer(["b"], {}) with pytest.raises(inst.InstallError, match="request failed"): installer.install() @@ -1253,25 +1181,19 @@ def test_install_read_locked_requeue(install_mockery, monkeypatch, capfd): def test_install_skip_patch(install_mockery, mock_fetch): """Test the path skip_patch install path.""" - spec_name = "b" - const_arg = installer_args([spec_name], {"fake": False, "skip_patch": True}) - installer = create_installer(const_arg) - + installer = create_installer(["b"], {"fake": False, "skip_patch": True}) installer.install() - - spec, install_args = const_arg[0] - assert inst.package_id(spec) in installer.installed + assert inst.package_id(installer.build_requests[0].pkg.spec) in installer.installed def test_install_implicit(install_mockery, mock_fetch): """Test the path skip_patch install path.""" spec_name = "trivial-install-test-package" - const_arg = installer_args([spec_name], {"fake": False}) - installer = create_installer(const_arg) + installer = create_installer([spec_name], {"fake": False}) pkg = installer.build_requests[0].pkg - assert not create_build_task(pkg, {"explicit": False}).explicit - assert create_build_task(pkg, {"explicit": True}).explicit - assert create_build_task(pkg).explicit + assert not create_build_task(pkg, {"explicit": []}).explicit + assert create_build_task(pkg, {"explicit": [pkg.spec.dag_hash()]}).explicit + assert not create_build_task(pkg).explicit def test_overwrite_install_backup_success(temporary_store, config, mock_packages, tmpdir): @@ -1280,8 +1202,7 @@ def test_overwrite_install_backup_success(temporary_store, config, mock_packages of the original prefix, and leave the original spec marked installed. """ # Get a build task. TODO: refactor this to avoid calling internal methods - const_arg = installer_args(["b"]) - installer = create_installer(const_arg) + installer = create_installer(["b"]) installer._init_queue() task = installer._pop_task() @@ -1341,8 +1262,7 @@ def test_overwrite_install_backup_failure(temporary_store, config, mock_packages self.called = True # Get a build task. TODO: refactor this to avoid calling internal methods - const_arg = installer_args(["b"]) - installer = create_installer(const_arg) + installer = create_installer(["b"]) installer._init_queue() task = installer._pop_task() @@ -1375,22 +1295,20 @@ def test_term_status_line(): x.clear() -@pytest.mark.parametrize( - "explicit_args,is_explicit", - [({"explicit": False}, False), ({"explicit": True}, True), ({}, True)], -) -def test_single_external_implicit_install(install_mockery, explicit_args, is_explicit): +@pytest.mark.parametrize("explicit", [True, False]) +def test_single_external_implicit_install(install_mockery, explicit): pkg = "trivial-install-test-package" s = spack.spec.Spec(pkg).concretized() s.external_path = "/usr" - create_installer([(s, explicit_args)]).install() - assert spack.store.STORE.db.get_record(pkg).explicit == is_explicit + args = {"explicit": [s.dag_hash()] if explicit else []} + create_installer([s], args).install() + assert spack.store.STORE.db.get_record(pkg).explicit == explicit def test_overwrite_install_does_install_build_deps(install_mockery, mock_fetch): """When overwrite installing something from sources, build deps should be installed.""" s = spack.spec.Spec("dtrun3").concretized() - create_installer([(s, {})]).install() + create_installer([s]).install() # Verify there is a pure build dep edge = s.edges_to_dependencies(name="dtbuild3").pop() @@ -1401,7 +1319,7 @@ def test_overwrite_install_does_install_build_deps(install_mockery, mock_fetch): build_dep.package.do_uninstall() # Overwrite install the root dtrun3 - create_installer([(s, {"overwrite": [s.dag_hash()]})]).install() + create_installer([s], {"overwrite": [s.dag_hash()]}).install() # Verify that the build dep was also installed. assert build_dep.installed |