Installer: update installation progress tracking

- test/installer: use existing inst for spack.installer
- remove install status from Installing message
- Add specs count visitor
- Report status on installed plus minor refactor
- Add the "+" to the tracker; include one experimental dynamic calculation
- tweak status reporting to include ensuring numerator unique across installed packages
- _print_installed_pkg -> InstallStatus.print_installed()
- move set_term_title outside of InstallStatus
- InstallStatus: remove unnecessary next_pkg
- InstallStatus: class and method name changes
  * changed InstallStatus to InstallerStatus since already have former in
    database.py and spec.py
  * changed print_installed to set_installed since does more than print now
- InstallerStatus -> InstallerProgress, install_status -> progress
- InstallerProgress: cache config:install_status
- InstallerProgress: restore get_progress and set_term_title methods (w/ tweaks)
- Task execute(): added returns to docstrings
- Don't pass progress to build_process or Installer.run, but set installed on successful return
- fix mypy issue with pkg.run_tests assignment

lib/spack/spack/installer.py

@@ -65,6 +65,7 @@
 import spack.util.executable
 import spack.util.path
 import spack.util.timer as timer
+from spack.traverse import CoverNodesVisitor, traverse_breadth_first_with_visitor
 from spack.util.environment import EnvironmentModifications, dump_environment
 from spack.util.executable import which
 
@@ -134,22 +135,46 @@ class InstallAction(enum.Enum):
     OVERWRITE = enum.auto()
 
 
-class InstallStatus:
+class InstallerProgress:
-    def __init__(self, pkg_count: int):
+    """Installation progress tracker"""
-        # Counters used for showing status information
+
-        self.pkg_num: int = 0
+    def __init__(self, packages: List["spack.package_base.PackageBase"]):
-        self.pkg_count: int = pkg_count
+        self.counter = SpecsCount(dt.BUILD | dt.LINK | dt.RUN)
+        self.pkg_count: int = self.counter.total([pkg.spec for pkg in packages])
         self.pkg_ids: Set[str] = set()
+        self.pkg_num: int = 0
+        self.add_progress: bool = spack.config.get("config:install_status", True)
 
-    def next_pkg(self, pkg: "spack.package_base.PackageBase"):
+    def set_installed(self, pkg: "spack.package_base.PackageBase", message: str) -> None:
+        """
+        Flag package as installed and output the installation status if
+        enabled by config:install_status.
+
+        Args:
+            pkg: installed package
+            message: message to be output
+        """
         pkg_id = package_id(pkg.spec)
-
         if pkg_id not in self.pkg_ids:
-            self.pkg_num += 1
             self.pkg_ids.add(pkg_id)
+            visited = max(len(self.pkg_ids), self.counter.total([pkg.spec]), self.pkg_num + 1)
+            self.pkg_num = visited
+
+        if tty.msg_enabled():
+            post = self.get_progress() if self.add_progress else ""
+            print(
+                colorize("@*g{[+]} ") + spack.util.path.debug_padded_filter(message) + f" {post}"
+            )
+
+        self.set_term_title("Installed")
 
     def set_term_title(self, text: str):
-        if not spack.config.get("config:install_status", True):
+        """Update the terminal title bar.
+
+        Args:
+            text: message to output in the terminal title bar
+        """
+        if not self.add_progress:
             return
 
         if not sys.stdout.isatty():
@@ -160,7 +185,11 @@ def set_term_title(self, text: str):
         sys.stdout.flush()
 
     def get_progress(self) -> str:
-        return f"[{self.pkg_num}/{self.pkg_count}]"
+        """Current installation progress
+
+        Returns:  string showing the current installation progress
+        """
+        return f"[{self.pkg_num}/{self.pkg_count} completed]"
 
 
 class TermStatusLine:
@@ -229,7 +258,9 @@ def _check_last_phase(pkg: "spack.package_base.PackageBase") -> None:
         pkg.last_phase = None  # type: ignore[attr-defined]
 
 
-def _handle_external_and_upstream(pkg: "spack.package_base.PackageBase", explicit: bool) -> bool:
+def _handle_external_and_upstream(
+    pkg: "spack.package_base.PackageBase", explicit: bool, progress: InstallerProgress
+) -> bool:
     """
     Determine if the package is external or upstream and register it in the
     database if it is external package.
@@ -237,6 +268,8 @@ def _handle_external_and_upstream(pkg: "spack.package_base.PackageBase", explici
     Args:
         pkg: the package whose installation is under consideration
         explicit: the package was explicitly requested by the user
+        progress: installation progress tracker
+
     Return:
         ``True`` if the package is not to be installed locally, otherwise ``False``
     """
@@ -244,7 +277,7 @@ def _handle_external_and_upstream(pkg: "spack.package_base.PackageBase", explici
     # consists in module file generation and registration in the DB.
     if pkg.spec.external:
         _process_external_package(pkg, explicit)
-        _print_installed_pkg(f"{pkg.prefix} (external {package_id(pkg.spec)})")
+        progress.set_installed(pkg, f"{pkg.prefix} (external {package_id(pkg.spec)})")
         return True
 
     if pkg.spec.installed_upstream:
@@ -252,7 +285,7 @@ def _handle_external_and_upstream(pkg: "spack.package_base.PackageBase", explici
             f"{package_id(pkg.spec)} is installed in an upstream Spack instance at "
             f"{pkg.spec.prefix}"
         )
-        _print_installed_pkg(pkg.prefix)
+        progress.set_installed(pkg, pkg.prefix)
 
         # This will result in skipping all post-install hooks. In the case
         # of modules this is considered correct because we want to retrieve
@@ -328,17 +361,6 @@ def _log_prefix(pkg_name) -> str:
     return f"{pid}{pkg_name}:"
 
 
-def _print_installed_pkg(message: str) -> None:
-    """
-    Output a message with a package icon.
-
-    Args:
-        message (str): message to be output
-    """
-    if tty.msg_enabled():
-        print(colorize("@*g{[+]} ") + spack.util.path.debug_padded_filter(message))
-
-
 def print_install_test_log(pkg: "spack.package_base.PackageBase") -> None:
     """Output install test log file path but only if have test failures.
 
@@ -359,13 +381,17 @@ def _print_timer(pre: str, pkg_id: str, timer: timer.BaseTimer) -> None:
 
 
 def _install_from_cache(
-    pkg: "spack.package_base.PackageBase", explicit: bool, unsigned: Optional[bool] = False
+    pkg: "spack.package_base.PackageBase",
+    progress: InstallerProgress,
+    explicit: bool,
+    unsigned: Optional[bool] = False,
 ) -> bool:
     """
     Install the package from binary cache
 
     Args:
         pkg: package to install from the binary cache
+        progress: installation status tracker
         explicit: ``True`` if installing the package was explicitly
             requested by the user, otherwise, ``False``
         unsigned: if ``True`` or ``False`` override the mirror signature verification defaults
@@ -385,7 +411,7 @@ def _install_from_cache(
 
     _write_timer_json(pkg, t, True)
     _print_timer(pre=_log_prefix(pkg.name), pkg_id=pkg_id, timer=t)
-    _print_installed_pkg(pkg.spec.prefix)
+    progress.set_installed(pkg, pkg.spec.prefix)
     spack.hooks.post_install(pkg.spec, explicit)
     return True
 
@@ -596,7 +622,7 @@ def get_dependent_ids(spec: "spack.spec.Spec") -> List[str]:
     return [package_id(d) for d in spec.dependents()]
 
 
-def install_msg(name: str, pid: int, install_status: InstallStatus) -> str:
+def install_msg(name: str, pid: int) -> str:
     """
     Colorize the name/id of the package being installed
 
@@ -607,12 +633,7 @@ def install_msg(name: str, pid: int, install_status: InstallStatus) -> str:
     Return: Colorized installing message
     """
     pre = f"{pid}: " if tty.show_pid() else ""
-    post = (
+    return pre + colorize("@*{Installing} @*g{%s}" % (name))
-        " @*{%s}" % install_status.get_progress()
-        if install_status and spack.config.get("config:install_status", True)
-        else ""
-    )
-    return pre + colorize("@*{Installing} @*g{%s}%s" % (name, post))
 
 
 def archive_install_logs(pkg: "spack.package_base.PackageBase", phase_log_dir: str) -> None:
@@ -721,6 +742,18 @@ def package_id(spec: "spack.spec.Spec") -> str:
     return f"{spec.name}-{spec.version}-{spec.dag_hash()}"
 
 
+class SpecsCount:
+    def __init__(self, depflag: int):
+        self.depflag = depflag
+
+    def total(self, specs: List["spack.spec.Spec"]):
+        visitor = CoverNodesVisitor(
+            spack.spec.DagCountVisitor(self.depflag), key=lambda s: package_id(s)
+        )
+        traverse_breadth_first_with_visitor(specs, visitor)
+        return visitor.visitor.number
+
+
 class BuildRequest:
     """Class for representing an installation request."""
 
@@ -962,11 +995,14 @@ def __init__(
         self.attempts = attempts
         self._update()
 
-    def execute(self, install_status: InstallStatus) -> ExecuteResult:
+    def execute(self, progress: InstallerProgress) -> ExecuteResult:
         """Execute the work of this task.
 
-        The ``install_status`` is an ``InstallStatus`` object used to format progress reporting for
+        Args:
-        this task in the context of the full ``BuildRequest``."""
+            progress:  installation progress tracker
+
+        Returns: execution result
+        """
         raise NotImplementedError
 
     def __eq__(self, other):
@@ -1131,21 +1167,26 @@ def priority(self):
 class BuildTask(Task):
     """Class for representing a build task for a package."""
 
-    def execute(self, install_status):
+    def execute(self, progress: InstallerProgress) -> ExecuteResult:
         """
         Perform the installation of the requested spec and/or dependency
         represented by the build task.
+
+        Args:
+            progress:  installation progress tracker
+
+        Returns: execution result
         """
         install_args = self.request.install_args
-        tests = install_args.get("tests")
+        tests = install_args.get("tests", False)
 
         pkg, pkg_id = self.pkg, self.pkg_id
 
-        tty.msg(install_msg(pkg_id, self.pid, install_status))
+        tty.msg(install_msg(pkg_id, self.pid))
         self.start = self.start or time.time()
         self.status = BuildStatus.INSTALLING
 
-        pkg.run_tests = tests is True or tests and pkg.name in tests
+        pkg.run_tests = tests is True or (tests and pkg.name in tests)
 
         # hook that allows tests to inspect the Package before installation
         # see unit_test_check() docs.
@@ -1168,6 +1209,8 @@ def execute(self, install_status):
             # Note: PARENT of the build process adds the new package to
             # the database, so that we don't need to re-read from file.
             spack.store.STORE.db.add(pkg.spec, explicit=self.explicit)
+
+            progress.set_installed(self.pkg, self.pkg.prefix)
         except spack.error.StopPhase as e:
             # A StopPhase exception means that do_install was asked to
             # stop early from clients, and is not an error at this point
@@ -1180,13 +1223,17 @@ def execute(self, install_status):
 class InstallTask(Task):
     """Class for representing a build task for a package."""
 
-    def execute(self, install_status):
+    def execute(self, progress: InstallerProgress) -> ExecuteResult:
         """
         Perform the installation of the requested spec and/or dependency
         represented by the build task.
+
+        Args:
+            progress:  installation progress tracker
+
+        Returns: execution result
         """
         # no-op and requeue to build if not allowed to use cache
-        # this
         if not self.use_cache:
             return ExecuteResult.MISSING_BINARY
 
@@ -1195,12 +1242,12 @@ def execute(self, install_status):
 
         pkg, pkg_id = self.pkg, self.pkg_id
 
-        tty.msg(install_msg(pkg_id, self.pid, install_status))
+        tty.msg(install_msg(pkg_id, self.pid))
         self.start = self.start or time.time()
         self.status = BuildStatus.INSTALLING
 
         try:
-            if _install_from_cache(pkg, self.explicit, unsigned):
+            if _install_from_cache(pkg, progress, self.explicit, unsigned):
                 return ExecuteResult.SUCCESS
             elif self.cache_only:
                 raise spack.error.InstallError(
@@ -1243,7 +1290,7 @@ def build_task(self, installed):
 class RewireTask(Task):
     """Class for representing a rewire task for a package."""
 
-    def execute(self, install_status):
+    def execute(self, progress: InstallerProgress) -> ExecuteResult:
         """Execute rewire task
 
         Rewire tasks are executed by either rewiring self.package.spec.build_spec that is already
@@ -1252,24 +1299,30 @@ def execute(self, install_status):
         If not available installed or as binary, return ExecuteResult.MISSING_BUILD_SPEC.
         This will prompt the Installer to requeue the task with a dependency on the BuildTask
         to install self.pkg.spec.build_spec
+
+        Args:
+            progress:  installation progress tracker
+
+        Returns: execution result
         """
         oldstatus = self.status
         self.status = BuildStatus.INSTALLING
-        tty.msg(install_msg(self.pkg_id, self.pid, install_status))
+        tty.msg(install_msg(self.pkg_id, self.pid))
         self.start = self.start or time.time()
         if not self.pkg.spec.build_spec.installed:
             try:
                 install_args = self.request.install_args
                 unsigned = install_args.get("unsigned")
                 _process_binary_cache_tarball(self.pkg, explicit=self.explicit, unsigned=unsigned)
-                _print_installed_pkg(self.pkg.prefix)
+                progress.set_installed(self.pkg, self.pkg.prefix)
                 return ExecuteResult.SUCCESS
             except BaseException as e:
                 tty.error(f"Failed to rewire {self.pkg.spec} from binary. {e}")
                 self.status = oldstatus
                 return ExecuteResult.MISSING_BUILD_SPEC
+
         spack.rewiring.rewire_node(self.pkg.spec, self.explicit)
-        _print_installed_pkg(self.pkg.prefix)
+        progress.set_installed(self.pkg, self.pkg.prefix)
         return ExecuteResult.SUCCESS
 
 
@@ -1369,6 +1422,9 @@ def __init__(
         # Priority queue of tasks
         self.build_pq: List[Tuple[Tuple[int, int], Task]] = []
 
+        # Installation status tracker
+        self.progress: InstallerProgress = InstallerProgress(packages)
+
         # Mapping of unique package ids to task
         self.build_tasks: Dict[str, Task] = {}
 
@@ -1836,39 +1892,37 @@ def _add_tasks(self, request: BuildRequest, all_deps):
         fail_fast = bool(request.install_args.get("fail_fast"))
         self.fail_fast = self.fail_fast or fail_fast
 
-    def _install_task(self, task: Task, install_status: InstallStatus) -> ExecuteResult:
+    def _install_task(self, task: Task) -> ExecuteResult:
         """
         Perform the installation of the requested spec and/or dependency
         represented by the task.
 
         Args:
             task: the installation task for a package
-            install_status: the installation status for the package"""
+        """
-        rc = task.execute(install_status)
+        rc = task.execute(self.progress)
         if rc == ExecuteResult.MISSING_BUILD_SPEC:
             self._requeue_with_build_spec_tasks(task)
         elif rc == ExecuteResult.MISSING_BINARY:
             self._requeue_as_build_task(task)
         else:  # if rc == ExecuteResult.SUCCESS or rc == ExecuteResult.FAILED
             self._update_installed(task)
-        return rc  # Used by reporters to skip requeued tasks
+        return rc
 
-    def _overwrite_install_task(self, task: Task, install_status: InstallStatus) -> ExecuteResult:
+    def _overwrite_install_task(self, task: Task):
         """
         Try to run the install task overwriting the package prefix.
         If this fails, try to recover the original install prefix. If that fails
-        too, mark the spec as uninstalled. This function always the original
+        too, mark the spec as uninstalled.
-        install error if installation fails.
         """
         try:
             with fs.replace_directory_transaction(task.pkg.prefix):
-                rc = self._install_task(task, install_status)
+                rc = self._install_task(task)
                 if rc in requeue_results:
                     raise Requeue  # raise to trigger transactional replacement of directory
-                return rc
 
         except Requeue:
-            return rc  # This task is requeueing, not failing
+            pass  # This task is requeueing, not failing
         except fs.CouldNotRestoreDirectoryBackup as e:
             spack.store.STORE.db.remove(task.pkg.spec)
             if isinstance(e.inner_exception, Requeue):
@@ -1885,7 +1939,6 @@ def _overwrite_install_task(self, task: Task, install_status: InstallStatus) ->
             # Unwrap the actuall installation exception
             if isinstance(e.inner_exception, Requeue):
                 tty.warn("Task will be requeued to build from source")
-                return rc
             else:
                 raise e.inner_exception
 
@@ -1991,7 +2044,7 @@ def _remove_task(self, pkg_id: str) -> Optional[Task]:
         else:
             return None
 
-    def _requeue_task(self, task: Task, install_status: InstallStatus) -> None:
+    def _requeue_task(self, task: Task) -> None:
         """
         Requeues a task that appears to be in progress by another process.
 
@@ -1999,10 +2052,7 @@ def _requeue_task(self, task: Task, install_status: InstallStatus) -> None:
             task (Task): the installation task for a package
         """
         if task.status not in [BuildStatus.INSTALLED, BuildStatus.INSTALLING]:
-            tty.debug(
+            tty.debug(f"{install_msg(task.pkg_id, self.pid)} in progress by another process")
-                f"{install_msg(task.pkg_id, self.pid, install_status)} "
-                "in progress by another process"
-            )
 
         new_task = task.next_attempt(self.installed)
         new_task.status = BuildStatus.INSTALLING
@@ -2148,8 +2198,6 @@ def install(self) -> None:
         single_requested_spec = len(self.build_requests) == 1
         failed_build_requests = []
 
-        install_status = InstallStatus(len(self.build_pq))
-
         # Only enable the terminal status line when we're in a tty without debug info
         # enabled, so that the output does not get cluttered.
         term_status = TermStatusLine(
@@ -2165,8 +2213,7 @@ def install(self) -> None:
             keep_prefix = install_args.get("keep_prefix")
 
             pkg, pkg_id, spec = task.pkg, task.pkg_id, task.pkg.spec
-            install_status.next_pkg(pkg)
+            self.progress.set_term_title(f"Processing {pkg.name}")
-            install_status.set_term_title(f"Processing {pkg.name}")
             tty.debug(f"Processing {pkg_id}: task={task}")
             # Ensure that the current spec has NO uninstalled dependencies,
             # which is assumed to be reflected directly in its priority.
@@ -2195,7 +2242,7 @@ def install(self) -> None:
             # Skip the installation if the spec is not being installed locally
             # (i.e., if external or upstream) BUT flag it as installed since
             # some package likely depends on it.
-            if _handle_external_and_upstream(pkg, task.explicit):
+            if _handle_external_and_upstream(pkg, task.explicit, self.progress):
                 term_status.clear()
                 self._flag_installed(pkg, task.dependents)
                 continue
@@ -2216,7 +2263,7 @@ def install(self) -> None:
             # another process is likely (un)installing the spec or has
             # determined the spec has already been installed (though the
             # other process may be hung).
-            install_status.set_term_title(f"Acquiring lock for {pkg.name}")
+            self.progress.set_term_title(f"Acquiring lock for {pkg.name}")
             term_status.add(pkg_id)
             ltype, lock = self._ensure_locked("write", pkg)
             if lock is None:
@@ -2228,7 +2275,7 @@ def install(self) -> None:
             # can check the status presumably established by another process
             # -- failed, installed, or uninstalled -- on the next pass.
             if lock is None:
-                self._requeue_task(task, install_status)
+                self._requeue_task(task)
                 continue
 
             term_status.clear()
@@ -2239,7 +2286,7 @@ def install(self) -> None:
                 task.request.overwrite_time = time.time()
 
             # Determine state of installation artifacts and adjust accordingly.
-            install_status.set_term_title(f"Preparing {pkg.name}")
+            self.progress.set_term_title(f"Preparing {pkg.name}")
             self._prepare_for_install(task)
 
             # Flag an already installed package
@@ -2251,7 +2298,7 @@ def install(self) -> None:
                 if lock is not None:
                     self._update_installed(task)
                     path = spack.util.path.debug_padded_filter(pkg.prefix)
-                    _print_installed_pkg(path)
+                    self.progress.set_installed(pkg, path)
                 else:
                     # At this point we've failed to get a write or a read
                     # lock, which means another process has taken a write
@@ -2262,7 +2309,7 @@ def install(self) -> None:
                     # established by the other process -- failed, installed,
                     # or uninstalled -- on the next pass.
                     self.installed.remove(pkg_id)
-                    self._requeue_task(task, install_status)
+                    self._requeue_task(task)
                 continue
 
             # Having a read lock on an uninstalled pkg may mean another
@@ -2275,19 +2322,19 @@ def install(self) -> None:
             # uninstalled -- on the next pass.
             if ltype == "read":
                 lock.release_read()
-                self._requeue_task(task, install_status)
+                self._requeue_task(task)
                 continue
 
             # Proceed with the installation since we have an exclusive write
             # lock on the package.
-            install_status.set_term_title(f"Installing {pkg.name}")
+            self.progress.set_term_title(f"Installing {pkg.name}")
             try:
                 action = self._install_action(task)
 
                 if action == InstallAction.INSTALL:
-                    self._install_task(task, install_status)
+                    self._install_task(task)
                 elif action == InstallAction.OVERWRITE:
-                    self._overwrite_install_task(task, install_status)
+                    self._overwrite_install_task(task)
 
                 # If we installed then we should keep the prefix
                 stop_before_phase = getattr(pkg, "stop_before_phase", None)
@@ -2494,7 +2541,6 @@ def run(self) -> bool:
 
         print_install_test_log(self.pkg)
         _print_timer(pre=self.pre, pkg_id=self.pkg_id, timer=self.timer)
-        _print_installed_pkg(self.pkg.prefix)
 
         # preserve verbosity across runs
         return self.echo

lib/spack/spack/spec.py

@@ -153,8 +153,7 @@
     r"(})?"  # finish format string with non-escaped close brace }, or missing if not present
     r"|"
     # OPTION 3: mismatched close brace (option 2 would consume a matched open brace)
-    r"(})"  # brace
+    r"(})" r")",  # brace
-    r")",
     re.IGNORECASE,
 )
 
@@ -2680,7 +2679,7 @@ def name_and_dependency_types(s: str) -> Tuple[str, dt.DepFlag]:
                 return name, depflag
 
             def spec_and_dependency_types(
-                s: Union[Spec, Tuple[Spec, str]],
+                s: Union[Spec, Tuple[Spec, str]]
             ) -> Tuple[Spec, dt.DepFlag]:
                 """Given a non-string key in the literal, extracts the spec
                 and its dependency types.
@@ -5151,6 +5150,21 @@ def eval_conditional(string):
     return eval(string, valid_variables)
 
 
+class DagCountVisitor:
+    """Class for counting the number of specs encountered during traversal."""
+
+    def __init__(self, depflag: int):
+        self.depflag: int = depflag
+        self.number: int = 0
+
+    def accept(self, item: traverse.EdgeAndDepth) -> bool:
+        self.number += 1
+        return True
+
+    def neighbors(self, item: traverse.EdgeAndDepth):
+        return item.edge.spec.edges_to_dependencies(depflag=self.depflag)
+
+
 class SpecParseError(spack.error.SpecError):
     """Wrapper for ParseError for when we're parsing specs."""
 

lib/spack/spack/test/installer.py

@@ -28,7 +28,7 @@
 import spack.spec
 import spack.store
 import spack.util.lock as lk
-from spack.installer import PackageInstaller
+import spack.util.spack_json as sjson
 from spack.main import SpackCommand
 
 
@@ -123,19 +123,15 @@ def test_install_msg(monkeypatch):
     install_msg = "Installing {0}".format(name)
 
     monkeypatch.setattr(tty, "_debug", 0)
-    assert inst.install_msg(name, pid, None) == install_msg
+    assert inst.install_msg(name, pid) == install_msg
-
-    install_status = inst.InstallStatus(1)
-    expected = "{0} [0/1]".format(install_msg)
-    assert inst.install_msg(name, pid, install_status) == expected
 
     monkeypatch.setattr(tty, "_debug", 1)
-    assert inst.install_msg(name, pid, None) == install_msg
+    assert inst.install_msg(name, pid) == install_msg
 
     # Expect the PID to be added at debug level 2
     monkeypatch.setattr(tty, "_debug", 2)
     expected = "{0}: {1}".format(pid, install_msg)
-    assert inst.install_msg(name, pid, None) == expected
+    assert inst.install_msg(name, pid) == expected
 
 
 def test_install_from_cache_errors(install_mockery):
@@ -147,13 +143,15 @@ def test_install_from_cache_errors(install_mockery):
     with pytest.raises(
         spack.error.InstallError, match="No binary found when cache-only was specified"
     ):
-        PackageInstaller(
+        inst.PackageInstaller(
             [spec.package], package_cache_only=True, dependencies_cache_only=True
         ).install()
     assert not spec.package.installed_from_binary_cache
 
     # Check when don't expect to install only from binary cache
-    assert not inst._install_from_cache(spec.package, explicit=True, unsigned=False)
+    assert not inst._install_from_cache(
+        spec.package, inst.InstallerProgress([spec.package]), explicit=True, unsigned=False
+    )
     assert not spec.package.installed_from_binary_cache
 
 
@@ -163,7 +161,9 @@ def test_install_from_cache_ok(install_mockery, monkeypatch):
     monkeypatch.setattr(inst, "_try_install_from_binary_cache", _true)
     monkeypatch.setattr(spack.hooks, "post_install", _noop)
 
-    assert inst._install_from_cache(spec.package, explicit=True, unsigned=False)
+    assert inst._install_from_cache(
+        spec.package, inst.InstallerProgress([spec.package]), explicit=True, unsigned=False
+    )
 
 
 def test_process_external_package_module(install_mockery, monkeypatch, capfd):
@@ -627,7 +627,7 @@ def test_install_spliced_build_spec_installed(install_mockery, capfd, mock_fetch
 
     # Do the splice.
     out = spec.splice(dep, transitive)
-    PackageInstaller([out.build_spec.package]).install()
+    inst.PackageInstaller([out.build_spec.package]).install()
 
     installer = create_installer([out], {"verbose": True, "fail_fast": True})
     installer._init_queue()
@@ -659,7 +659,7 @@ def test_install_splice_root_from_binary(
     original_spec = spack.concretize.concretize_one(root_str)
     spec_to_splice = spack.concretize.concretize_one("splice-h+foo")
 
-    PackageInstaller([original_spec.package, spec_to_splice.package]).install()
+    inst.PackageInstaller([original_spec.package, spec_to_splice.package]).install()
 
     out = original_spec.splice(spec_to_splice, transitive)
 
@@ -676,7 +676,7 @@ def test_install_splice_root_from_binary(
     uninstall = SpackCommand("uninstall")
     uninstall("-ay")
 
-    PackageInstaller([out.package], unsigned=True).install()
+    inst.PackageInstaller([out.package], unsigned=True).install()
 
     assert len(spack.store.STORE.db.query()) == len(list(out.traverse()))
 
@@ -687,7 +687,7 @@ def test_install_task_use_cache(install_mockery, monkeypatch):
     task = create_install_task(request.pkg)
 
     monkeypatch.setattr(inst, "_install_from_cache", _true)
-    installer._install_task(task, None)
+    installer._install_task(task)
     assert request.pkg_id in installer.installed
 
 
@@ -711,7 +711,7 @@ def _missing(*args, **kwargs):
     assert inst.package_id(popped_task.pkg.spec) not in installer.build_tasks
 
     monkeypatch.setattr(task, "execute", _missing)
-    installer._install_task(task, None)
+    installer._install_task(task)
 
     # Ensure the dropped task/spec was added back by _install_task
     assert inst.package_id(popped_task.pkg.spec) in installer.build_tasks
@@ -759,7 +759,7 @@ def test_requeue_task(install_mockery, capfd):
     # temporarily set tty debug messages on so we can test output
     current_debug_level = tty.debug_level()
     tty.set_debug(1)
-    installer._requeue_task(task, None)
+    installer._requeue_task(task)
     tty.set_debug(current_debug_level)
 
     ids = list(installer.build_tasks)
@@ -912,11 +912,11 @@ def test_install_failed_not_fast(install_mockery, monkeypatch, capsys):
     assert "Skipping build of pkg-a" in out
 
 
-def _interrupt(installer, task, install_status, **kwargs):
+def _interrupt(installer, task, **kwargs):
     if task.pkg.name == "pkg-a":
         raise KeyboardInterrupt("mock keyboard interrupt for pkg-a")
     else:
-        return installer._real_install_task(task, None)
+        return installer._real_install_task(task)
         # installer.installed.add(task.pkg.name)
 
 
@@ -942,12 +942,12 @@ class MyBuildException(Exception):
     pass
 
 
-def _install_fail_my_build_exception(installer, task, install_status, **kwargs):
+def _install_fail_my_build_exception(installer, task, **kwargs):
     if task.pkg.name == "pkg-a":
         raise MyBuildException("mock internal package build error for pkg-a")
     else:
         # No need for more complex logic here because no splices
-        task.execute(install_status)
+        task.execute(installer.progress)
         installer._update_installed(task)
 
 
@@ -1032,8 +1032,8 @@ def test_install_lock_failures(install_mockery, monkeypatch, capfd):
     """Cover basic install lock failure handling in a single pass."""
 
     # Note: this test relies on installing a package with no dependencies
-    def _requeued(installer, task, install_status):
+    def _requeued(installer, task):
-        tty.msg("requeued {0}".format(task.pkg.spec.name))
+        tty.msg(f"requeued {task.pkg.spec.name}")
 
     installer = create_installer(["pkg-c"], {})
 
@@ -1066,7 +1066,7 @@ def _prep(installer, task):
         # also do not allow the package to be locked again
         monkeypatch.setattr(inst.PackageInstaller, "_ensure_locked", _not_locked)
 
-    def _requeued(installer, task, install_status):
+    def _requeued(installer, task):
         tty.msg(f"requeued {inst.package_id(task.pkg.spec)}")
 
     # Flag the package as installed
@@ -1098,8 +1098,8 @@ def _prep(installer, task):
         tty.msg("preparing {0}".format(task.pkg.spec.name))
         assert task.pkg.spec.name not in installer.installed
 
-    def _requeued(installer, task, install_status):
+    def _requeued(installer, task):
-        tty.msg("requeued {0}".format(task.pkg.spec.name))
+        tty.msg(f"requeued {task.pkg.spec.name}")
 
     # Force a read lock
     monkeypatch.setattr(inst.PackageInstaller, "_ensure_locked", _read)
@@ -1156,7 +1156,7 @@ def test_overwrite_install_backup_success(temporary_store, config, mock_packages
     installed_file = os.path.join(task.pkg.prefix, "some_file")
     fs.touchp(installed_file)
 
-    def _install_task(self, task, install_status):
+    def _install_task(self, task):
         shutil.rmtree(task.pkg.prefix, ignore_errors=True)
         fs.mkdirp(task.pkg.prefix)
         raise Exception("Some fatal install error")
@@ -1174,7 +1174,7 @@ def remove(self, spec):
     # Installation should throw the installation exception, not the backup
     # failure.
     with pytest.raises(Exception, match="Some fatal install error"):
-        installer._overwrite_install_task(task, None)
+        installer._overwrite_install_task(task)
 
     # Make sure the package is not marked uninstalled and the original dir
     # is back.
@@ -1197,7 +1197,7 @@ def test_overwrite_install_backup_failure(temporary_store, config, mock_packages
     installed_file = os.path.join(task.pkg.prefix, "some_file")
     fs.touchp(installed_file)
 
-    def _install_task(self, task, install_status):
+    def _install_task(self, task):
         # Remove the backup directory, which is at the same level as the prefix,
         # starting with .backup
         backup_glob = os.path.join(os.path.dirname(os.path.normpath(task.pkg.prefix)), ".backup*")
@@ -1218,7 +1218,7 @@ def remove(self, spec):
     # Installation should throw the installation exception, not the backup
     # failure.
     with pytest.raises(Exception, match="Some fatal install error"):
-        installer._overwrite_install_task(task, None)
+        installer._overwrite_install_task(task)
 
     # Make sure that `remove` was called on the database after an unsuccessful
     # attempt to restore the backup.
@@ -1274,7 +1274,7 @@ def test_print_install_test_log_skipped(install_mockery, mock_packages, capfd, r
     pkg = s.package
 
     pkg.run_tests = run_tests
-    spack.installer.print_install_test_log(pkg)
+    inst.print_install_test_log(pkg)
     out = capfd.readouterr()[0]
     assert out == ""
 
@@ -1291,12 +1291,23 @@ def test_print_install_test_log_failures(
     pkg.run_tests = True
     pkg.tester.test_log_file = str(tmpdir.join("test-log.txt"))
     pkg.tester.add_failure(AssertionError("test"), "test-failure")
-    spack.installer.print_install_test_log(pkg)
+    inst.print_install_test_log(pkg)
     err = capfd.readouterr()[1]
     assert "no test log file" in err
 
     # Having test log results in path being output
     fs.touch(pkg.tester.test_log_file)
-    spack.installer.print_install_test_log(pkg)
+    inst.print_install_test_log(pkg)
     out = capfd.readouterr()[0]
     assert "See test results at" in out
+
+
+def test_specs_count(install_mockery, mock_packages):
+    """Check SpecCounts DAG visitor total matches expected."""
+    spec = spack.spec.Spec("mpileaks^mpich").concretized()
+    counter = inst.SpecsCount(dt.LINK | dt.RUN)
+    number_specs = counter.total([spec])
+
+    json = sjson.load(spec.to_json())
+    number_spec_nodes = len(json["spec"]["nodes"])
+    assert number_specs == number_spec_nodes