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.

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]
+        installs = [s.package for s in concrete_specs]
-        builder = PackageInstaller(installs)
+        install_kwargs["explicit"] = [s.dag_hash() for s in concrete_specs]
+        builder = PackageInstaller(installs, install_kwargs)
         builder.install()

lib/spack/spack/environment/environment.py

@@ -1948,13 +1948,19 @@ def install_specs(self, specs: Optional[List[Spec]] = None, **install_args):
         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"] = (
+        overwrite: Set[str] = set()
-            install_args.get("overwrite", []) + self._dev_specs_that_need_overwrite()
+        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([spec.package for spec in specs], install_args).install()
-
-        PackageInstaller(installs).install()
 
     def all_specs_generator(self) -> Iterable[Spec]:
         """Returns a generator for all concrete specs"""

lib/spack/spack/installer.py

@@ -759,12 +759,8 @@ def __init__(self, pkg: "spack.package_base.PackageBase", install_args: dict):
         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,
+        self.pkg.stop_before_phase = install_args.get("stop_before")  # type: ignore[attr-defined] # noqa: E501
-        # popping the options to ensure installation of associated
+        self.pkg.last_phase = install_args.get("stop_at")  # type: ignore[attr-defined]
-        # 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]
 
         # Cache the package id for convenience
         self.pkg_id = package_id(pkg.spec)
@@ -1074,19 +1070,17 @@ def flag_installed(self, installed: List[str]) -> None:
 
     @property
     def explicit(self) -> bool:
-        """The package was explicitly requested by the user."""
+        return self.pkg.spec.dag_hash() in self.request.install_args.get("explicit", [])
-        return self.is_root and self.request.install_args.get("explicit", True)
 
     @property
-    def is_root(self) -> bool:
+    def is_build_request(self) -> bool:
-        """The package was requested directly, but may or may not be explicit
+        """The package was requested directly"""
-        in an environment."""
         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 @@ def use_cache(self) -> bool:
     @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 @@ def priority(self):
 
 class PackageInstaller:
     """
-    Class for managing the install process for a Spack instance based on a
+    Class for managing the install process for a Spack instance based on a bottom-up DAG approach.
-    bottom-up DAG approach.
 
-    This installer can coordinate concurrent batch and interactive, local
+    This installer can coordinate concurrent batch and interactive, local and distributed (on a
-    and distributed (on a shared file system) builds for the same Spack
+    shared file system) builds for the same Spack instance.
-    instance.
     """
 
-    def __init__(self, installs: List[Tuple["spack.package_base.PackageBase", dict]] = []) -> None:
+    def __init__(
-        """Initialize the installer.
+        self, packages: List["spack.package_base.PackageBase"], install_args: dict
-
+    ) -> None:
-        Args:
-            installs (list): list of tuples, where each
-                tuple consists of a package (PackageBase) and its associated
-                 install arguments (dict)
-        """
         # 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 @@ def _add_tasks(self, request: BuildRequest, all_deps):
         #
         # 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 @@ def _install_task(self, task: BuildTask, install_status: InstallStatus) -> None:
         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 @@ def install(self) -> None:
 
         self._init_queue()
         fail_fast_err = "Terminating after first install failure"
-        single_explicit_spec = len(self.build_requests) == 1
+        single_requested_spec = len(self.build_requests) == 1
-        failed_explicits = []
+        failed_build_requests = []
 
         install_status = InstallStatus(len(self.build_pq))
 
@@ -2195,14 +2178,11 @@ def install(self) -> None:
                 if self.fail_fast:
                     raise InstallError(f"{fail_fast_err}: {str(exc)}", pkg=pkg)
 
-                # Terminate at this point if the single explicit spec has
+                # Terminate when a single build request has failed, or summarize errors later.
-                # failed to install.
+                if task.is_build_request:
-                if single_explicit_spec and task.explicit:
+                    if single_requested_spec:
                         raise
-
+                    failed_build_requests.append((pkg, pkg_id, str(exc)))
-                # Track explicit spec id and error to summarize when done
-                if task.explicit:
-                    failed_explicits.append((pkg, pkg_id, str(exc)))
 
             finally:
                 # Remove the install prefix if anything went wrong during
@@ -2225,16 +2205,16 @@ def install(self) -> None:
             if request.install_args.get("install_package") and request.pkg_id not in self.installed
         ]
 
-        if failed_explicits or missing:
+        if failed_build_requests or missing:
-            for _, pkg_id, err in failed_explicits:
+            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:
+            if len(failed_build_requests) > 0:
-                pkg = failed_explicits[0][0]
+                pkg = failed_build_requests[0][0]
-                ids = [pkg_id for _, pkg_id, _ in failed_explicits]
+                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 @@ def __init__(self, pkg: "spack.package_base.PackageBase", install_args: dict):
         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", {})

lib/spack/spack/package_base.py

@@ -1881,7 +1881,10 @@ def do_install(self, **kwargs):
             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

lib/spack/spack/test/buildtask.py

@@ -12,21 +12,21 @@
 
 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, [])
+    task = inst.BuildTask(spec.package, request, False, 0, 0, inst.STATUS_ADDED, set())
-    assert task.explicit  # package was "explicitly" requested
+    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__()

lib/spack/spack/test/installer.py

@@ -7,6 +7,7 @@
 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}
+   namespace: {namespace}
-""".format(
+"""
-            namespace
-        )
     )
 
 
@@ -73,53 +72,21 @@ def _true(*args, **kwargs):
     return True
 
 
-def create_build_task(pkg, install_args={}):
+def create_build_task(
-    """
+    pkg: spack.package_base.PackageBase, install_args: Optional[dict] = None
-    Create a built task for the given (concretized) package
+) -> inst.BuildTask:
-
+    request = inst.BuildRequest(pkg, {} if install_args is None else install_args)
-    Args:
+    return inst.BuildTask(pkg, request, False, 0, 0, inst.STATUS_ADDED, set())
-        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):
+def create_installer(
-    """
+    specs: Union[List[str], List[spack.spec.Spec]], install_args: Optional[dict] = None
-    Create an installer using the concretized spec for each arg
+) -> inst.PackageInstaller:
-
+    """Create an installer instance for a list of specs or package names that will be
-    Args:
+    concretized."""
-        installer_args (list): the list of (spec name, kwargs) tuples
+    _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:
+    return inst.PackageInstaller([spec.package for spec in _specs], _install_args)
-        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
 
 
 @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(["trivial-install-test-package"])
-    installer = create_installer(const_arg)
 
     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(["trivial-install-test-package"])
-    installer = create_installer(const_arg)
 
     istr = str(installer)
     assert "#tasks=0" in istr
@@ -296,8 +261,7 @@ def _mock_installed(self):
     builder.add_package("f")
 
     with spack.repo.use_repositories(builder.root):
-        const_arg = installer_args(["a"], {})
+        installer = create_installer(["a"])
-        installer = create_installer(const_arg)
 
         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(["trivial-install-test-package"])
-    installer = create_installer(const_arg)
     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(["trivial-install-test-package"])
-    installer = create_installer(const_arg)
     spec = installer.build_requests[0].pkg.spec
 
     monkeypatch.setattr(ulk.Lock, "acquire_read", _raise)
@@ -382,8 +344,7 @@ def _raise(lock, timeout=None):
 
 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(["trivial-install-test-package"], {})
-    installer = create_installer(const_arg)
     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([pkg_id], {})
-    installer = create_installer(const_arg)
     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 _pl(db, spec, timeout):
         return lock
 
     pkg_id = "a"
-    const_arg = installer_args([pkg_id], {})
+    installer = create_installer([pkg_id], {})
-    installer = create_installer(const_arg)
     spec = installer.build_requests[0].pkg.spec
 
     monkeypatch.setattr(spack.database.SpecLocker, "lock", _pl)
@@ -510,7 +469,7 @@ def _conc_spec(compiler):
 
 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(["a"], {})
-    installer = create_installer(const_arg)
     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(["a"], {})
-    installer = create_installer(const_arg)
     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(["a"], {})
-    installer = create_installer(const_arg)
     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(["a"], {})
-    installer = create_installer(const_arg)
     request = installer.build_requests[0]
 
     # Mock the known dependencies as installed upstream
@@ -747,8 +702,7 @@ def _pkgs(compiler, architecture, pkgs):
         spec = spack.spec.Spec("mpi").concretized()
         return [(spec.package, True)]
 
-    const_arg = installer_args(["trivial-install-test-package"], {})
+    installer = create_installer(["trivial-install-test-package"], {})
-    installer = create_installer(const_arg)
     request = installer.build_requests[0]
     all_deps = defaultdict(set)
 
@@ -763,8 +717,7 @@ def _pkgs(compiler, architecture, pkgs):
 
 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(["dependent-install"], {})
-    installer = create_installer(const_arg)
     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([spec_name], {})
-        installer = create_installer(const_arg)
 
         # 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(["trivial-install-test-package"], {})
-    installer = create_installer(const_arg)
     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(["a"], {})
-    installer = create_installer(const_arg)
     task = create_build_task(installer.build_requests[0].pkg)
     task.compiler = True
 
@@ -825,8 +775,7 @@ def _add(_compilers):
 
 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(["trivial-install-test-package"], {})
-    installer = create_installer(const_arg)
 
     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(["a"], {})
-    installer = create_installer(const_arg)
     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(["a"], {})
-    installer = create_installer(const_arg)
     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 _mktask(pkg):
     def _rmtask(installer, pkg_id):
         raise RuntimeError("Raise an exception to test except path")
 
-    const_arg = installer_args(["a"], {})
+    installer = create_installer(["a"], {})
-    installer = create_installer(const_arg)
     spec = installer.build_requests[0].pkg.spec
 
     # Cover task removal happy path
@@ -922,8 +868,7 @@ def _chgrp(path, group, follow_symlinks=True):
     monkeypatch.setattr(prefs, "get_package_group", _get_group)
     monkeypatch.setattr(fs, "chgrp", _chgrp)
 
-    const_arg = installer_args(["trivial-install-test-package"], {})
+    installer = create_installer(["trivial-install-test-package"], {})
-    installer = create_installer(const_arg)
     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(["trivial-install-test-package"], {})
-    installer = create_installer(const_arg)
 
     monkeypatch.setattr(lk.Lock, "release_write", _raise_except)
     pkg_id = "test"
@@ -966,8 +910,7 @@ def _raise_except(lock):
 
 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(["dependent-install"], {})
-    installer = create_installer(const_arg)
     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(["dependent-install"], {})
-    installer = create_installer(const_arg)
 
     # 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(["b"], {})
-    installer = create_installer(const_arg)
 
     # 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(["a"], {"fail_fast": False})
-    installer = create_installer(const_arg)
 
     # Make sure the package is identified as failed
     monkeypatch.setattr(spack.database.FailureTracker, "has_failed", _true)
@@ -1037,8 +977,7 @@ def _interrupt(installer, task, install_status, **kwargs):
         else:
             installer.installed.add(task.pkg.name)
 
-    const_arg = installer_args([spec_name], {})
+    installer = create_installer([spec_name], {})
-    installer = create_installer(const_arg)
 
     # Raise a KeyboardInterrupt error to trigger early termination
     monkeypatch.setattr(inst.PackageInstaller, "_install_task", _interrupt)
@@ -1064,8 +1003,7 @@ def _install(installer, task, install_status, **kwargs):
         else:
             installer.installed.add(task.pkg.name)
 
-    const_arg = installer_args([spec_name], {})
+    installer = create_installer([spec_name], {})
-    installer = create_installer(const_arg)
 
     # Raise a KeyboardInterrupt error to trigger early termination
     monkeypatch.setattr(inst.PackageInstaller, "_install_task", _install)
@@ -1091,8 +1029,7 @@ def _install(installer, task, install_status, **kwargs):
         else:
             installer.installed.add(task.pkg.name)
 
-    const_arg = installer_args([spec_name, "a"], {})
+    installer = create_installer([spec_name, "a"], {})
-    installer = create_installer(const_arg)
 
     # Raise a KeyboardInterrupt error to trigger early termination
     monkeypatch.setattr(inst.PackageInstaller, "_install_task", _install)
@@ -1106,25 +1043,21 @@ def _install(installer, task, install_status, **kwargs):
 
 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})
+    b, c = spack.spec.Spec("b").concretized(), spack.spec.Spec("c").concretized()
-    const_arg.extend(installer_args(["c"], {"fail_fast": True}))
+    b_id, c_id = inst.package_id(b), inst.package_id(c)
-    installer = create_installer(const_arg)
+
-    pkg_ids = [inst.package_id(spec) for spec, _ in const_arg]
+    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 c to be skipped.
-    # This will prevent b from installing, which will cause the build of a
-    # 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 b_id in installer.failed, "Expected b to be marked as failed"
-    assert pkg_ids[1] not in installer.failed, "Expected no attempt to install c"
+    assert c_id not in installer.failed, "Expected no attempt to install c"
-
+    assert f"{b_id} failed to install" in capsys.readouterr().err
-    out = capsys.readouterr()[1]
-    assert "{0} failed to install".format(pkg_ids[0]) in out
 
 
 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(["a"], {"fail_fast": True})
-    installer = create_installer(const_arg)
 
     # 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(["b"], {})
-    installer = create_installer(const_arg)
 
     # Ensure never acquire a lock
     monkeypatch.setattr(inst.PackageInstaller, "_ensure_locked", _not_locked)
@@ -1181,20 +1112,19 @@ def _requeued(installer, task, install_status):
 
 def test_install_lock_installed_requeue(install_mockery, monkeypatch, capfd):
     """Cover basic install handling for installed package."""
-    const_arg = installer_args(["b"], {})
+    b = spack.spec.Spec("b").concretized()
-    b, _ = const_arg[0]
-    installer = create_installer(const_arg)
     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 _requeued(installer, task, install_status):
 
     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 _requeued(installer, task, install_status):
     # Ensure don't continually requeue the task
     monkeypatch.setattr(inst.PackageInstaller, "_requeue_task", _requeued)
 
-    const_arg = installer_args(["b"], {})
+    installer = create_installer(["b"], {})
-    installer = create_installer(const_arg)
 
     with pytest.raises(inst.InstallError, match="request failed"):
         installer.install()
@@ -1253,25 +1181,19 @@ def _requeued(installer, task, install_status):
 
 def test_install_skip_patch(install_mockery, mock_fetch):
     """Test the path skip_patch install path."""
-    spec_name = "b"
+    installer = create_installer(["b"], {"fake": False, "skip_patch": True})
-    const_arg = installer_args([spec_name], {"fake": False, "skip_patch": True})
-    installer = create_installer(const_arg)
-
     installer.install()
-
+    assert inst.package_id(installer.build_requests[0].pkg.spec) in installer.installed
-    spec, install_args = const_arg[0]
-    assert inst.package_id(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([spec_name], {"fake": False})
-    installer = create_installer(const_arg)
     pkg = installer.build_requests[0].pkg
-    assert not create_build_task(pkg, {"explicit": False}).explicit
+    assert not create_build_task(pkg, {"explicit": []}).explicit
-    assert create_build_task(pkg, {"explicit": True}).explicit
+    assert create_build_task(pkg, {"explicit": [pkg.spec.dag_hash()]}).explicit
-    assert create_build_task(pkg).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(["b"])
-    installer = create_installer(const_arg)
     installer._init_queue()
     task = installer._pop_task()
 
@@ -1341,8 +1262,7 @@ def remove(self, spec):
             self.called = True
 
     # Get a build task. TODO: refactor this to avoid calling internal methods
-    const_arg = installer_args(["b"])
+    installer = create_installer(["b"])
-    installer = create_installer(const_arg)
     installer._init_queue()
     task = installer._pop_task()
 
@@ -1375,22 +1295,20 @@ def test_term_status_line():
     x.clear()
 
 
-@pytest.mark.parametrize(
+@pytest.mark.parametrize("explicit", [True, False])
-    "explicit_args,is_explicit",
+def test_single_external_implicit_install(install_mockery, explicit):
-    [({"explicit": False}, False), ({"explicit": True}, True), ({}, True)],
-)
-def test_single_external_implicit_install(install_mockery, explicit_args, is_explicit):
     pkg = "trivial-install-test-package"
     s = spack.spec.Spec(pkg).concretized()
     s.external_path = "/usr"
-    create_installer([(s, explicit_args)]).install()
+    args = {"explicit": [s.dag_hash()] if explicit else []}
-    assert spack.store.STORE.db.get_record(pkg).explicit == is_explicit
+    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