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.

3 files changed, 35 insertions, 5 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index cab455695e..dfadabb10c 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -57,6 +57,7 @@ import os
 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 []
diff --git a/lib/spack/spack/test/flag_handlers.py b/lib/spack/spack/test/flag_handlers.py
index 0b785a948f..80dad9c35e 100644
--- a/lib/spack/spack/test/flag_handlers.py
+++ b/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 @@ class TestFlagHandlers(object):
         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()
diff --git a/var/spack/repos/builtin/packages/parmetis/package.py b/var/spack/repos/builtin/packages/parmetis/package.py
index 927a7715e6..565c06686c 100644
--- a/var/spack/repos/builtin/packages/parmetis/package.py
+++ b/var/spack/repos/builtin/packages/parmetis/package.py
@@ -54,6 +54,13 @@ class Parmetis(Package):
     # 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'):
@@ -70,9 +77,7 @@ class Parmetis(Package):
             '-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_C_FLAGS:STRING=%s' % (
-                '-c11' if '%pgi' in spec else ''),
+            '-DCMAKE_CXX_COMPILER:STRING=%s' % spec['mpi'].mpicxx
         ])
 
         if '+shared' in spec: