Fix type issues with setting flag handlers (#6960)

The flag_handlers method was being set as a bound method, but when
reset in the package.py file it was being set as an unbound method
(all python2 issues). This gets the underlying function information,
which is the same in either case.

The bug was uncovered for parmetis in #6858. This is a partial fix.
Included are changes to the parmetis package.py file to make use of
flag_handlers.

lib/spack/spack/build_environment.py

@@ -57,6 +57,7 @@
 import shutil
 import sys
 import traceback
+import types
 from six import iteritems
 from six import StringIO
 
@@ -165,7 +166,19 @@ def set_compiler_environment_variables(pkg, env):
     env_flags = {}
     build_system_flags = {}
     for flag in spack.spec.FlagMap.valid_compiler_flags():
-        injf, envf, bsf = pkg.flag_handler(flag, pkg.spec.compiler_flags[flag])
+        # Always convert flag_handler to function type.
+        # This avoids discrepencies in calling conventions between functions
+        # and methods, or between bound and unbound methods in python 2.
+        # We cannot effectively convert everything to a bound method, which
+        # would be the simpler solution.
+        if isinstance(pkg.flag_handler, types.FunctionType):
+            handler = pkg.flag_handler
+        else:
+            if sys.version_info >= (3, 0):
+                handler = pkg.flag_handler.__func__
+            else:
+                handler = pkg.flag_handler.im_func
+        injf, envf, bsf = handler(pkg, flag, pkg.spec.compiler_flags[flag])
         inject_flags[flag] = injf or []
         env_flags[flag] = envf or []
         build_system_flags[flag] = bsf or []

lib/spack/spack/test/flag_handlers.py

@@ -36,7 +36,7 @@ def temp_env():
     os.environ = old_env
 
 
-def add_O3_to_build_system_cflags(name, flags):
+def add_O3_to_build_system_cflags(pkg, name, flags):
     build_system_flags = []
     if name == 'cflags':
         build_system_flags.append('-O3')
@@ -61,6 +61,18 @@ def test_no_build_system_flags(self, temp_env):
         assert 'SPACK_CPPFLAGS' not in os.environ
         assert 'CPPFLAGS' not in os.environ
 
+    def test_unbound_method(self, temp_env):
+        # Other tests test flag_handlers set as bound methods and functions.
+        # This tests an unbound method in python2 (no change in python3).
+        s = spack.spec.Spec('mpileaks cppflags=-g')
+        s.concretize()
+        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
+
     def test_inject_flags(self, temp_env):
         s = spack.spec.Spec('mpileaks cppflags=-g')
         s.concretize()

var/spack/repos/builtin/packages/parmetis/package.py

@@ -53,6 +53,13 @@ class Parmetis(CMakePackage):
     # https://bitbucket.org/petsc/pkg-parmetis/commits/82409d68aa1d6cbc70740d0f35024aae17f7d5cb/raw/  # NOQA: E501
     patch('pkg-parmetis-82409d68aa1d6cbc70740d0f35024aae17f7d5cb.patch')
 
+    def flag_handler(self, name, flags):
+        if name == 'cflags':
+            if '%pgi' in self.spec:
+                my_flags = flags + ['-c11']
+                return (None, None, my_flags)
+        return (None, None, flags)
+
     def url_for_version(self, version):
         url = 'http://glaros.dtc.umn.edu/gkhome/fetch/sw/parmetis'
         if version < Version('3.2.0'):
@@ -68,9 +75,7 @@ def cmake_args(self):
             '-DGKLIB_PATH:PATH=%s/GKlib' % spec['metis'].prefix.include,
             '-DMETIS_PATH:PATH=%s' % spec['metis'].prefix,
             '-DCMAKE_C_COMPILER:STRING=%s' % spec['mpi'].mpicc,
-            '-DCMAKE_CXX_COMPILER:STRING=%s' % spec['mpi'].mpicxx,
+            '-DCMAKE_CXX_COMPILER:STRING=%s' % spec['mpi'].mpicxx
-            '-DCMAKE_C_FLAGS:STRING=%s' % (
-                '-c11' if '%pgi' in spec else ''),
         ])
 
         if '+shared' in spec: