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

@@ -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 = <PackageClass>.env_flags
+   flag_handler = env_flags
-
-where ``<PackageClass>`` can be any of the subclasses of PackageBase
-discussed in :ref:`installation_procedure`,
 
 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

lib/spack/spack/package.py

@@ -1738,6 +1738,31 @@ def module(self):
         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 @@ def setup_dependent_package(self, module, dependent_spec):
         """
         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 @@ def _run_default_install_time_test_callbacks(self):
                 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.

lib/spack/spack/pkgkit.py

@@ -31,6 +31,7 @@
 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

lib/spack/spack/test/flag_handlers.py

@@ -29,6 +29,8 @@
 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 @@ def test_unbound_method(self, temp_env):
         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 @@ def test_inject_flags(self, temp_env):
         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 @@ def test_env_flags(self, temp_env):
         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 @@ def test_build_system_flags_cmake(self, temp_env):
         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 @@ def test_build_system_flags_autotools(self, temp_env):
         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 @@ def test_build_system_flags_not_implemented(self, temp_env):
         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 @@ def test_ld_flags_cmake(self, temp_env):
         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 @@ def test_ld_libs_cmake(self, temp_env):
         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

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 = []

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