From a8a62e8f5a437bbc19f311b8581faa5a64d9c14c Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Wed, 13 Nov 2024 15:43:47 -0800 Subject: [PATCH] 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 | 204 ++++++++++++++++++------------ lib/spack/spack/spec.py | 20 ++- lib/spack/spack/test/installer.py | 77 ++++++----- 3 files changed, 186 insertions(+), 115 deletions(-) diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 9e5fbef8800..e9261beeccd 100644 --- a/lib/spack/spack/installer.py +++ b/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: - def __init__(self, pkg_count: int): - # Counters used for showing status information - self.pkg_num: int = 0 - self.pkg_count: int = pkg_count +class InstallerProgress: + """Installation progress tracker""" + + def __init__(self, packages: List["spack.package_base.PackageBase"]): + 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 = ( - " @*{%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)) + return pre + colorize("@*{Installing} @*g{%s}" % (name)) 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 - this task in the context of the full ``BuildRequest``.""" + Args: + 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 - install error if installation fails. + too, mark the spec as uninstalled. """ 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( - f"{install_msg(task.pkg_id, self.pid, install_status)} " - "in progress by another process" - ) + tty.debug(f"{install_msg(task.pkg_id, self.pid)} 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) - install_status.set_term_title(f"Processing {pkg.name}") + self.progress.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 diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index cf116e1fc16..da49b62f7bd 100644 --- a/lib/spack/spack/spec.py +++ b/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"(})" r")", # brace 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.""" diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index 4488eaf6542..999cccd6852 100644 --- a/lib/spack/spack/test/installer.py +++ b/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 - - install_status = inst.InstallStatus(1) - expected = "{0} [0/1]".format(install_msg) - assert inst.install_msg(name, pid, install_status) == expected + assert inst.install_msg(name, pid) == install_msg 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): - tty.msg("requeued {0}".format(task.pkg.spec.name)) + def _requeued(installer, task): + 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): - tty.msg("requeued {0}".format(task.pkg.spec.name)) + def _requeued(installer, task): + 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