From 9ddc98e46a73d53e9937cad1a430a34522d241de Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 17 Oct 2019 19:17:21 +0200 Subject: Separate setting build environment and run environment in packages (#11115) * Methods setting the environment now do it separately for build and run Before this commit the `*_environment` methods were setting modifications to both the build-time and run-time environment simultaneously. This might cause issues as the two environments inherently rely on different preconditions: 1. The build-time environment is set before building a package, thus the package prefix doesn't exist and can't be inspected 2. The run-time environment instead is set assuming the target package has been already installed Here we split each of these functions into two: one setting the build-time environment, one the run-time. We also adopt a fallback strategy that inspects for old methods and executes them as before, but prints a deprecation warning to tty. This permits to port packages to use the new methods in a distributed way, rather than having to modify all the packages at once. * Added a test that fails if any package uses the old API Marked the test xfail for now as we have a lot of packages in that state. * Added a test to check that a package modified by a PR is up to date This test can be used any time we deprecate a method call to ensure that during the first modification of the package we update also the deprecated calls. * Updated documentation --- lib/spack/docs/module_file_support.rst | 14 ++- lib/spack/docs/packaging_guide.rst | 71 +++++++------- lib/spack/docs/tutorial_advanced_packaging.rst | 79 ++++++++------- lib/spack/spack/build_environment.py | 64 ++++++++---- lib/spack/spack/cmd/flake8.py | 9 +- lib/spack/spack/modules/common.py | 17 +--- lib/spack/spack/package.py | 131 ++++++++++++++++--------- lib/spack/spack/test/package_sanity.py | 50 ++++++++++ 8 files changed, 275 insertions(+), 160 deletions(-) (limited to 'lib') diff --git a/lib/spack/docs/module_file_support.rst b/lib/spack/docs/module_file_support.rst index 7ce2398af2..1b5de3a0e8 100644 --- a/lib/spack/docs/module_file_support.rst +++ b/lib/spack/docs/module_file_support.rst @@ -303,8 +303,7 @@ content of the module files generated by Spack. The first one: .. code-block:: python - def setup_environment(self, spack_env, run_env): - """Set up the compile and runtime environments for a package.""" + def setup_run_environment(self, env): pass can alter the content of the module file associated with the same package where it is overridden. @@ -312,16 +311,15 @@ The second method: .. code-block:: python - def setup_dependent_environment(self, spack_env, run_env, dependent_spec): - """Set up the environment of packages that depend on this one""" + def setup_dependent_run_environment(self, env, dependent_spec): pass can instead inject run-time environment modifications in the module files of packages that depend on it. In both cases you need to fill ``run_env`` with the desired list of environment modifications. -.. note:: - The ``r`` package and callback APIs +.. admonition:: The ``r`` package and callback APIs + An example in which it is crucial to override both methods is given by the ``r`` package. This package installs libraries and headers in non-standard locations and it is possible to prepend the appropriate directory @@ -336,14 +334,14 @@ list of environment modifications. with the following snippet: .. literalinclude:: _spack_root/var/spack/repos/builtin/packages/r/package.py - :pyobject: R.setup_environment + :pyobject: R.setup_run_environment The ``r`` package also knows which environment variable should be modified to make language extensions provided by other packages available, and modifies it appropriately in the override of the second method: .. literalinclude:: _spack_root/var/spack/repos/builtin/packages/r/package.py - :pyobject: R.setup_dependent_environment + :pyobject: R.setup_dependent_run_environment .. _modules-yaml: diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index 5adb996232..767a744baa 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -2032,55 +2032,58 @@ appear in the package file (or in this case, in the list). .. _setup-dependent-environment: -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -``setup_dependent_environment()`` -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -Spack provides a mechanism for dependencies to provide variables that -can be used in their dependents' build. Any package can declare a -``setup_dependent_environment()`` function, and this function will be -called before the ``install()`` method of any dependent packages. -This allows dependencies to set up environment variables and other -properties to be used by dependents. - -The function declaration should look like this: +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Influence how dependents are built or run +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Spack provides a mechanism for dependencies to influence the +environment of their dependents by overriding the +:meth:`setup_dependent_run_environment ` +or the +:meth:`setup_dependent_build_environment ` +methods. +The Qt package, for instance, uses this call: .. literalinclude:: _spack_root/var/spack/repos/builtin/packages/qt/package.py - :pyobject: Qt.setup_dependent_environment + :pyobject: Qt.setup_dependent_build_environment :linenos: -Here, the Qt package sets the ``QTDIR`` environment variable so that -packages that depend on a particular Qt installation will find it. - -The arguments to this function are: +to set the ``QTDIR`` environment variable so that packages +that depend on a particular Qt installation will find it. +Another good example of how a dependency can influence +the build environment of dependents is the Python package: -* **spack_env**: List of environment modifications to be applied when - the dependent package is built within Spack. -* **run_env**: List of environment modifications to be applied when - the dependent package is run outside of Spack. These are added to the - resulting module file. -* **dependent_spec**: The spec of the dependent package about to be - built. This allows the extendee (self) to query the dependent's state. - Note that *this* package's spec is available as ``self.spec``. +.. literalinclude:: _spack_root/var/spack/repos/builtin/packages/python/package.py + :pyobject: Python.setup_dependent_build_environment + :linenos: -A good example of using these is in the Python package: +In the method above it is ensured that any package that depends on Python +will have the ``PYTHONPATH``, ``PYTHONHOME`` and ``PATH`` environment +variables set appropriately before starting the installation. To make things +even simpler the ``python setup.py`` command is also inserted into the module +scope of dependents by overriding a third method called +:meth:`setup_dependent_package ` +: .. literalinclude:: _spack_root/var/spack/repos/builtin/packages/python/package.py - :pyobject: Python.setup_dependent_environment + :pyobject: Python.setup_dependent_package :linenos: -The first thing that happens here is that the ``python`` command is -inserted into module scope of the dependent. This allows most python -packages to have a very simple install method, like this: +This allows most python packages to have a very simple install procedure, +like the following: .. code-block:: python def install(self, spec, prefix): - python('setup.py', 'install', '--prefix={0}'.format(prefix)) + setup_py('install', '--prefix={0}'.format(prefix)) + +Finally the Python package takes also care of the modifications to ``PYTHONPATH`` +to allow dependencies to run correctly: + +.. literalinclude:: _spack_root/var/spack/repos/builtin/packages/python/package.py + :pyobject: Python.setup_dependent_run_environment + :linenos: -Python's ``setup_dependent_environment`` method also sets up some -other variables, creates a directory, and sets up the ``PYTHONPATH`` -so that dependent packages can find their dependencies at build time. .. _packaging_conflicts: diff --git a/lib/spack/docs/tutorial_advanced_packaging.rst b/lib/spack/docs/tutorial_advanced_packaging.rst index bd90f724a7..876a6e8f70 100644 --- a/lib/spack/docs/tutorial_advanced_packaging.rst +++ b/lib/spack/docs/tutorial_advanced_packaging.rst @@ -90,21 +90,23 @@ like py-numpy, Spack's ``python`` package will add it to ``PYTHONPATH`` so it is available at build time; this is required because the default setup that spack does is not sufficient for python to import modules. -To provide environment setup for a dependent, a package can implement the -:py:func:`setup_dependent_environment ` -function. This function takes as a parameter a :py:class:`EnvironmentModifications ` +Any package can override the +:py:func:`setup_dependent_build_environment ` +method to setup the build environment for a dependent. +This method takes as an argument a :py:class:`EnvironmentModifications ` object which includes convenience methods to update the environment. For example, an MPI implementation can set ``MPICC`` for packages that depend on it: .. code-block:: python - def setup_dependent_environment(self, spack_env, run_env, dependent_spec): - spack_env.set('MPICC', join_path(self.prefix.bin, 'mpicc')) + def setup_dependent_build_environment(self, env, dependent_spec): + env.set('MPICC', join_path(self.prefix.bin, 'mpicc')) In this case packages that depend on ``mpi`` will have ``MPICC`` defined in -their environment when they build. This section is focused on modifying the -build-time environment represented by ``spack_env``, but it's worth noting that -modifications to ``run_env`` are included in Spack's automatically-generated +their environment when they build. This section is focused on setting up the +build-time environment but it's worth noting that a similar method called +:py:func:`setup_dependent_run_environment ` +can be used to code modifications that will be included in Spack's automatically-generated module files. We can practice by editing the ``mpich`` package to set the ``MPICC`` @@ -118,17 +120,17 @@ Once you're finished, the method should look like this: .. code-block:: python - def setup_dependent_environment(self, spack_env, run_env, dependent_spec): - spack_env.set('MPICC', join_path(self.prefix.bin, 'mpicc')) - spack_env.set('MPICXX', join_path(self.prefix.bin, 'mpic++')) - spack_env.set('MPIF77', join_path(self.prefix.bin, 'mpif77')) - spack_env.set('MPIF90', join_path(self.prefix.bin, 'mpif90')) + def setup_dependent_build_environment(self, env, dependent_spec): + env.set('MPICC', join_path(self.prefix.bin, 'mpicc')) + env.set('MPICXX', join_path(self.prefix.bin, 'mpic++')) + env.set('MPIF77', join_path(self.prefix.bin, 'mpif77')) + env.set('MPIF90', join_path(self.prefix.bin, 'mpif90')) - spack_env.set('MPICH_CC', spack_cc) - spack_env.set('MPICH_CXX', spack_cxx) - spack_env.set('MPICH_F77', spack_f77) - spack_env.set('MPICH_F90', spack_fc) - spack_env.set('MPICH_FC', spack_fc) + env.set('MPICH_CC', spack_cc) + env.set('MPICH_CXX', spack_cxx) + env.set('MPICH_F77', spack_f77) + env.set('MPICH_F90', spack_fc) + env.set('MPICH_FC', spack_fc) At this point we can, for instance, install ``netlib-scalapack`` with ``mpich``: @@ -155,25 +157,32 @@ set to the correct value. Set environment variables in your own package ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Packages can modify their own build-time environment by implementing the -:py:func:`setup_environment ` function. -For ``qt`` this looks like: +Packages can override the +:py:func:`setup_build_environment ` +or the +:py:func:`setup_run_environment ` +methods to modify their own build-time or run-time environment respectively. +An example of a package that overrides both methods is ``qt``: .. code-block:: python - def setup_environment(self, spack_env, run_env): - spack_env.set('MAKEFLAGS', '-j{0}'.format(make_jobs)) - run_env.set('QTDIR', self.prefix) + def setup_build_environment(self, env): + env.set('MAKEFLAGS', '-j{0}'.format(make_jobs)) -When ``qt`` builds, ``MAKEFLAGS`` will be defined in the environment. + def setup_run_environment(self, env): + env.set('QTDIR', self.prefix) -To contrast with ``qt``'s :py:func:`setup_dependent_environment ` +When ``qt`` builds, ``MAKEFLAGS`` will be defined in the environment. Likewise, when a +module file is created for ``qt`` it will contain commands to define ``QTDIR`` appropriately. + +To contrast with ``qt``'s +:py:func:`setup_dependent_build_environment ` function: .. code-block:: python - def setup_dependent_environment(self, spack_env, run_env, dependent_spec): - spack_env.set('QTDIR', self.prefix) + def setup_dependent_build_environment(self, env, dependent_spec): + env.set('QTDIR', self.prefix) Let's see how it works by completing the ``elpa`` package: @@ -185,16 +194,16 @@ In the end your method should look like: .. code-block:: python - def setup_environment(self, spack_env, run_env): + def setup_build_environment(self, env): spec = self.spec - spack_env.set('CC', spec['mpi'].mpicc) - spack_env.set('FC', spec['mpi'].mpifc) - spack_env.set('CXX', spec['mpi'].mpicxx) - spack_env.set('SCALAPACK_LDFLAGS', spec['scalapack'].libs.joined()) + env.set('CC', spec['mpi'].mpicc) + env.set('FC', spec['mpi'].mpifc) + env.set('CXX', spec['mpi'].mpicxx) + env.set('SCALAPACK_LDFLAGS', spec['scalapack'].libs.joined()) - spack_env.append_flags('LDFLAGS', spec['lapack'].libs.search_flags) - spack_env.append_flags('LIBS', spec['lapack'].libs.link_flags) + env.append_flags('LDFLAGS', spec['lapack'].libs.search_flags) + env.append_flags('LIBS', spec['lapack'].libs.link_flags) At this point it's possible to proceed with the installation of ``elpa ^mpich`` diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 3702a989c3..db99a7a4c1 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -673,35 +673,26 @@ def load_external_modules(pkg): def setup_package(pkg, dirty): """Execute all environment setup routines.""" - spack_env = EnvironmentModifications() - run_env = EnvironmentModifications() + build_env = EnvironmentModifications() if not dirty: clean_environment() - set_compiler_environment_variables(pkg, spack_env) - set_build_environment_variables(pkg, spack_env, dirty) - pkg.architecture.platform.setup_platform_environment(pkg, spack_env) + set_compiler_environment_variables(pkg, build_env) + set_build_environment_variables(pkg, build_env, dirty) + pkg.architecture.platform.setup_platform_environment(pkg, build_env) - # traverse in postorder so package can use vars from its dependencies - spec = pkg.spec - for dspec in pkg.spec.traverse(order='post', root=False, - deptype=('build', 'test')): - spkg = dspec.package - set_module_variables_for_package(spkg) - - # Allow dependencies to modify the module - dpkg = dspec.package - dpkg.setup_dependent_package(pkg.module, spec) - dpkg.setup_dependent_environment(spack_env, run_env, spec) + build_env.extend( + modifications_from_dependencies(pkg.spec, context='build') + ) - if (not dirty) and (not spack_env.is_unset('CPATH')): + if (not dirty) and (not build_env.is_unset('CPATH')): tty.debug("A dependency has updated CPATH, this may lead pkg-config" " to assume that the package is part of the system" " includes and omit it when invoked with '--cflags'.") set_module_variables_for_package(pkg) - pkg.setup_environment(spack_env, run_env) + pkg.setup_build_environment(build_env) # Loading modules, in particular if they are meant to be used outside # of Spack, can change environment variables that are relevant to the @@ -711,7 +702,7 @@ def setup_package(pkg, dirty): # unnecessary. Modules affecting these variables will be overwritten anyway with preserve_environment('CC', 'CXX', 'FC', 'F77'): # All module loads that otherwise would belong in previous - # functions have to occur after the spack_env object has its + # functions have to occur after the build_env object has its # modifications applied. Otherwise the environment modifications # could undo module changes, such as unsetting LD_LIBRARY_PATH # after a module changes it. @@ -727,8 +718,39 @@ def setup_package(pkg, dirty): load_external_modules(pkg) # Make sure nothing's strange about the Spack environment. - validate(spack_env, tty.warn) - spack_env.apply_modifications() + validate(build_env, tty.warn) + build_env.apply_modifications() + + +def modifications_from_dependencies(spec, context): + """Returns the environment modifications that are required by + the dependencies of a spec and also applies modifications + to this spec's package at module scope, if need be. + + Args: + spec (Spec): spec for which we want the modifications + context (str): either 'build' for build-time modifications or 'run' + for run-time modifications + """ + env = EnvironmentModifications() + pkg = spec.package + + # Maps the context to deptype and method to be called + deptype_and_method = { + 'build': (('build', 'link', 'test'), + 'setup_dependent_build_environment'), + 'run': (('link', 'run'), 'setup_dependent_run_environment') + } + deptype, method = deptype_and_method[context] + + for dspec in spec.traverse(order='post', root=False, deptype=deptype): + dpkg = dspec.package + set_module_variables_for_package(dpkg) + # Allow dependencies to modify the module + dpkg.setup_dependent_package(pkg.module, spec) + getattr(dpkg, method)(env, spec) + + return env def fork(pkg, function, dirty, fake): diff --git a/lib/spack/spack/cmd/flake8.py b/lib/spack/spack/cmd/flake8.py index ebcad88a06..61f7dd567f 100644 --- a/lib/spack/spack/cmd/flake8.py +++ b/lib/spack/spack/cmd/flake8.py @@ -93,12 +93,11 @@ pattern_exemptions = dict( for file_pattern, error_dict in pattern_exemptions.items()) -def changed_files(args): +def changed_files(base=None, untracked=True, all_files=False): """Get list of changed files in the Spack repository.""" git = which('git', required=True) - base = args.base if base is None: base = os.environ.get('TRAVIS_BRANCH', 'develop') @@ -114,11 +113,11 @@ def changed_files(args): ] # Add new files that are untracked - if args.untracked: + if untracked: git_args.append(['ls-files', '--exclude-standard', '--other']) # add everything if the user asked for it - if args.all: + if all_files: git_args.append(['ls-files', '--exclude-standard']) excludes = [os.path.realpath(f) for f in exclude_directories] @@ -246,7 +245,7 @@ def flake8(parser, args): with working_dir(spack.paths.prefix): if not file_list: - file_list = changed_files(args) + file_list = changed_files(args.base, args.untracked, args.all) print('=======================================================') print('flake8: running flake8 code checks on spack.') diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py index 116d4f5471..d38ede1385 100644 --- a/lib/spack/spack/modules/common.py +++ b/lib/spack/spack/modules/common.py @@ -634,23 +634,16 @@ class BaseContext(tengine.Context): exclude=spack.util.environment.is_system_path ) - # Modifications that are coded at package level - _ = spack.util.environment.EnvironmentModifications() - # TODO : the code down below is quite similar to - # TODO : build_environment.setup_package and needs to be factored out - # TODO : to a single place # Let the extendee/dependency modify their extensions/dependencies # before asking for package-specific modifications - for item in dependencies(self.spec, 'all'): - package = self.spec[item.name].package - build_environment.set_module_variables_for_package(package) - package.setup_dependent_package( - self.spec.package.module, self.spec + env.extend( + build_environment.modifications_from_dependencies( + self.spec, context='run' ) - package.setup_dependent_environment(_, env, self.spec) + ) # Package specific modifications build_environment.set_module_variables_for_package(self.spec.package) - self.spec.package.setup_environment(_, env) + self.spec.package.setup_run_environment(env) # Modifications required from modules.yaml env.extend(self.conf.env) diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index ecff265508..e3d4f1b6e9 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -45,6 +45,7 @@ import spack.mixins import spack.multimethod import spack.repo import spack.url +import spack.util.environment import spack.util.web import spack.multimethod import spack.binary_distribution as binary_distribution @@ -1951,68 +1952,108 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): """ return (None, None, flags) - def setup_environment(self, spack_env, run_env): - """Set up the compile and runtime environments for a package. + def _get_legacy_environment_method(self, method_name): + legacy_fn = getattr(self, method_name, None) + name_prefix = method_name.split('_environment')[0] + if legacy_fn: + msg = '[DEPRECATED METHOD]\n"{0}" ' \ + 'still defines the deprecated method "{1}" ' \ + '[should be split into "{2}_build_environment" and ' \ + '"{2}_run_environment"]' + tty.debug(msg.format(self.name, method_name, name_prefix)) + return legacy_fn - ``spack_env`` and ``run_env`` are ``EnvironmentModifications`` - objects. Package authors can call methods on them to alter - the environment within Spack and at runtime. + def setup_build_environment(self, env): + """Sets up the build environment for a package. - Both ``spack_env`` and ``run_env`` are applied within the build - process, before this package's ``install()`` method is called. + This method will be called before the current package prefix exists in + Spack's store. - Modifications in ``run_env`` will *also* be added to the - generated environment modules for this package. - - Default implementation does nothing, but this can be - overridden if the package needs a particular environment. - - Example: + Args: + env (EnvironmentModifications): environment modifications to be + applied when the package is built. Package authors can call + methods on it to alter the build environment. + """ + legacy_fn = self._get_legacy_environment_method('setup_environment') + if legacy_fn: + _ = spack.util.environment.EnvironmentModifications() + legacy_fn(env, _) - 1. Qt extensions need ``QTDIR`` set. + def setup_run_environment(self, env): + """Sets up the run environment for a package. Args: - spack_env (EnvironmentModifications): List of environment - modifications to be applied when this package is built - within Spack. - run_env (EnvironmentModifications): List of environment - modifications to be applied when this package is run outside - of Spack. These are added to the resulting module file. + env (EnvironmentModifications): environment modifications to be + applied when the package is run. Package authors can call + methods on it to alter the run environment. """ - pass + legacy_fn = self._get_legacy_environment_method('setup_environment') + if legacy_fn: + _ = spack.util.environment.EnvironmentModifications() + legacy_fn(_, env) - def setup_dependent_environment(self, spack_env, run_env, dependent_spec): - """Set up the environment of packages that depend on this one. + def setup_dependent_build_environment(self, env, dependent_spec): + """Sets up the build environment of packages that depend on this one. - This is similar to ``setup_environment``, but it is used to - modify the compile and runtime environments of packages that - *depend* on this one. This gives packages like Python and - others that follow the extension model a way to implement - common environment or compile-time settings for dependencies. + This is similar to ``setup_build_environment``, but it is used to + modify the build environments of packages that *depend* on this one. - This is useful if there are some common steps to installing - all extensions for a certain package. + This gives packages like Python and others that follow the extension + model a way to implement common environment or compile-time settings + for dependencies. - Example: + This method will be called before the dependent package prefix exists + in Spack's store. - 1. Installing python modules generally requires ``PYTHONPATH`` to point - to the ``lib/pythonX.Y/site-packages`` directory in the module's - install prefix. This method could be used to set that variable. + Examples: + 1. Installing python modules generally requires ``PYTHONPATH`` + to point to the ``lib/pythonX.Y/site-packages`` directory in the + module's install prefix. This method could be used to set that + variable. Args: - spack_env (EnvironmentModifications): List of environment - modifications to be applied when the dependent package is - built within Spack. - run_env (EnvironmentModifications): List of environment - modifications to be applied when the dependent package is - run outside of Spack. These are added to the resulting - module file. - dependent_spec (Spec): The spec of the dependent package + env (EnvironmentModifications): environment modifications to be + applied when the dependent package is built. Package authors + can call methods on it to alter the build environment. + + dependent_spec (Spec): the spec of the dependent package about to be built. This allows the extendee (self) to query the dependent's state. Note that *this* package's spec is - available as ``self.spec``. + available as ``self.spec`` """ - pass + legacy_fn = self._get_legacy_environment_method( + 'setup_dependent_environment' + ) + if legacy_fn: + _ = spack.util.environment.EnvironmentModifications() + legacy_fn(env, _, dependent_spec) + + def setup_dependent_run_environment(self, env, dependent_spec): + """Sets up the run environment of packages that depend on this one. + + This is similar to ``setup_run_environment``, but it is used to + modify the run environments of packages that *depend* on this one. + + This gives packages like Python and others that follow the extension + model a way to implement common environment or run-time settings + for dependencies. + + Args: + env (EnvironmentModifications): environment modifications to be + applied when the dependent package is run. Package authors + can call methods on it to alter the build environment. + + dependent_spec (Spec): The spec of the dependent package + about to be run. This allows the extendee (self) to query + the dependent's state. Note that *this* package's spec is + available as ``self.spec`` + """ + legacy_fn = self._get_legacy_environment_method( + 'setup_dependent_environment' + ) + if legacy_fn: + _ = spack.util.environment.EnvironmentModifications() + legacy_fn(_, env, dependent_spec) def setup_dependent_package(self, module, dependent_spec): """Set up Python module-scope variables for dependent packages. diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py index 1dd96dccec..f83a2dbb8e 100644 --- a/lib/spack/spack/test/package_sanity.py +++ b/lib/spack/spack/test/package_sanity.py @@ -4,6 +4,7 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) """This test does sanity checks on Spack's builtin package database.""" +import os.path import re import pytest @@ -11,6 +12,10 @@ import pytest import spack.fetch_strategy import spack.paths import spack.repo +import spack.util.executable as executable +# A few functions from this module are used to +# do sanity checks only on packagess modified by a PR +import spack.cmd.flake8 as flake8 import spack.util.crypto as crypto @@ -134,3 +139,48 @@ def test_all_packages_use_sha256_checksums(): ) assert [] == errors + + +@pytest.mark.xfail +def test_api_for_build_and_run_environment(): + """Ensure that every package uses the correct API to set build and + run environment, and not the old one. + """ + failing = [] + for pkg in spack.repo.path.all_packages(): + add_to_list = (hasattr(pkg, 'setup_environment') or + hasattr(pkg, 'setup_dependent_environment')) + if add_to_list: + failing.append(pkg) + + msg = ('there are {0} packages using the old API to set build ' + 'and run environment [{1}]') + assert not failing, msg.format( + len(failing), ','.join(x.name for x in failing) + ) + + +@pytest.mark.skipif( + not executable.which('git'), reason='requires git to be installed' +) +def test_prs_update_old_api(): + """Ensures that every package modified in a PR doesn't contain + deprecated calls to any method. + """ + changed_package_files = [ + x for x in flake8.changed_files() if flake8.is_package(x) + ] + failing = [] + for file in changed_package_files: + name = os.path.basename(os.path.dirname(file)) + pkg = spack.repo.get(name) + + # Check for old APIs + failed = (hasattr(pkg, 'setup_environment') or + hasattr(pkg, 'setup_dependent_environment')) + if failed: + failing.append(pkg) + msg = 'there are {0} packages still using old APIs in this PR [{1}]' + assert not failing, msg.format( + len(failing), ','.join(x.name for x in failing) + ) -- cgit v1.2.3-70-g09d2