From ca0d9ae7f024806a93295af19ec37aa491cdaac0 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Tue, 9 Oct 2018 15:18:31 -0700 Subject: Make builtin flag handlers available in package scope (#8668) * Push default flag handlers into module scope * Preserve backwards compatibility of builtin flag handler names Ensure Spack continues to work for packages using the `Package.env_flags` idiom and equivalent. * update docs and tests to match * Update packages to match new syntax --- lib/spack/docs/packaging_guide.rst | 13 +++--- lib/spack/spack/package.py | 52 +++++++++++++---------- lib/spack/spack/pkgkit.py | 1 + lib/spack/spack/test/flag_handlers.py | 17 ++++---- var/spack/repos/builtin/packages/hiop/package.py | 2 +- var/spack/repos/builtin/packages/ipopt/package.py | 2 +- 6 files changed, 47 insertions(+), 40 deletions(-) diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index 683bb01503..a9c8c13710 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -2689,13 +2689,13 @@ Here are the definitions of the three built-in flag handlers: .. code-block:: python - def inject_flags(self, name, flags): + def inject_flags(pkg, name, flags): return (flags, None, None) - def env_flags(self, name, flags): + def env_flags(pkg, name, flags): return (None, flags, None) - def build_system_flags(self, name, flags): + def build_system_flags(pkg, name, flags): return (None, None, flags) .. note:: @@ -2708,10 +2708,7 @@ the built-in flag handlers, .. code-block:: python - flag_handler = .env_flags - -where ```` can be any of the subclasses of PackageBase -discussed in :ref:`installation_procedure`, + flag_handler = env_flags or by implementing the flag_handler method. Suppose for a package ``Foo`` we need to pass ``cflags``, ``cxxflags``, and ``cppflags`` @@ -2737,7 +2734,7 @@ method of the ``EnvironmentModifications`` class to append values to a list of flags whenever the flag handler is ``env_flags``. If the package passes flags through the environment or the build system manually (in the install method, for example), we recommend using the -default flag handler, or removind manual references and implementing a +default flag handler, or removing manual references and implementing a custom flag handler method that adds the desired flags to export as environment variables or pass to the build system. Manual flag passing is likely to interfere with the ``env_flags`` and diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index a614e108e4..6aa07ed0b9 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -1738,6 +1738,31 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): return __import__(self.__class__.__module__, fromlist=[self.__class__.__name__]) + @classmethod + def inject_flags(cls, name, flags): + """ + flag_handler that injects all flags through the compiler wrapper. + """ + return (flags, None, None) + + @classmethod + def env_flags(cls, name, flags): + """ + flag_handler that adds all flags to canonical environment variables. + """ + return (None, flags, None) + + @classmethod + def build_system_flags(cls, name, flags): + """ + flag_handler that passes flags to the build system arguments. Any + package using `build_system_flags` must also implement + `flags_to_build_system_args`, or derive from a class that + implements it. Currently, AutotoolsPackage and CMakePackage + implement it. + """ + return (None, None, flags) + def setup_environment(self, spack_env, run_env): """Set up the compile and runtime environments for a package. @@ -1838,28 +1863,6 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): """ pass - def inject_flags(self, name, flags): - """ - flag_handler that injects all flags through the compiler wrapper. - """ - return (flags, None, None) - - def env_flags(self, name, flags): - """ - flag_handler that adds all flags to canonical environment variables. - """ - return (None, flags, None) - - def build_system_flags(self, name, flags): - """ - flag_handler that passes flags to the build system arguments. Any - package using `build_system_flags` must also implement - `flags_to_build_system_args`, or derive from a class that - implements it. Currently, AutotoolsPackage and CMakePackage - implement it. - """ - return (None, None, flags) - flag_handler = inject_flags # The flag handler method is called for each of the allowed compiler flags. # It returns a triple of inject_flags, env_flags, build_system_flags. @@ -2196,6 +2199,11 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): tty.warn(msg.format(name)) +inject_flags = PackageBase.inject_flags +env_flags = PackageBase.env_flags +build_system_flags = PackageBase.build_system_flags + + class Package(PackageBase): """General purpose class with a single ``install`` phase that needs to be coded by packagers. diff --git a/lib/spack/spack/pkgkit.py b/lib/spack/spack/pkgkit.py index bb688d1acd..a9251335af 100644 --- a/lib/spack/spack/pkgkit.py +++ b/lib/spack/spack/pkgkit.py @@ -31,6 +31,7 @@ import llnl.util.filesystem from llnl.util.filesystem import * from spack.package import Package, run_before, run_after, on_package_attributes +from spack.package import inject_flags, env_flags, build_system_flags from spack.build_systems.makefile import MakefilePackage from spack.build_systems.aspell_dict import AspellDictPackage from spack.build_systems.autotools import AutotoolsPackage diff --git a/lib/spack/spack/test/flag_handlers.py b/lib/spack/spack/test/flag_handlers.py index 5366b60f11..441f3fcbae 100644 --- a/lib/spack/spack/test/flag_handlers.py +++ b/lib/spack/spack/test/flag_handlers.py @@ -29,6 +29,8 @@ import spack.spec import spack.repo import spack.build_environment +from spack.pkgkit import inject_flags, env_flags, build_system_flags + @pytest.fixture() def temp_env(): @@ -70,7 +72,6 @@ class TestFlagHandlers(object): pkg = spack.repo.get(s) pkg.flag_handler = pkg.__class__.inject_flags spack.build_environment.setup_package(pkg, False) - assert os.environ['SPACK_CPPFLAGS'] == '-g' assert 'CPPFLAGS' not in os.environ @@ -78,7 +79,7 @@ class TestFlagHandlers(object): s = spack.spec.Spec('mpileaks cppflags=-g') s.concretize() pkg = spack.repo.get(s) - pkg.flag_handler = pkg.inject_flags + pkg.flag_handler = inject_flags spack.build_environment.setup_package(pkg, False) assert os.environ['SPACK_CPPFLAGS'] == '-g' @@ -88,7 +89,7 @@ class TestFlagHandlers(object): s = spack.spec.Spec('mpileaks cppflags=-g') s.concretize() pkg = spack.repo.get(s) - pkg.flag_handler = pkg.env_flags + pkg.flag_handler = env_flags spack.build_environment.setup_package(pkg, False) assert os.environ['CPPFLAGS'] == '-g' @@ -98,7 +99,7 @@ class TestFlagHandlers(object): s = spack.spec.Spec('cmake-client cppflags=-g') s.concretize() pkg = spack.repo.get(s) - pkg.flag_handler = pkg.build_system_flags + pkg.flag_handler = build_system_flags spack.build_environment.setup_package(pkg, False) assert 'SPACK_CPPFLAGS' not in os.environ @@ -112,7 +113,7 @@ class TestFlagHandlers(object): s = spack.spec.Spec('patchelf cppflags=-g') s.concretize() pkg = spack.repo.get(s) - pkg.flag_handler = pkg.build_system_flags + pkg.flag_handler = build_system_flags spack.build_environment.setup_package(pkg, False) assert 'SPACK_CPPFLAGS' not in os.environ @@ -124,7 +125,7 @@ class TestFlagHandlers(object): s = spack.spec.Spec('mpileaks cppflags=-g') s.concretize() pkg = spack.repo.get(s) - pkg.flag_handler = pkg.build_system_flags + pkg.flag_handler = build_system_flags # Test the command line flags method raises a NotImplementedError try: @@ -161,7 +162,7 @@ class TestFlagHandlers(object): s = spack.spec.Spec('cmake-client ldflags=-mthreads') s.concretize() pkg = spack.repo.get(s) - pkg.flag_handler = pkg.build_system_flags + pkg.flag_handler = build_system_flags spack.build_environment.setup_package(pkg, False) assert 'SPACK_LDFLAGS' not in os.environ @@ -177,7 +178,7 @@ class TestFlagHandlers(object): s = spack.spec.Spec('cmake-client ldlibs=-lfoo') s.concretize() pkg = spack.repo.get(s) - pkg.flag_handler = pkg.build_system_flags + pkg.flag_handler = build_system_flags spack.build_environment.setup_package(pkg, False) assert 'SPACK_LDLIBS' not in os.environ diff --git a/var/spack/repos/builtin/packages/hiop/package.py b/var/spack/repos/builtin/packages/hiop/package.py index 2720973271..3cb55747fc 100644 --- a/var/spack/repos/builtin/packages/hiop/package.py +++ b/var/spack/repos/builtin/packages/hiop/package.py @@ -48,7 +48,7 @@ class Hiop(CMakePackage): depends_on('lapack') depends_on('blas') - flag_handler = CMakePackage.build_system_flags + flag_handler = build_system_flags def cmake_args(self): args = [] diff --git a/var/spack/repos/builtin/packages/ipopt/package.py b/var/spack/repos/builtin/packages/ipopt/package.py index 33a818bd89..200992e82a 100644 --- a/var/spack/repos/builtin/packages/ipopt/package.py +++ b/var/spack/repos/builtin/packages/ipopt/package.py @@ -59,7 +59,7 @@ class Ipopt(AutotoolsPackage): patch('ipopt_ppc_build.patch', when='arch=ppc64le') - flag_handler = AutotoolsPackage.build_system_flags + flag_handler = build_system_flags build_directory = 'spack-build' # IPOPT does not build correctly in parallel on OS X -- cgit v1.2.3-70-g09d2