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
This commit is contained in:
Peter Scheibel 2020-07-07 11:37:36 -07:00 committed by GitHub
parent 90285c7d61
commit 650ab563f4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -22,6 +22,7 @@
import sys import sys
import textwrap import textwrap
import time import time
import traceback
from six import StringIO from six import StringIO
from six import string_types from six import string_types
from six import with_metaclass from six import with_metaclass
@ -1744,7 +1745,23 @@ def uninstall_by_spec(spec, force=False, deprecator=None):
with spack.store.db.prefix_write_lock(spec): with spack.store.db.prefix_write_lock(spec):
if pkg is not None: 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. # Uninstalling in Spack only requires removing the prefix.
if not spec.external: if not spec.external:
@ -1765,7 +1782,20 @@ def uninstall_by_spec(spec, force=False, deprecator=None):
spack.store.db.remove(spec) spack.store.db.remove(spec)
if pkg is not None: 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) tty.msg("Successfully uninstalled %s" % spec.short_spec)