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

13 files changed, 48 insertions, 21 deletions

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