Parmetis (CMakePackage) doesn't always respect cflags

[KineticTheory]

Summary

The current recipe for parmetis allows conflicting optimization levels to appear on the compile line in a way that isn't expected. For example: spack -d install parmetis%gcc cflags="-O3 -march=native" doesn't compile with the expected CFLAGS. I expect to see compile lines similar to this:

/usr/bin/gcc -O3 -march=native <include directives and defines> -o file.o -c file.c

Actual Result

/usr/bin/gcc -O3 -march=native <include directives and defines> -O2 -g -DNDEBUG -o file.o -c file.c

Because -O2 is listed after -O3 the optimization is not what I expect. I believe the -O2 -g -DNDEBUG flags are inserted by CMake as they are the default CMAKE_C_FLAGS_RELWITHDEBINFO values for gcc.

Possible solution

If SPACK_CFLAGS is set in the build environment, then zero-out the cmake defaulted CMAKE_C_FLAGS_XXX variables. #6839 demonstrates this model for metis and #6860 demonstrates this for parmetis, but I think it should be included in CMakePackage (possibly via the flag_handler function).

Related Issues

#6640
#6749
#6839
#6860

[KineticTheory]

As recommended in #6839, I added a flag_handler to the parmetis recipe:

@@ -53,6 +53,8 @@ class Parmetis(CMakePackage):
     # https://bitbucket.org/petsc/pkg-parmetis/commits/82409d68aa1d6cbc70740d0f35024aae17f7d5cb/raw/  # NOQA: E501
     patch('pkg-parmetis-82409d68aa1d6cbc70740d0f35024aae17f7d5cb.patch')
 
+    flag_handler = Package.build_system_flags
+
     def url_for_version(self, version):
         url = 'http://glaros.dtc.umn.edu/gkhome/fetch/sw/parmetis'
         if version < Version('3.2.0'):

But this doesn't work as expected:

% spack -d install parmetis cflags="-O0 -g" cxxflags="-O0 -g"

==> Installing parmetis
==> Error: TypeError: unbound method build_system_flags() must be called with Package instance as first argument (got str instance instead)
TypeError: TypeError: unbound method build_system_flags() must be called with Package instance as first argument (got str instance instead)

/scratch/kellyt/spack/lib/spack/spack/build_environment.py:683, in child_process:
     27    
     28                # build up some context from the offending package so we can
     29                # show that, too.
  >> 30                package_context = get_package_context(tb)
     31    
     32                build_log = None
     33                if hasattr(pkg, 'log_path'):


Traceback (most recent call last):
  File "/scratch/kellyt/spack/lib/spack/spack/build_environment.py", line 664, in child_process
    setup_package(pkg, dirty=dirty)
  File "/scratch/kellyt/spack/lib/spack/spack/build_environment.py", line 578, in setup_package
    set_compiler_environment_variables(pkg, spack_env)
  File "/scratch/kellyt/spack/lib/spack/spack/build_environment.py", line 168, in set_compiler_environment_variables
    injf, envf, bsf = pkg.flag_handler(flag, pkg.spec.compiler_flags[flag])
TypeError: unbound method build_system_flags() must be called with Package instance as first argument (got str instance instead)

It is very likely that my implementation is incorrect and I am looking for guidance to implement the flag_handler correctly.

cc: @becker33 @scheibelp

[scheibelp]

Hi @KineticTheory we'll be looking at this later today or on 1/16. Your usage looks like what we'd intend so we need to look at whether there are any issues with the flag handler implementation.

[becker33]

@scheibelp @KineticTheory there are two bugs here. One with Spack and one with the interaction between Spack and CMake. A fix for the first is up in #6960, the second will go up shortly.

[KineticTheory]

@becker33 Thanks for diving into this issue. I'll watch the related PR and issues and close this issue when it is resolved.

[becker33]

The other part of the fix is #6970. See issue #6961 for why it's a problem.

Once both are in, this should be resolved.