build systems: simpler, clearer decorators: run_after, run_before (#2860)

* PackageMeta: `run_before` is an alias of `precondition`, `run_after` an alias of `sanity_check`

* PackageMeta: removed `precondition` and `sanity_check`

* PackageMeta: decorators are now free-standing

* package: modified/added docstrings. Fixed the semantics of `on_package_attributes`.

* package: added unit test assertion as side effects of install

* build_systems: factored build-time test running into base class

* r: updated decorators in package.py

* docs: updated decorator names

21 files changed, 174 insertions, 135 deletions

diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst
index f3927a0709..9b08a7d498 100644
--- a/lib/spack/docs/packaging_guide.rst
+++ b/lib/spack/docs/packaging_guide.rst
@@ -2775,17 +2775,17 @@ any base class listed in :ref:`installation_procedure`.
 
 .. code-block:: python
 
-   @MakefilePackage.sanity_check('build')
-   @MakefilePackage.on_package_attributes(run_tests=True)
+   @run_after('build')
+   @on_package_attributes(run_tests=True)
    def check_build(self):
         # Custom implementation goes here
         pass
 
-The first decorator ``MakefilePackage.sanity_check('build')`` schedules this
+The first decorator ``run_after('build')`` schedules this
 function to be invoked after the ``build`` phase has been executed, while the
 second one makes the invocation  conditional on the fact that ``self.run_tests == True``.
 It is also possible to schedule a function to be invoked *before* a given phase
-using the ``MakefilePackage.precondition`` decorator.
+using the ``run_before`` decorator.
 
 .. note::
 
diff --git a/lib/spack/spack/__init__.py b/lib/spack/spack/__init__.py
index 0b1934112e..6a28fbb2b0 100644
--- a/lib/spack/spack/__init__.py
+++ b/lib/spack/spack/__init__.py
@@ -156,14 +156,24 @@ dirty = _config.get('dirty', False)
 #-----------------------------------------------------------------------------
 __all__ = []
 
-from spack.package import Package
+from spack.package import Package, run_before, run_after, on_package_attributes
 from spack.build_systems.makefile import MakefilePackage
 from spack.build_systems.autotools import AutotoolsPackage
 from spack.build_systems.cmake import CMakePackage
 from spack.build_systems.python import PythonPackage
 from spack.build_systems.r import RPackage
-__all__ += ['Package', 'CMakePackage', 'AutotoolsPackage', 'MakefilePackage',
-            'PythonPackage', 'RPackage']
+
+__all__ += [
+    'run_before',
+    'run_after',
+    'on_package_attributes',
+    'Package',
+    'CMakePackage',
+    'AutotoolsPackage',
+    'MakefilePackage',
+    'PythonPackage',
+    'RPackage'
+]
 
 from spack.version import Version, ver
 __all__ += ['Version', 'ver']
diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py
index 37c780b360..d08ea02428 100644
--- a/lib/spack/spack/build_systems/autotools.py
+++ b/lib/spack/spack/build_systems/autotools.py
@@ -30,9 +30,8 @@ import shutil
 from subprocess import PIPE
 from subprocess import check_call
 
-import llnl.util.tty as tty
 from llnl.util.filesystem import working_dir
-from spack.package import PackageBase
+from spack.package import PackageBase, run_after
 
 
 class AutotoolsPackage(PackageBase):
@@ -80,6 +79,8 @@ class AutotoolsPackage(PackageBase):
     #: phase
     install_targets = ['install']
 
+    build_time_test_callbacks = ['check']
+
     def _do_patch_config_guess(self):
         """Some packages ship with an older config.guess and need to have
         this updated when installed on a newer architecture."""
@@ -168,7 +169,7 @@ class AutotoolsPackage(PackageBase):
         """Not needed usually, configure should be already there"""
         pass
 
-    @PackageBase.sanity_check('autoreconf')
+    @run_after('autoreconf')
     def is_configure_or_die(self):
         """Checks the presence of a `configure` file after the
         :py:meth:`.autoreconf` phase.
@@ -211,20 +212,7 @@ class AutotoolsPackage(PackageBase):
         with working_dir(self.build_directory()):
             inspect.getmodule(self).make(*self.install_targets)
 
-    @PackageBase.sanity_check('build')
-    @PackageBase.on_package_attributes(run_tests=True)
-    def _run_default_function(self):
-        """This function is run after build if ``self.run_tests == True``
-
-        It will search for a method named :py:meth:`.check` and run it. A
-        sensible default is provided in the base class.
-        """
-        try:
-            fn = getattr(self, 'check')
-            tty.msg('Trying default sanity checks [check]')
-            fn()
-        except AttributeError:
-            tty.msg('Skipping default sanity checks [method `check` not implemented]')  # NOQA: ignore=E501
+    run_after('build')(PackageBase._run_default_build_time_test_callbacks)
 
     def check(self):
         """Searches the Makefile for targets ``test`` and ``check``
@@ -235,4 +223,4 @@ class AutotoolsPackage(PackageBase):
             self._if_make_target_execute('check')
 
     # Check that self.prefix is there after installation
-    PackageBase.sanity_check('install')(PackageBase.sanity_check_prefix)
+    run_after('install')(PackageBase.sanity_check_prefix)
diff --git a/lib/spack/spack/build_systems/cmake.py b/lib/spack/spack/build_systems/cmake.py
index a5e23e54f4..823ef502c4 100644
--- a/lib/spack/spack/build_systems/cmake.py
+++ b/lib/spack/spack/build_systems/cmake.py
@@ -26,11 +26,10 @@
 import inspect
 import platform
 
-import llnl.util.tty as tty
 import spack.build_environment
 from llnl.util.filesystem import working_dir, join_path
 from spack.directives import depends_on
-from spack.package import PackageBase
+from spack.package import PackageBase, run_after
 
 
 class CMakePackage(PackageBase):
@@ -72,6 +71,8 @@ class CMakePackage(PackageBase):
     build_targets = []
     install_targets = ['install']
 
+    build_time_test_callbacks = ['check']
+
     depends_on('cmake', type='build')
 
     def build_type(self):
@@ -155,20 +156,7 @@ class CMakePackage(PackageBase):
         with working_dir(self.build_directory()):
             inspect.getmodule(self).make(*self.install_targets)
 
-    @PackageBase.sanity_check('build')
-    @PackageBase.on_package_attributes(run_tests=True)
-    def _run_default_function(self):
-        """This function is run after build if ``self.run_tests == True``
-
-        It will search for a method named ``check`` and run it. A sensible
-        default is provided in the base class.
-        """
-        try:
-            fn = getattr(self, 'check')
-            tty.msg('Trying default build sanity checks [check]')
-            fn()
-        except AttributeError:
-            tty.msg('Skipping default build sanity checks [method `check` not implemented]')  # NOQA: ignore=E501
+    run_after('build')(PackageBase._run_default_build_time_test_callbacks)
 
     def check(self):
         """Searches the CMake-generated Makefile for the target ``test``
@@ -178,4 +166,4 @@ class CMakePackage(PackageBase):
             self._if_make_target_execute('test')
 
     # Check that self.prefix is there after installation
-    PackageBase.sanity_check('install')(PackageBase.sanity_check_prefix)
+    run_after('install')(PackageBase.sanity_check_prefix)
diff --git a/lib/spack/spack/build_systems/makefile.py b/lib/spack/spack/build_systems/makefile.py
index e8fa86264b..f07bcd62ab 100644
--- a/lib/spack/spack/build_systems/makefile.py
+++ b/lib/spack/spack/build_systems/makefile.py
@@ -27,7 +27,7 @@ import inspect
 
 import llnl.util.tty as tty
 from llnl.util.filesystem import working_dir
-from spack.package import PackageBase
+from spack.package import PackageBase, run_after
 
 
 class MakefilePackage(PackageBase):
@@ -100,4 +100,4 @@ class MakefilePackage(PackageBase):
             inspect.getmodule(self).make(*self.install_targets)
 
     # Check that self.prefix is there after installation
-    PackageBase.sanity_check('install')(PackageBase.sanity_check_prefix)
+    run_after('install')(PackageBase.sanity_check_prefix)
diff --git a/lib/spack/spack/build_systems/python.py b/lib/spack/spack/build_systems/python.py
index d21c291ae6..5e7c1c356d 100644
--- a/lib/spack/spack/build_systems/python.py
+++ b/lib/spack/spack/build_systems/python.py
@@ -26,7 +26,7 @@
 import inspect
 
 from spack.directives import extends
-from spack.package import PackageBase
+from spack.package import PackageBase, run_after
 
 from llnl.util.filesystem import working_dir
 
@@ -306,4 +306,4 @@ class PythonPackage(PackageBase):
         return []
 
     # Check that self.prefix is there after installation
-    PackageBase.sanity_check('install')(PackageBase.sanity_check_prefix)
+    run_after('install')(PackageBase.sanity_check_prefix)
diff --git a/lib/spack/spack/build_systems/r.py b/lib/spack/spack/build_systems/r.py
index a4f7359ec8..cde3dc9fdd 100644
--- a/lib/spack/spack/build_systems/r.py
+++ b/lib/spack/spack/build_systems/r.py
@@ -26,7 +26,7 @@
 import inspect
 
 from spack.directives import extends
-from spack.package import PackageBase
+from spack.package import PackageBase, run_after
 
 
 class RPackage(PackageBase):
@@ -55,4 +55,4 @@ class RPackage(PackageBase):
             self.stage.source_path)
 
     # Check that self.prefix is there after installation
-    PackageBase.sanity_check('install')(PackageBase.sanity_check_prefix)
+    run_after('install')(PackageBase.sanity_check_prefix)
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 24ff82fa38..0841ddcd61 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -78,13 +78,14 @@ class InstallPhase(object):
     search for execution. The method is retrieved at __get__ time, so that
     it can be overridden by subclasses of whatever class declared the phases.
 
-    It also provides hooks to execute prerequisite and sanity checks.
+    It also provides hooks to execute arbitrary callbacks before and after
+    the phase.
     """
 
     def __init__(self, name):
         self.name = name
-        self.preconditions = []
-        self.sanity_checks = []
+        self.run_before = []
+        self.run_after = []
 
     def __get__(self, instance, owner):
         # The caller is a class that is trying to customize
@@ -101,14 +102,13 @@ class InstallPhase(object):
             self._on_phase_start(instance)
             # Execute phase pre-conditions,
             # and give them the chance to fail
-            for check in self.preconditions:
-                # Do something sensible at some point
-                check(instance)
+            for callback in self.run_before:
+                callback(instance)
             phase(spec, prefix)
             # Execute phase sanity_checks,
             # and give them the chance to fail
-            for check in self.sanity_checks:
-                check(instance)
+            for callback in self.run_after:
+                callback(instance)
             # Check instance attributes at the end of a phase
             self._on_phase_exit(instance)
         return phase_wrapper
@@ -129,8 +129,8 @@ class InstallPhase(object):
             # This bug-fix was not back-ported in Python 2.6
             # http://bugs.python.org/issue1515
             other = InstallPhase(self.name)
-            other.preconditions.extend(self.preconditions)
-            other.sanity_checks.extend(self.sanity_checks)
+            other.run_before.extend(self.run_before)
+            other.run_after.extend(self.run_after)
             return other
 
 
@@ -142,22 +142,23 @@ class PackageMeta(spack.directives.DirectiveMetaMixin):
     """
     phase_fmt = '_InstallPhase_{0}'
 
-    _InstallPhase_sanity_checks = {}
-    _InstallPhase_preconditions = {}
+    _InstallPhase_run_before = {}
+    _InstallPhase_run_after = {}
+
+    def __new__(mcs, name, bases, attr_dict):
 
-    def __new__(meta, name, bases, attr_dict):
-        # Check if phases is in attr dict, then set
-        # install phases wrappers
         if 'phases' in attr_dict:
+            # Turn the strings in 'phases' into InstallPhase instances
+            # and add them as private attributes
             _InstallPhase_phases = [PackageMeta.phase_fmt.format(x) for x in attr_dict['phases']]  # NOQA: ignore=E501
             for phase_name, callback_name in zip(_InstallPhase_phases, attr_dict['phases']):  # NOQA: ignore=E501
                 attr_dict[phase_name] = InstallPhase(callback_name)
             attr_dict['_InstallPhase_phases'] = _InstallPhase_phases
 
-        def _append_checks(check_name):
+        def _flush_callbacks(check_name):
             # Name of the attribute I am going to check it exists
             attr_name = PackageMeta.phase_fmt.format(check_name)
-            checks = getattr(meta, attr_name)
+            checks = getattr(mcs, attr_name)
             if checks:
                 for phase_name, funcs in checks.items():
                     try:
@@ -180,57 +181,61 @@ class PackageMeta(spack.directives.DirectiveMetaMixin):
                             PackageMeta.phase_fmt.format(phase_name)]
                     getattr(phase, check_name).extend(funcs)
                 # Clear the attribute for the next class
-                setattr(meta, attr_name, {})
-
-        @classmethod
-        def _register_checks(cls, check_type, *args):
-            def _register_sanity_checks(func):
-                attr_name = PackageMeta.phase_fmt.format(check_type)
-                check_list = getattr(meta, attr_name)
-                for item in args:
-                    checks = check_list.setdefault(item, [])
-                    checks.append(func)
-                setattr(meta, attr_name, check_list)
-                return func
-            return _register_sanity_checks
-
-        @staticmethod
-        def on_package_attributes(**attrs):
-            def _execute_under_condition(func):
-                @functools.wraps(func)
-                def _wrapper(instance):
-                    # If all the attributes have the value we require, then
-                    # execute
-                    if all([getattr(instance, key, None) == value for key, value in attrs.items()]):  # NOQA: ignore=E501
-                        func(instance)
-                return _wrapper
-            return _execute_under_condition
-
-        @classmethod
-        def precondition(cls, *args):
-            return cls._register_checks('preconditions', *args)
-
-        @classmethod
-        def sanity_check(cls, *args):
-            return cls._register_checks('sanity_checks', *args)
-
-        if all([not hasattr(x, '_register_checks') for x in bases]):
-            attr_dict['_register_checks'] = _register_checks
-
-        if all([not hasattr(x, 'sanity_check') for x in bases]):
-            attr_dict['sanity_check'] = sanity_check
-
-        if all([not hasattr(x, 'precondition') for x in bases]):
-            attr_dict['precondition'] = precondition
-
-        if all([not hasattr(x, 'on_package_attributes') for x in bases]):
-            attr_dict['on_package_attributes'] = on_package_attributes
+                setattr(mcs, attr_name, {})
 
         # Preconditions
-        _append_checks('preconditions')
+        _flush_callbacks('run_before')
         # Sanity checks
-        _append_checks('sanity_checks')
-        return super(PackageMeta, meta).__new__(meta, name, bases, attr_dict)
+        _flush_callbacks('run_after')
+        return super(PackageMeta, mcs).__new__(mcs, name, bases, attr_dict)
+
+    @staticmethod
+    def register_callback(check_type, *phases):
+        def _decorator(func):
+            attr_name = PackageMeta.phase_fmt.format(check_type)
+            check_list = getattr(PackageMeta, attr_name)
+            for item in phases:
+                checks = check_list.setdefault(item, [])
+                checks.append(func)
+            setattr(PackageMeta, attr_name, check_list)
+            return func
+        return _decorator
+
+
+def run_before(*phases):
+    """Registers a method of a package to be run before a given phase"""
+    return PackageMeta.register_callback('run_before', *phases)
+
+
+def run_after(*phases):
+    """Registers a method of a package to be run after a given phase"""
+    return PackageMeta.register_callback('run_after', *phases)
+
+
+def on_package_attributes(**attr_dict):
+    """Executes the decorated method only if at the moment of calling
+    the instance has attributes that are equal to certain values.
+
+    :param dict attr_dict: dictionary mapping attribute names to their
+        required values
+    """
+    def _execute_under_condition(func):
+
+        @functools.wraps(func)
+        def _wrapper(instance, *args, **kwargs):
+            # If all the attributes have the value we require, then execute
+            has_all_attributes = all(
+                [hasattr(instance, key) for key in attr_dict]
+            )
+            if has_all_attributes:
+                has_the_right_values = all(
+                    [getattr(instance, key) == value for key, value in attr_dict.items()]  # NOQA: ignore=E501
+                )
+                if has_the_right_values:
+                    func(instance, *args, **kwargs)
+        return _wrapper
+
+    return _execute_under_condition
 
 
 class PackageBase(object):
@@ -1704,6 +1709,27 @@ class PackageBase(object):
         """
         return " ".join("-Wl,-rpath,%s" % p for p in self.rpath)
 
+    build_time_test_callbacks = None
+
+    @on_package_attributes(run_tests=True)
+    def _run_default_build_time_test_callbacks(self):
+        """Tries to call all the methods that are listed in the attribute
+        ``build_time_test_callbacks`` if ``self.run_tests is True``.
+
+        If ``build_time_test_callbacks is None`` returns immediately.
+        """
+        if self.build_time_test_callbacks is None:
+            return
+
+        for name in self.build_time_test_callbacks:
+            try:
+                fn = getattr(self, name)
+                tty.msg('RUN-TESTS: build-time tests [{0}]'.format(name))
+                fn()
+            except AttributeError:
+                msg = 'RUN-TESTS: method not implemented [{0}]'
+                tty.warn(msg.format(name))
+
 
 class Package(PackageBase):
     """General purpose class with a single ``install``
@@ -1716,7 +1742,7 @@ class Package(PackageBase):
     build_system_class = 'Package'
     # This will be used as a registration decorator in user
     # packages, if need be
-    PackageBase.sanity_check('install')(PackageBase.sanity_check_prefix)
+    run_after('install')(PackageBase.sanity_check_prefix)
 
 
 def install_dependency_symlinks(pkg, spec, prefix):
diff --git a/var/spack/repos/builtin.mock/packages/cmake-client/package.py b/var/spack/repos/builtin.mock/packages/cmake-client/package.py
index 51704e3f4b..e82d2cd781 100644
--- a/var/spack/repos/builtin.mock/packages/cmake-client/package.py
+++ b/var/spack/repos/builtin.mock/packages/cmake-client/package.py
@@ -22,9 +22,10 @@
 # License along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
-from spack import *
 import os
 
+from spack import *
+
 
 def check(condition, msg):
     """Raise an install error if condition is False."""
@@ -39,6 +40,28 @@ class CmakeClient(CMakePackage):
 
     version('1.0', '4cb3ff35b2472aae70f542116d616e63')
 
+    callback_counter = 0
+
+    flipped = False
+    run_this = True
+    check_this_is_None = None
+    did_something = False
+
+    @run_after('cmake')
+    @run_before('cmake', 'build', 'install')
+    def increment(self):
+        self.callback_counter += 1
+
+    @run_after('cmake')
+    @on_package_attributes(run_this=True, check_this_is_None=None)
+    def flip(self):
+        self.flipped = True
+
+    @run_after('cmake')
+    @on_package_attributes(does_not_exist=None)
+    def do_not_execute(self):
+        self.did_something = True
+
     def setup_environment(self, spack_env, run_env):
         spack_cc    # Ensure spack module-scope variable is avaiabl
         check(from_cmake == "from_cmake",
@@ -67,11 +90,15 @@ class CmakeClient(CMakePackage):
               "setup_dependent_package.")
 
     def cmake(self, spec, prefix):
-        pass
+        assert self.callback_counter == 1
 
-    build = cmake
+    def build(self, spec, prefix):
+        assert self.did_something is False
+        assert self.flipped is True
+        assert self.callback_counter == 3
 
     def install(self, spec, prefix):
+        assert self.callback_counter == 4
         # check that cmake is in the global scope.
         global cmake
         check(cmake is not None, "No cmake was in environment!")
diff --git a/var/spack/repos/builtin/packages/cmor/package.py b/var/spack/repos/builtin/packages/cmor/package.py
index b5debf9537..bb4f3b4df2 100644
--- a/var/spack/repos/builtin/packages/cmor/package.py
+++ b/var/spack/repos/builtin/packages/cmor/package.py
@@ -49,7 +49,7 @@ class Cmor(AutotoolsPackage):
     depends_on('python@:2.7', when='+python')
     depends_on('py-numpy', type=('build', 'run'), when='+python')
 
-    @AutotoolsPackage.precondition('configure')
+    @run_before('configure')
     def validate(self):
         if '+fortran' in self.spec and not self.compiler.fc:
             msg = 'cannot build a fortran variant without a fortran compiler'
diff --git a/var/spack/repos/builtin/packages/h5hut/package.py b/var/spack/repos/builtin/packages/h5hut/package.py
index 22146372dc..b12549df0d 100644
--- a/var/spack/repos/builtin/packages/h5hut/package.py
+++ b/var/spack/repos/builtin/packages/h5hut/package.py
@@ -47,7 +47,7 @@ class H5hut(AutotoolsPackage):
     # install: .libs/libH5hut.a: No such file or directory
     parallel = False
 
-    @AutotoolsPackage.precondition('configure')
+    @run_before('configure')
     def validate(self):
         """Checks if Fortran compiler is available."""
 
diff --git a/var/spack/repos/builtin/packages/hdf5/package.py b/var/spack/repos/builtin/packages/hdf5/package.py
index 222af53601..5e82818697 100644
--- a/var/spack/repos/builtin/packages/hdf5/package.py
+++ b/var/spack/repos/builtin/packages/hdf5/package.py
@@ -70,7 +70,7 @@ class Hdf5(AutotoolsPackage):
     depends_on('szip', when='+szip')
     depends_on('zlib@1.1.2:')
 
-    @AutotoolsPackage.precondition('configure')
+    @run_before('configure')
     def validate(self):
         """
         Checks if incompatible variants have been activated at the same time
@@ -170,7 +170,7 @@ class Hdf5(AutotoolsPackage):
                     arg for arg in m.group(1).split(' ') if arg != '-l'),
                 'libtool')
 
-    @AutotoolsPackage.sanity_check('install')
+    @run_after('install')
     def check_install(self):
         # Build and run a small program to test the installed HDF5 library
         spec = self.spec
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index dd864acbe3..4c34f3e3a0 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -86,7 +86,7 @@ class Mpich(AutotoolsPackage):
             join_path(self.prefix.lib, 'libmpi.{0}'.format(dso_suffix))
         ]
 
-    @AutotoolsPackage.precondition('autoreconf')
+    @run_before('autoreconf')
     def die_without_fortran(self):
         # Until we can pass variants such as +fortran through virtual
         # dependencies depends_on('mpi'), require Fortran compiler to
@@ -106,7 +106,7 @@ class Mpich(AutotoolsPackage):
             '--{0}-ibverbs'.format('with' if '+verbs' in spec else 'without')
         ]
 
-    @AutotoolsPackage.sanity_check('install')
+    @run_after('install')
     def filter_compilers(self):
         """Run after install to make the MPI compilers use the
         compilers that Spack built the package with.
diff --git a/var/spack/repos/builtin/packages/openblas/package.py b/var/spack/repos/builtin/packages/openblas/package.py
index d5a70f077b..327031b87e 100644
--- a/var/spack/repos/builtin/packages/openblas/package.py
+++ b/var/spack/repos/builtin/packages/openblas/package.py
@@ -68,7 +68,7 @@ class Openblas(MakefilePackage):
     def lapack_libs(self):
         return self.blas_libs
 
-    @MakefilePackage.precondition('edit')
+    @run_before('edit')
     def check_compilers(self):
         # As of 06/2016 there is no mechanism to specify that packages which
         # depends on Blas/Lapack need C or/and Fortran symbols. For now
@@ -126,7 +126,7 @@ class Openblas(MakefilePackage):
 
         return self.make_defs + targets
 
-    @MakefilePackage.sanity_check('build')
+    @run_after('build')
     def check_build(self):
         make('tests', *self.make_defs)
 
@@ -138,7 +138,7 @@ class Openblas(MakefilePackage):
         ]
         return make_args + self.make_defs
 
-    @MakefilePackage.sanity_check('install')
+    @run_after('install')
     def check_install(self):
         spec = self.spec
         # Openblas may pass its own test but still fail to compile Lapack
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index 2c6b6d68ac..bf844b7459 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -145,7 +145,7 @@ class Openmpi(AutotoolsPackage):
         elif self.spec.satisfies('@1.7:'):
             return 'verbs'
 
-    @AutotoolsPackage.precondition('autoreconf')
+    @run_before('autoreconf')
     def die_without_fortran(self):
         # Until we can pass variants such as +fortran through virtual
         # dependencies depends_on('mpi'), require Fortran compiler to
@@ -239,7 +239,7 @@ class Openmpi(AutotoolsPackage):
 
         return config_args
 
-    @AutotoolsPackage.sanity_check('install')
+    @run_after('install')
     def filter_compilers(self):
         """Run after install to make the MPI compilers use the
            compilers that Spack built the package with.
diff --git a/var/spack/repos/builtin/packages/py-basemap/package.py b/var/spack/repos/builtin/packages/py-basemap/package.py
index 4a35134e40..d2934eb3c7 100644
--- a/var/spack/repos/builtin/packages/py-basemap/package.py
+++ b/var/spack/repos/builtin/packages/py-basemap/package.py
@@ -44,7 +44,7 @@ class PyBasemap(PythonPackage):
     def setup_environment(self, spack_env, run_env):
         spack_env.set('GEOS_DIR', self.spec['geos'].prefix)
 
-    @PythonPackage.sanity_check('install')
+    @run_after('install')
     def post_install_patch(self):
         spec = self.spec
         # We are not sure if this fix is needed before Python 3.5.2.
diff --git a/var/spack/repos/builtin/packages/py-matplotlib/package.py b/var/spack/repos/builtin/packages/py-matplotlib/package.py
index d808b0fc4b..fc40dce9c1 100644
--- a/var/spack/repos/builtin/packages/py-matplotlib/package.py
+++ b/var/spack/repos/builtin/packages/py-matplotlib/package.py
@@ -95,7 +95,7 @@ class PyMatplotlib(PythonPackage):
     # depends_on('ttconv')
     depends_on('py-six@1.9.0:', type=('build', 'run'))
 
-    @PythonPackage.sanity_check('install')
+    @run_after('install')
     def set_backend(self):
         spec = self.spec
         prefix = self.prefix
diff --git a/var/spack/repos/builtin/packages/py-yt/package.py b/var/spack/repos/builtin/packages/py-yt/package.py
index 6ab967d8a5..4a61bfcec9 100644
--- a/var/spack/repos/builtin/packages/py-yt/package.py
+++ b/var/spack/repos/builtin/packages/py-yt/package.py
@@ -65,7 +65,7 @@ class PyYt(PythonPackage):
     depends_on("py-sympy", type=('build', 'run'))
     depends_on("python @2.7:2.999,3.4:")
 
-    @PythonPackage.sanity_check('install')
+    @run_after('install')
     def check_install(self):
         # The Python interpreter path can be too long for this
         # yt = Executable(join_path(prefix.bin, "yt"))
diff --git a/var/spack/repos/builtin/packages/r/package.py b/var/spack/repos/builtin/packages/r/package.py
index 30a6887a69..2babb250a5 100644
--- a/var/spack/repos/builtin/packages/r/package.py
+++ b/var/spack/repos/builtin/packages/r/package.py
@@ -113,7 +113,7 @@ class R(AutotoolsPackage):
 
         return config_args
 
-    @AutotoolsPackage.sanity_check('install')
+    @run_after('install')
     def copy_makeconf(self):
         # Make a copy of Makeconf because it will be needed to properly build R
         # dependencies in Spack.
@@ -121,7 +121,7 @@ class R(AutotoolsPackage):
         dst_makeconf = join_path(self.etcdir, 'Makeconf.spack')
         shutil.copy(src_makeconf, dst_makeconf)
 
-    @AutotoolsPackage.sanity_check('install')
+    @run_after('install')
     def filter_compilers(self):
         """Run after install to tell the configuration files and Makefiles
         to use the compilers that Spack built the package with.
diff --git a/var/spack/repos/builtin/packages/tcl/package.py b/var/spack/repos/builtin/packages/tcl/package.py
index d9b535305d..2ec8bb5236 100644
--- a/var/spack/repos/builtin/packages/tcl/package.py
+++ b/var/spack/repos/builtin/packages/tcl/package.py
@@ -55,7 +55,7 @@ class Tcl(AutotoolsPackage):
     def build_directory(self):
         return 'unix'
 
-    @AutotoolsPackage.sanity_check('install')
+    @run_after('install')
     def symlink_tclsh(self):
         with working_dir(self.prefix.bin):
             symlink('tclsh{0}'.format(self.version.up_to(2)), 'tclsh')
diff --git a/var/spack/repos/builtin/packages/trilinos/package.py b/var/spack/repos/builtin/packages/trilinos/package.py
index 5f8c2debb0..9add3b238c 100644
--- a/var/spack/repos/builtin/packages/trilinos/package.py
+++ b/var/spack/repos/builtin/packages/trilinos/package.py
@@ -379,7 +379,7 @@ class Trilinos(CMakePackage):
             ])
         return options
 
-    @CMakePackage.sanity_check('install')
+    @run_after('install')
     def filter_python(self):
         # When trilinos is built with Python, libpytrilinos is included
         # through cmake configure files. Namely, Trilinos_LIBRARIES in