installer: Improve status reporting (#37903)

Refactor `TermTitle` into `InstallStatus` and use it to show progress
information both in the terminal title as well as inline. This also
turns on the terminal title status by default.

The inline output will look like the following after this change:
```
==> Installing m4-1.4.19-w2fxrpuz64zdq63woprqfxxzc3tzu7p3 [4/4]
```
This commit is contained in:
Michael Kuhn 2023-07-12 08:54:45 +02:00 committed by GitHub
parent d3704130b6
commit d5d0b8821c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 70 additions and 53 deletions

View File

@ -216,10 +216,11 @@ config:
# manipulation by unprivileged user (e.g. AFS) # manipulation by unprivileged user (e.g. AFS)
allow_sgid: true allow_sgid: true
# Whether to set the terminal title to display status information during # Whether to show status information during building and installing packages.
# building and installing packages. This gives information about Spack's # This gives information about Spack's current progress as well as the current
# current progress as well as the current and total number of packages. # and total number of packages. Information is shown both in the terminal
terminal_title: false # title and inline.
install_status: true
# Number of seconds a buildcache's index.json is cached locally before probing # Number of seconds a buildcache's index.json is cached locally before probing
# for updates, within a single Spack invocation. Defaults to 10 minutes. # for updates, within a single Spack invocation. Defaults to 10 minutes.

View File

@ -292,12 +292,13 @@ It is also worth noting that:
non_bindable_shared_objects = ["libinterface.so"] non_bindable_shared_objects = ["libinterface.so"]
---------------------- ----------------------
``terminal_title`` ``install_status``
---------------------- ----------------------
By setting this option to ``true``, Spack will update the terminal's title to When set to ``true``, Spack will show information about its current progress
provide information about its current progress as well as the current and as well as the current and total package numbers. Progress is shown both
total package numbers. in the terminal title and inline. Setting it to ``false`` will not show any
progress information.
To work properly, this requires your terminal to reset its title after To work properly, this requires your terminal to reset its title after
Spack has finished its work, otherwise Spack's status information will Spack has finished its work, otherwise Spack's status information will

View File

@ -536,7 +536,7 @@ def get_dependent_ids(spec):
return [package_id(d.package) for d in spec.dependents()] return [package_id(d.package) for d in spec.dependents()]
def install_msg(name, pid): def install_msg(name, pid, install_status):
""" """
Colorize the name/id of the package being installed Colorize the name/id of the package being installed
@ -548,7 +548,12 @@ def install_msg(name, pid):
str: Colorized installing message str: Colorized installing message
""" """
pre = "{0}: ".format(pid) if tty.show_pid() else "" pre = "{0}: ".format(pid) if tty.show_pid() else ""
return pre + colorize("@*{Installing} @*g{%s}" % name) 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))
def archive_install_logs(pkg, phase_log_dir): def archive_install_logs(pkg, phase_log_dir):
@ -657,9 +662,9 @@ def package_id(pkg):
return "{0}-{1}-{2}".format(pkg.name, pkg.version, pkg.spec.dag_hash()) return "{0}-{1}-{2}".format(pkg.name, pkg.version, pkg.spec.dag_hash())
class TermTitle: class InstallStatus:
def __init__(self, pkg_count): def __init__(self, pkg_count):
# Counters used for showing status information in the terminal title # Counters used for showing status information
self.pkg_num = 0 self.pkg_num = 0
self.pkg_count = pkg_count self.pkg_count = pkg_count
self.pkg_ids = set() self.pkg_ids = set()
@ -671,17 +676,20 @@ def next_pkg(self, pkg):
self.pkg_num += 1 self.pkg_num += 1
self.pkg_ids.add(pkg_id) self.pkg_ids.add(pkg_id)
def set(self, text): def set_term_title(self, text):
if not spack.config.get("config:terminal_title", False): if not spack.config.get("config:install_status", True):
return return
if not sys.stdout.isatty(): if not sys.stdout.isatty():
return return
status = "{0} [{1}/{2}]".format(text, self.pkg_num, self.pkg_count) status = "{0} {1}".format(text, self.get_progress())
sys.stdout.write("\033]0;Spack: {0}\007".format(status)) sys.stdout.write("\033]0;Spack: {0}\007".format(status))
sys.stdout.flush() sys.stdout.flush()
def get_progress(self):
return "[{0}/{1}]".format(self.pkg_num, self.pkg_count)
class TermStatusLine: class TermStatusLine:
""" """
@ -1240,7 +1248,7 @@ def _add_compiler_package_to_config(self, pkg):
spack.compilers.find_compilers([compiler_search_prefix]) spack.compilers.find_compilers([compiler_search_prefix])
) )
def _install_task(self, task): def _install_task(self, task, install_status):
""" """
Perform the installation of the requested spec and/or dependency Perform the installation of the requested spec and/or dependency
represented by the build task. represented by the build task.
@ -1257,7 +1265,7 @@ def _install_task(self, task):
pkg, pkg_id = task.pkg, task.pkg_id pkg, pkg_id = task.pkg, task.pkg_id
tty.msg(install_msg(pkg_id, self.pid)) tty.msg(install_msg(pkg_id, self.pid, install_status))
task.start = task.start or time.time() task.start = task.start or time.time()
task.status = STATUS_INSTALLING task.status = STATUS_INSTALLING
@ -1406,7 +1414,7 @@ def _remove_task(self, pkg_id):
else: else:
return None return None
def _requeue_task(self, task): def _requeue_task(self, task, install_status):
""" """
Requeues a task that appears to be in progress by another process. Requeues a task that appears to be in progress by another process.
@ -1416,7 +1424,8 @@ def _requeue_task(self, task):
if task.status not in [STATUS_INSTALLED, STATUS_INSTALLING]: if task.status not in [STATUS_INSTALLED, STATUS_INSTALLING]:
tty.debug( tty.debug(
"{0} {1}".format( "{0} {1}".format(
install_msg(task.pkg_id, self.pid), "in progress by another process" install_msg(task.pkg_id, self.pid, install_status),
"in progress by another process",
) )
) )
@ -1595,7 +1604,7 @@ def install(self):
single_explicit_spec = len(self.build_requests) == 1 single_explicit_spec = len(self.build_requests) == 1
failed_explicits = [] failed_explicits = []
term_title = TermTitle(len(self.build_pq)) install_status = InstallStatus(len(self.build_pq))
# Only enable the terminal status line when we're in a tty without debug info # Only enable the terminal status line when we're in a tty without debug info
# enabled, so that the output does not get cluttered. # enabled, so that the output does not get cluttered.
@ -1611,8 +1620,8 @@ def install(self):
keep_prefix = install_args.get("keep_prefix") keep_prefix = install_args.get("keep_prefix")
pkg, pkg_id, spec = task.pkg, task.pkg_id, task.pkg.spec pkg, pkg_id, spec = task.pkg, task.pkg_id, task.pkg.spec
term_title.next_pkg(pkg) install_status.next_pkg(pkg)
term_title.set("Processing {0}".format(pkg.name)) install_status.set_term_title("Processing {0}".format(pkg.name))
tty.debug("Processing {0}: task={1}".format(pkg_id, task)) tty.debug("Processing {0}: task={1}".format(pkg_id, task))
# Ensure that the current spec has NO uninstalled dependencies, # Ensure that the current spec has NO uninstalled dependencies,
# which is assumed to be reflected directly in its priority. # which is assumed to be reflected directly in its priority.
@ -1676,7 +1685,7 @@ def install(self):
# another process is likely (un)installing the spec or has # another process is likely (un)installing the spec or has
# determined the spec has already been installed (though the # determined the spec has already been installed (though the
# other process may be hung). # other process may be hung).
term_title.set("Acquiring lock for {0}".format(pkg.name)) install_status.set_term_title("Acquiring lock for {0}".format(pkg.name))
term_status.add(pkg_id) term_status.add(pkg_id)
ltype, lock = self._ensure_locked("write", pkg) ltype, lock = self._ensure_locked("write", pkg)
if lock is None: if lock is None:
@ -1689,7 +1698,7 @@ def install(self):
# can check the status presumably established by another process # can check the status presumably established by another process
# -- failed, installed, or uninstalled -- on the next pass. # -- failed, installed, or uninstalled -- on the next pass.
if lock is None: if lock is None:
self._requeue_task(task) self._requeue_task(task, install_status)
continue continue
term_status.clear() term_status.clear()
@ -1700,7 +1709,7 @@ def install(self):
task.request.overwrite_time = time.time() task.request.overwrite_time = time.time()
# Determine state of installation artifacts and adjust accordingly. # Determine state of installation artifacts and adjust accordingly.
term_title.set("Preparing {0}".format(pkg.name)) install_status.set_term_title("Preparing {0}".format(pkg.name))
self._prepare_for_install(task) self._prepare_for_install(task)
# Flag an already installed package # Flag an already installed package
@ -1728,7 +1737,7 @@ def install(self):
# established by the other process -- failed, installed, # established by the other process -- failed, installed,
# or uninstalled -- on the next pass. # or uninstalled -- on the next pass.
self.installed.remove(pkg_id) self.installed.remove(pkg_id)
self._requeue_task(task) self._requeue_task(task, install_status)
continue continue
# Having a read lock on an uninstalled pkg may mean another # Having a read lock on an uninstalled pkg may mean another
@ -1741,19 +1750,19 @@ def install(self):
# uninstalled -- on the next pass. # uninstalled -- on the next pass.
if ltype == "read": if ltype == "read":
lock.release_read() lock.release_read()
self._requeue_task(task) self._requeue_task(task, install_status)
continue continue
# Proceed with the installation since we have an exclusive write # Proceed with the installation since we have an exclusive write
# lock on the package. # lock on the package.
term_title.set("Installing {0}".format(pkg.name)) install_status.set_term_title("Installing {0}".format(pkg.name))
try: try:
action = self._install_action(task) action = self._install_action(task)
if action == InstallAction.INSTALL: if action == InstallAction.INSTALL:
self._install_task(task) self._install_task(task, install_status)
elif action == InstallAction.OVERWRITE: elif action == InstallAction.OVERWRITE:
OverwriteInstall(self, spack.store.db, task).install() OverwriteInstall(self, spack.store.db, task, install_status).install()
self._update_installed(task) self._update_installed(task)
@ -1779,7 +1788,7 @@ def install(self):
err += " Requeueing to install from source." err += " Requeueing to install from source."
tty.error(err.format(pkg.name, str(exc))) tty.error(err.format(pkg.name, str(exc)))
task.use_cache = False task.use_cache = False
self._requeue_task(task) self._requeue_task(task, install_status)
continue continue
except (Exception, SystemExit) as exc: except (Exception, SystemExit) as exc:
@ -2092,10 +2101,11 @@ def build_process(pkg, install_args):
class OverwriteInstall: class OverwriteInstall:
def __init__(self, installer, database, task): def __init__(self, installer, database, task, install_status):
self.installer = installer self.installer = installer
self.database = database self.database = database
self.task = task self.task = task
self.install_status = install_status
def install(self): def install(self):
""" """
@ -2106,7 +2116,7 @@ def install(self):
""" """
try: try:
with fs.replace_directory_transaction(self.task.pkg.prefix): with fs.replace_directory_transaction(self.task.pkg.prefix):
self.installer._install_task(self.task) self.installer._install_task(self.task, self.install_status)
except fs.CouldNotRestoreDirectoryBackup as e: except fs.CouldNotRestoreDirectoryBackup as e:
self.database.remove(self.task.pkg.spec) self.database.remove(self.task.pkg.spec)
tty.error( tty.error(

View File

@ -87,15 +87,16 @@
"anyOf": [{"type": "integer", "minimum": 1}, {"type": "null"}] "anyOf": [{"type": "integer", "minimum": 1}, {"type": "null"}]
}, },
"allow_sgid": {"type": "boolean"}, "allow_sgid": {"type": "boolean"},
"install_status": {"type": "boolean"},
"binary_index_root": {"type": "string"}, "binary_index_root": {"type": "string"},
"url_fetch_method": {"type": "string", "enum": ["urllib", "curl"]}, "url_fetch_method": {"type": "string", "enum": ["urllib", "curl"]},
"additional_external_search_paths": {"type": "array", "items": {"type": "string"}}, "additional_external_search_paths": {"type": "array", "items": {"type": "string"}},
"binary_index_ttl": {"type": "integer", "minimum": 0}, "binary_index_ttl": {"type": "integer", "minimum": 0},
}, },
"deprecatedProperties": { "deprecatedProperties": {
"properties": ["module_roots"], "properties": ["terminal_title"],
"message": "config:module_roots has been replaced by " "message": "config:terminal_title has been replaced by "
"modules:[module set]:roots and is ignored", "install_status and is ignored",
"error": False, "error": False,
}, },
} }

View File

@ -148,15 +148,19 @@ def test_install_msg(monkeypatch):
install_msg = "Installing {0}".format(name) install_msg = "Installing {0}".format(name)
monkeypatch.setattr(tty, "_debug", 0) monkeypatch.setattr(tty, "_debug", 0)
assert inst.install_msg(name, pid) == install_msg 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
monkeypatch.setattr(tty, "_debug", 1) monkeypatch.setattr(tty, "_debug", 1)
assert inst.install_msg(name, pid) == install_msg assert inst.install_msg(name, pid, None) == install_msg
# Expect the PID to be added at debug level 2 # Expect the PID to be added at debug level 2
monkeypatch.setattr(tty, "_debug", 2) monkeypatch.setattr(tty, "_debug", 2)
expected = "{0}: {1}".format(pid, install_msg) expected = "{0}: {1}".format(pid, install_msg)
assert inst.install_msg(name, pid) == expected assert inst.install_msg(name, pid, None) == expected
def test_install_from_cache_errors(install_mockery, capsys): def test_install_from_cache_errors(install_mockery, capsys):
@ -795,7 +799,7 @@ def test_install_task_use_cache(install_mockery, monkeypatch):
task = create_build_task(request.pkg) task = create_build_task(request.pkg)
monkeypatch.setattr(inst, "_install_from_cache", _true) monkeypatch.setattr(inst, "_install_from_cache", _true)
installer._install_task(task) installer._install_task(task, None)
assert request.pkg_id in installer.installed assert request.pkg_id in installer.installed
@ -817,7 +821,7 @@ def _add(_compilers):
monkeypatch.setattr(spack.database.Database, "add", _noop) monkeypatch.setattr(spack.database.Database, "add", _noop)
monkeypatch.setattr(spack.compilers, "add_compilers_to_config", _add) monkeypatch.setattr(spack.compilers, "add_compilers_to_config", _add)
installer._install_task(task) installer._install_task(task, None)
out = capfd.readouterr()[0] out = capfd.readouterr()[0]
assert config_msg in out assert config_msg in out
@ -868,7 +872,7 @@ def test_requeue_task(install_mockery, capfd):
# temporarily set tty debug messages on so we can test output # temporarily set tty debug messages on so we can test output
current_debug_level = tty.debug_level() current_debug_level = tty.debug_level()
tty.set_debug(1) tty.set_debug(1)
installer._requeue_task(task) installer._requeue_task(task, None)
tty.set_debug(current_debug_level) tty.set_debug(current_debug_level)
ids = list(installer.build_tasks) ids = list(installer.build_tasks)
@ -1031,7 +1035,7 @@ def test_install_fail_on_interrupt(install_mockery, monkeypatch):
spec_name = "a" spec_name = "a"
err_msg = "mock keyboard interrupt for {0}".format(spec_name) err_msg = "mock keyboard interrupt for {0}".format(spec_name)
def _interrupt(installer, task, **kwargs): def _interrupt(installer, task, install_status, **kwargs):
if task.pkg.name == spec_name: if task.pkg.name == spec_name:
raise KeyboardInterrupt(err_msg) raise KeyboardInterrupt(err_msg)
else: else:
@ -1058,7 +1062,7 @@ def test_install_fail_single(install_mockery, monkeypatch):
class MyBuildException(Exception): class MyBuildException(Exception):
pass pass
def _install(installer, task, **kwargs): def _install(installer, task, install_status, **kwargs):
if task.pkg.name == spec_name: if task.pkg.name == spec_name:
raise MyBuildException(err_msg) raise MyBuildException(err_msg)
else: else:
@ -1085,7 +1089,7 @@ def test_install_fail_multi(install_mockery, monkeypatch):
class MyBuildException(Exception): class MyBuildException(Exception):
pass pass
def _install(installer, task, **kwargs): def _install(installer, task, install_status, **kwargs):
if task.pkg.name == spec_name: if task.pkg.name == spec_name:
raise MyBuildException(err_msg) raise MyBuildException(err_msg)
else: else:
@ -1157,7 +1161,7 @@ def test_install_fail_fast_on_except(install_mockery, monkeypatch, capsys):
def test_install_lock_failures(install_mockery, monkeypatch, capfd): def test_install_lock_failures(install_mockery, monkeypatch, capfd):
"""Cover basic install lock failure handling in a single pass.""" """Cover basic install lock failure handling in a single pass."""
def _requeued(installer, task): def _requeued(installer, task, install_status):
tty.msg("requeued {0}".format(task.pkg.spec.name)) tty.msg("requeued {0}".format(task.pkg.spec.name))
const_arg = installer_args(["b"], {}) const_arg = installer_args(["b"], {})
@ -1192,7 +1196,7 @@ def _prep(installer, task):
# also do not allow the package to be locked again # also do not allow the package to be locked again
monkeypatch.setattr(inst.PackageInstaller, "_ensure_locked", _not_locked) monkeypatch.setattr(inst.PackageInstaller, "_ensure_locked", _not_locked)
def _requeued(installer, task): def _requeued(installer, task, install_status):
tty.msg("requeued {0}".format(inst.package_id(task.pkg))) tty.msg("requeued {0}".format(inst.package_id(task.pkg)))
# Flag the package as installed # Flag the package as installed
@ -1224,7 +1228,7 @@ def _prep(installer, task):
tty.msg("preparing {0}".format(task.pkg.spec.name)) tty.msg("preparing {0}".format(task.pkg.spec.name))
assert task.pkg.spec.name not in installer.installed assert task.pkg.spec.name not in installer.installed
def _requeued(installer, task): def _requeued(installer, task, install_status):
tty.msg("requeued {0}".format(task.pkg.spec.name)) tty.msg("requeued {0}".format(task.pkg.spec.name))
# Force a read lock # Force a read lock
@ -1289,7 +1293,7 @@ def test_overwrite_install_backup_success(temporary_store, config, mock_packages
fs.touchp(installed_file) fs.touchp(installed_file)
class InstallerThatWipesThePrefixDir: class InstallerThatWipesThePrefixDir:
def _install_task(self, task): def _install_task(self, task, install_status):
shutil.rmtree(task.pkg.prefix, ignore_errors=True) shutil.rmtree(task.pkg.prefix, ignore_errors=True)
fs.mkdirp(task.pkg.prefix) fs.mkdirp(task.pkg.prefix)
raise Exception("Some fatal install error") raise Exception("Some fatal install error")
@ -1302,7 +1306,7 @@ def remove(self, spec):
fake_installer = InstallerThatWipesThePrefixDir() fake_installer = InstallerThatWipesThePrefixDir()
fake_db = FakeDatabase() fake_db = FakeDatabase()
overwrite_install = inst.OverwriteInstall(fake_installer, fake_db, task) overwrite_install = inst.OverwriteInstall(fake_installer, fake_db, task, None)
# Installation should throw the installation exception, not the backup # Installation should throw the installation exception, not the backup
# failure. # failure.
@ -1323,7 +1327,7 @@ def test_overwrite_install_backup_failure(temporary_store, config, mock_packages
""" """
class InstallerThatAccidentallyDeletesTheBackupDir: class InstallerThatAccidentallyDeletesTheBackupDir:
def _install_task(self, task): def _install_task(self, task, install_status):
# Remove the backup directory, which is at the same level as the prefix, # Remove the backup directory, which is at the same level as the prefix,
# starting with .backup # starting with .backup
backup_glob = os.path.join( backup_glob = os.path.join(
@ -1351,7 +1355,7 @@ def remove(self, spec):
fake_installer = InstallerThatAccidentallyDeletesTheBackupDir() fake_installer = InstallerThatAccidentallyDeletesTheBackupDir()
fake_db = FakeDatabase() fake_db = FakeDatabase()
overwrite_install = inst.OverwriteInstall(fake_installer, fake_db, task) overwrite_install = inst.OverwriteInstall(fake_installer, fake_db, task, None)
# Installation should throw the installation exception, not the backup # Installation should throw the installation exception, not the backup
# failure. # failure.

View File

@ -1,4 +1,4 @@
SPACK ?= spack SPACK ?= spack -c config:install_status:false
SPACK_INSTALL_FLAGS ?= SPACK_INSTALL_FLAGS ?=
# This variable can be used to add post install hooks # This variable can be used to add post install hooks