From 3686c250ed61473a0de456d9c7aed98c3187724b Mon Sep 17 00:00:00 2001
From: becker33 <becker33@llnl.gov>
Date: Thu, 18 Jan 2018 17:55:44 -0800
Subject: [PATCH] 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              | 15 ++++++++++++++-
 lib/spack/spack/test/flag_handlers.py             | 14 +++++++++++++-
 .../repos/builtin/packages/parmetis/package.py    | 11 ++++++++---
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index dcc6e46ec6..b2a7e1f6d6 100644
--- a/lib/spack/spack/build_environment.py
+++ b/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 []
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 @@ 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()
diff --git a/var/spack/repos/builtin/packages/parmetis/package.py b/var/spack/repos/builtin/packages/parmetis/package.py
index dacd218f99..86fee57abb 100644
--- a/var/spack/repos/builtin/packages/parmetis/package.py
+++ b/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_C_FLAGS:STRING=%s' % (
-                '-c11' if '%pgi' in spec else ''),
+            '-DCMAKE_CXX_COMPILER:STRING=%s' % spec['mpi'].mpicxx
         ])
 
         if '+shared' in spec:
-- 
GitLab