From 650ab563f42d68ea7e9bf0fe296e2b9b1e78755c Mon Sep 17 00:00:00 2001 From: Peter Scheibel Date: Tue, 7 Jul 2020 11:37:36 -0700 Subject: Uninstall: tolerate hook failures when force=true (#16513) Fixes #16478 This allows an uninstall to proceed even when encountering pre-uninstall hook failures if the user chooses the --force option for the uninstall. This also prevents post-uninstall hook failures from raising an exception, which would terminate a sequence of uninstalls. This isn't likely essential for #16478, but I think overall it will improve the user experience: if the post-uninstall hook fails, there isn't much point in terminating a sequence of spec uninstalls because at the point where the post-uninstall hook is run, the spec has already been removed from the database (so it will never have another chance to run). Notes: * When doing spack uninstall -a, certain pre/post-uninstall hooks aren't important to run, but this isn't easy to track with the current model. For example: if you are uninstalling a package and its extension, you do not have to do the activation check for the extension. * This doesn't handle the uninstallation of specs that are not in the DB, so it may leave "dangling" specs in the installation prefix --- lib/spack/spack/package.py | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 5c4ff23d40..cedcfffed2 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -22,6 +22,7 @@ import shutil import sys import textwrap import time +import traceback from six import StringIO from six import string_types from six import with_metaclass @@ -1744,7 +1745,23 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): with spack.store.db.prefix_write_lock(spec): if pkg is not None: - spack.hooks.pre_uninstall(spec) + try: + spack.hooks.pre_uninstall(spec) + except Exception as error: + if force: + error_msg = ( + "One or more pre_uninstall hooks have failed" + " for {0}, but Spack is continuing with the" + " uninstall".format(str(spec))) + if isinstance(error, spack.error.SpackError): + error_msg += ( + "\n\nError message: {0}".format(str(error))) + tty.warn(error_msg) + # Note that if the uninstall succeeds then we won't be + # seeing this error again and won't have another chance + # to run the hook. + else: + raise # Uninstalling in Spack only requires removing the prefix. if not spec.external: @@ -1765,7 +1782,20 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): spack.store.db.remove(spec) if pkg is not None: - spack.hooks.post_uninstall(spec) + try: + spack.hooks.post_uninstall(spec) + except Exception: + # If there is a failure here, this is our only chance to do + # something about it: at this point the Spec has been removed + # from the DB and prefix, so the post-uninstallation hooks + # will not have another chance to run. + error_msg = ( + "One or more post-uninstallation hooks failed for" + " {0}, but the prefix has been removed (if it is not" + " external).".format(str(spec))) + tb_msg = traceback.format_exc() + error_msg += "\n\nThe error:\n\n{0}".format(tb_msg) + tty.warn(error_msg) tty.msg("Successfully uninstalled %s" % spec.short_spec) -- cgit v1.2.3-70-g09d2