diff options
| -rw-r--r-- | lib/spack/llnl/string.py | 14 | ||||
| -rw-r--r-- | lib/spack/spack/installer.py | 130 | ||||
| -rw-r--r-- | lib/spack/spack/test/buildtask.py | 37 | ||||
| -rw-r--r-- | lib/spack/spack/test/installer.py | 4 |
4 files changed, 121 insertions, 64 deletions
diff --git a/lib/spack/llnl/string.py b/lib/spack/llnl/string.py index d2995f34c1..0ad67d6775 100644 --- a/lib/spack/llnl/string.py +++ b/lib/spack/llnl/string.py @@ -41,6 +41,20 @@ def comma_and(sequence: List[str]) -> str: return comma_list(sequence, "and") +def ordinal(number: int) -> str: + """Return the ordinal representation (1st, 2nd, 3rd, etc.) for the provided number. + + Args: + number: int to convert to ordinal number + + Returns: number's corresponding ordinal + """ + idx = (number % 10) << 1 + tens = number % 100 // 10 + suffix = "th" if tens == 1 or idx > 6 else "thstndrd"[idx : idx + 2] + return f"{number}{suffix}" + + def quote(sequence: List[str], q: str = "'") -> List[str]: """Quotes each item in the input list with the quote character passed as second argument.""" return [f"{q}{e}{q}" for e in sequence] diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index a02c6bc856..e4a5c33dd3 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -27,6 +27,7 @@ installations of packages in a Spack instance. """ import copy +import enum import glob import heapq import io @@ -42,6 +43,7 @@ from typing import Dict, Iterator, List, Optional, Set, Tuple, Union import llnl.util.filesystem as fs import llnl.util.lock as lk import llnl.util.tty as tty +from llnl.string import ordinal from llnl.util.lang import pretty_seconds from llnl.util.tty.color import colorize from llnl.util.tty.log import log_output @@ -70,25 +72,32 @@ from spack.util.executable import which #: were added (see https://docs.python.org/2/library/heapq.html). _counter = itertools.count(0) -#: Build status indicating task has been added. -STATUS_ADDED = "queued" -#: Build status indicating the spec failed to install -STATUS_FAILED = "failed" +class BuildStatus(enum.Enum): + """Different build (task) states.""" -#: Build status indicating the spec is being installed (possibly by another -#: process) -STATUS_INSTALLING = "installing" + #: Build status indicating task has been added/queued. + QUEUED = enum.auto() -#: Build status indicating the spec was sucessfully installed -STATUS_INSTALLED = "installed" + #: Build status indicating the spec failed to install + FAILED = enum.auto() -#: Build status indicating the task has been popped from the queue -STATUS_DEQUEUED = "dequeued" + #: Build status indicating the spec is being installed (possibly by another + #: process) + INSTALLING = enum.auto() -#: Build status indicating task has been removed (to maintain priority -#: queue invariants). -STATUS_REMOVED = "removed" + #: Build status indicating the spec was sucessfully installed + INSTALLED = enum.auto() + + #: Build status indicating the task has been popped from the queue + DEQUEUED = enum.auto() + + #: Build status indicating task has been removed (to maintain priority + #: queue invariants). + REMOVED = enum.auto() + + def __str__(self): + return f"{self.name.lower()}" def _write_timer_json(pkg, timer, cache): @@ -846,31 +855,38 @@ class BuildTask: def __init__( self, pkg: "spack.package_base.PackageBase", - request: Optional[BuildRequest], - compiler: bool, - start: float, - attempts: int, - status: str, - installed: Set[str], + request: BuildRequest, + *, + compiler: bool = False, + start: float = 0.0, + attempts: int = 0, + status: BuildStatus = BuildStatus.QUEUED, + installed: Set[str] = set(), ): """ Instantiate a build task for a package. Args: - pkg: the package to be built and installed - request: the associated install request where ``None`` can be - used to indicate the package was explicitly requested by the user + pkg: the package to be built and installed and whose spec is + concrete + request: the associated install request compiler: whether task is for a bootstrap compiler start: the initial start time for the package, in seconds - attempts: the number of attempts to install the package + attempts: the number of attempts to install the package, which + should be 0 when the task is initially instantiated status: the installation status - installed: the identifiers of packages that have + installed: the (string) identifiers of packages that have been installed so far + + Raises: + ``InstallError`` if the build status is incompatible with the task + ``TypeError`` if provided an argument of the wrong type + ``ValueError`` if provided an argument with the wrong value or state """ # Ensure dealing with a package that has a concrete spec if not isinstance(pkg, spack.package_base.PackageBase): - raise ValueError(f"{str(pkg)} must be a package") + raise TypeError(f"{str(pkg)} must be a package") self.pkg = pkg if not self.pkg.spec.concrete: @@ -881,18 +897,20 @@ class BuildTask: # The explicit build request associated with the package if not isinstance(request, BuildRequest): - raise ValueError(f"{str(pkg)} must have a build request") - + raise TypeError(f"{request} is not a valid build request") self.request = request # Initialize the status to an active state. The status is used to # ensure priority queue invariants when tasks are "removed" from the # queue. - if status == STATUS_REMOVED: + if not isinstance(status, BuildStatus): + raise TypeError(f"{status} is not a valid build status") + + # The initial build task cannot have status "removed". + if attempts == 0 and status == BuildStatus.REMOVED: raise spack.error.InstallError( f"Cannot create a build task for {self.pkg_id} with status '{status}'", pkg=pkg ) - self.status = status # Package is associated with a bootstrap compiler @@ -901,6 +919,12 @@ class BuildTask: # The initial start time for processing the spec self.start = start + if not isinstance(installed, set): + raise TypeError( + f"BuildTask constructor requires 'installed' be a 'set', " + f"not '{installed.__class__.__name__}'." + ) + # Set of dependents, which needs to include the requesting package # to support tracking of parallel, multi-spec, environment installs. self.dependents = set(get_dependent_ids(self.pkg.spec)) @@ -922,13 +946,12 @@ class BuildTask: # List of uninstalled dependencies, which is used to establish # the priority of the build task. - # self.uninstalled_deps = set( pkg_id for pkg_id in self.dependencies if pkg_id not in installed ) # Ensure key sequence-related properties are updated accordingly. - self.attempts = 0 + self.attempts = attempts self._update() def __eq__(self, other): @@ -1180,7 +1203,7 @@ class PackageInstaller: def _add_init_task( self, pkg: "spack.package_base.PackageBase", - request: Optional[BuildRequest], + request: BuildRequest, is_compiler: bool, all_deps: Dict[str, Set[str]], ) -> None: @@ -1189,14 +1212,17 @@ class PackageInstaller: Args: pkg: the package to be built and installed - request (BuildRequest or None): the associated install request - where ``None`` can be used to indicate the package was - explicitly requested by the user - is_compiler (bool): whether task is for a bootstrap compiler - all_deps (defaultdict(set)): dictionary of all dependencies and - associated dependents + request: the associated install request + is_compiler: whether task is for a bootstrap compiler + all_deps: dictionary of all dependencies and associated dependents """ - task = BuildTask(pkg, request, is_compiler, 0, 0, STATUS_ADDED, self.installed) + task = BuildTask( + pkg, + request=request, + compiler=is_compiler, + status=BuildStatus.QUEUED, + installed=self.installed, + ) for dep_id in task.dependencies: all_deps[dep_id].add(package_id(pkg.spec)) @@ -1514,7 +1540,7 @@ class PackageInstaller: dep_id = package_id(dep) if dep_id not in self.build_tasks: - self._add_init_task(dep_pkg, request, False, all_deps) + self._add_init_task(dep_pkg, request, is_compiler=False, all_deps=all_deps) # Clear any persistent failure markings _unless_ they are # associated with another process in this parallel build @@ -1532,7 +1558,7 @@ class PackageInstaller: self._check_deps_status(request) # Now add the package itself, if appropriate - self._add_init_task(request.pkg, request, False, all_deps) + self._add_init_task(request.pkg, request, is_compiler=False, all_deps=all_deps) # Ensure if one request is to fail fast then all requests will. fail_fast = bool(request.install_args.get("fail_fast")) @@ -1559,7 +1585,7 @@ class PackageInstaller: tty.msg(install_msg(pkg_id, self.pid, install_status)) task.start = task.start or time.time() - task.status = STATUS_INSTALLING + task.status = BuildStatus.INSTALLING # Use the binary cache if requested if use_cache: @@ -1623,9 +1649,9 @@ class PackageInstaller: """ while self.build_pq: task = heapq.heappop(self.build_pq)[1] - if task.status != STATUS_REMOVED: + if task.status != BuildStatus.REMOVED: del self.build_tasks[task.pkg_id] - task.status = STATUS_DEQUEUED + task.status = BuildStatus.DEQUEUED return task return None @@ -1654,7 +1680,9 @@ class PackageInstaller: # Remove any associated build task since its sequence will change self._remove_task(task.pkg_id) - desc = "Queueing" if task.attempts == 0 else "Requeueing" + desc = ( + "Queueing" if task.attempts == 1 else f"Requeueing ({ordinal(task.attempts)} attempt)" + ) tty.debug(msg.format(desc, task.pkg_id, task.status)) # Now add the new task to the queue with a new sequence number to @@ -1698,7 +1726,7 @@ class PackageInstaller: if pkg_id in self.build_tasks: tty.debug(f"Removing build task for {pkg_id} from list") task = self.build_tasks.pop(pkg_id) - task.status = STATUS_REMOVED + task.status = BuildStatus.REMOVED return task else: return None @@ -1710,14 +1738,14 @@ class PackageInstaller: Args: task (BuildTask): the installation build task for a package """ - if task.status not in [STATUS_INSTALLED, STATUS_INSTALLING]: + 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" ) new_task = task.next_attempt(self.installed) - new_task.status = STATUS_INSTALLING + new_task.status = BuildStatus.INSTALLING self._push_task(new_task) def _setup_install_dir(self, pkg: "spack.package_base.PackageBase") -> None: @@ -1772,7 +1800,7 @@ class PackageInstaller: self.failed[pkg_id] = spack.store.STORE.failure_tracker.mark(task.pkg.spec) else: self.failed[pkg_id] = None - task.status = STATUS_FAILED + task.status = BuildStatus.FAILED for dep_id in task.dependents: if dep_id in self.build_tasks: @@ -1792,7 +1820,7 @@ class PackageInstaller: Args: task (BuildTask): the build task for the installed package """ - task.status = STATUS_INSTALLED + task.status = BuildStatus.INSTALLED self._flag_installed(task.pkg, task.dependents) def _flag_installed( diff --git a/lib/spack/spack/test/buildtask.py b/lib/spack/spack/test/buildtask.py index 61def432c0..4f018eb53e 100644 --- a/lib/spack/spack/test/buildtask.py +++ b/lib/spack/spack/test/buildtask.py @@ -12,22 +12,37 @@ import spack.spec def test_build_task_errors(install_mockery): - with pytest.raises(ValueError, match="must be a package"): - inst.BuildTask("abc", None, False, 0, 0, 0, set()) - + """Check expected errors when instantiating a BuildTask.""" spec = spack.spec.Spec("trivial-install-test-package") pkg_cls = spack.repo.PATH.get_pkg_class(spec.name) + + # The value of the request argument is expected to not be checked. + for pkg in [None, "abc"]: + with pytest.raises(TypeError, match="must be a package"): + inst.BuildTask(pkg, None) + with pytest.raises(ValueError, match="must have a concrete spec"): - inst.BuildTask(pkg_cls(spec), None, False, 0, 0, 0, set()) + inst.BuildTask(pkg_cls(spec), None) + # Using a concretized package now means the request argument is checked. spec.concretize() assert spec.concrete - with pytest.raises(ValueError, match="must have a build request"): - inst.BuildTask(spec.package, None, False, 0, 0, 0, set()) + with pytest.raises(TypeError, match="is not a valid build request"): + inst.BuildTask(spec.package, None) + # Using a valid package and spec, the next check is the status argument. request = inst.BuildRequest(spec.package, {}) + with pytest.raises(TypeError, match="is not a valid build status"): + inst.BuildTask(spec.package, request, status="queued") + + # Now we can check that build tasks cannot be create when the status + # indicates the task is/should've been removed. with pytest.raises(spack.error.InstallError, match="Cannot create a build task"): - inst.BuildTask(spec.package, request, False, 0, 0, inst.STATUS_REMOVED, set()) + inst.BuildTask(spec.package, request, status=inst.BuildStatus.REMOVED) + + # Also make sure to not accept an incompatible installed argument value. + with pytest.raises(TypeError, match="'installed' be a 'set', not 'str'"): + inst.BuildTask(spec.package, request, installed="mpileaks") def test_build_task_basics(install_mockery): @@ -37,7 +52,7 @@ 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, set()) + task = inst.BuildTask(spec.package, request=request, status=inst.BuildStatus.QUEUED) assert not task.explicit assert task.priority == len(task.uninstalled_deps) assert task.key == (task.priority, task.sequence) @@ -59,16 +74,16 @@ 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, set()) + task = inst.BuildTask(spec.package, request=request, status=inst.BuildStatus.QUEUED) # Cover __repr__ irep = task.__repr__() assert irep.startswith(task.__class__.__name__) - assert "status='queued'" in irep # == STATUS_ADDED + assert "BuildStatus.QUEUED" in irep assert "sequence=" in irep # Cover __str__ istr = str(task) - assert "status=queued" in istr # == STATUS_ADDED + assert "status=queued" in istr # == BuildStatus.QUEUED assert "#dependencies=1" in istr assert "priority=" in istr diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index a95d151b50..f2d9df672f 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -73,7 +73,7 @@ def create_build_task( pkg: spack.package_base.PackageBase, install_args: Optional[dict] = None ) -> inst.BuildTask: request = inst.BuildRequest(pkg, {} if install_args is None else install_args) - return inst.BuildTask(pkg, request, False, 0, 0, inst.STATUS_ADDED, set()) + return inst.BuildTask(pkg, request=request, status=inst.BuildStatus.QUEUED) def create_installer( @@ -698,7 +698,7 @@ def test_requeue_task(install_mockery, capfd): ids = list(installer.build_tasks) assert len(ids) == 1 qtask = installer.build_tasks[ids[0]] - assert qtask.status == inst.STATUS_INSTALLING + assert qtask.status == inst.BuildStatus.INSTALLING assert qtask.sequence > task.sequence assert qtask.attempts == task.attempts + 1 |