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
This commit is contained in:
Massimiliano Culpo 2017-01-25 16:57:01 +01:00 committed by Todd Gamblin
parent 90d47a3ead
commit fc866ae0fe
21 changed files with 174 additions and 135 deletions

View File

@ -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::

View File

@ -156,14 +156,24 @@
#-----------------------------------------------------------------------------
__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']

View File

@ -30,9 +30,8 @@
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 @@ def autoreconf(self, spec, prefix):
"""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 @@ def install(self, spec, prefix):
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 @@ def check(self):
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)

View File

@ -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 @@ def install(self, spec, prefix):
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 @@ def check(self):
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)

View File

@ -27,7 +27,7 @@
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 @@ def install(self, spec, prefix):
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)

View File

@ -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 @@ def check_args(self, spec, prefix):
return []
# Check that self.prefix is there after installation
PackageBase.sanity_check('install')(PackageBase.sanity_check_prefix)
run_after('install')(PackageBase.sanity_check_prefix)

View File

@ -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 @@ def install(self, spec, prefix):
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)

View File

@ -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 @@ def phase_wrapper(spec, prefix):
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 @@ def copy(self):
# 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 @@ def _append_checks(check_name):
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 @@ def rpath_args(self):
"""
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):

View File

@ -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 @@ def setup_dependent_package(self, module, dspec):
"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!")

View File

@ -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'

View File

@ -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."""

View File

@ -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 @@ def configure(self, spec, prefix):
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

View File

@ -86,7 +86,7 @@ def setup_dependent_package(self, module, dep_spec):
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 @@ def configure_args(self):
'--{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.

View File

@ -68,7 +68,7 @@ def blas_libs(self):
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 @@ def build_targets(self):
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 @@ def install_targets(self):
]
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

View File

@ -145,7 +145,7 @@ def verbs(self):
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 @@ def configure_args(self):
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.

View File

@ -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.

View File

@ -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

View File

@ -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"))

View File

@ -113,7 +113,7 @@ def configure_args(self):
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 @@ def copy_makeconf(self):
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.

View File

@ -55,7 +55,7 @@ def setup_environment(self, spack_env, env):
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')

View File

@ -379,7 +379,7 @@ def cmake_args(self):
])
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