Allow packages to control handling of compiler flags (#4421)

* Initial work on flag trapping using functions called <flag>_handler and default_flag_handler

* Update packages so they do not obliterate flags

* Added append to EnvironmentModifications class

* changed EnvironmentModifications to have append_flags method

* changed flag_val to be a tuple

* Increased test coverage

* added documentation of flag handling
This commit is contained in:
becker33 2017-07-19 20:12:00 -07:00 committed by GitHub
parent acca75b368
commit f962aba6ce
16 changed files with 203 additions and 12 deletions

View File

@ -2390,6 +2390,94 @@ build system.
Compiler flags
^^^^^^^^^^^^^^
Compiler flags set by the user through the Spec object can be passed to
the build in one of two ways. For packages inheriting from the
``CmakePackage`` or ``AutotoolsPackage`` classes, the build environment
passes those flags to the relevant environment variables (``CFLAGS``,
``CXXFLAGS``, etc) that are respected by the build system. For all other
packages, the default behavior is to inject the flags directly into the
compiler commands using Spack's compiler wrappers.
Individual packages can override the default behavior for the flag
handling. Packages can define a ``default_flag_handler`` method that
applies to all sets of flags handled by Spack, or may define
individual methods ``cflags_handler``, ``cxxflags_handler``,
etc. Spack will apply the individual method for a flag set if it
exists, otherwise the ``default_flag_handler`` method if it exists,
and fall back on the default for that package class if neither exists.
These methods are defined on the package class, and take two
parameters in addition to the packages itself. The ``env`` parameter
is an ``EnvironmentModifications`` object that can be used to change
the build environment. The ``flag_val`` parameter is a tuple. Its
first entry is the name of the flag (``cflags``, ``cxxflags``, etc.)
and its second entry is a list of the values for that flag.
There are three primary idioms that can be combined to create whatever
behavior the package requires.
1. The default behavior for packages inheriting from
``AutotoolsPackage`` or ``CmakePackage``.
.. code-block:: python
def default_flag_handler(self, env, flag_val):
env.append_flags(flag_val[0].upper(), ' '.join(flag_val[1]))
return []
2. The default behavior for other packages
.. code-block:: python
def default_flag_handler(self, env, flag_val):
return flag_val[1]
3. Packages may have additional flags to add to the build. These flags
can be added to either idiom above. For example:
.. code-block:: python
def default_flag_handler(self, env, flag_val):
flags = flag_val[1]
flags.append('-flag')
return flags
or
.. code-block:: python
def default_flag_handler(self, env, flag_val):
env.append_flags(flag_val[0].upper(), ' '.join(flag_val[1]))
env.append_flags(flag_val[0].upper(), '-flag')
return []
Packages may also opt for methods that include aspects of any of the
idioms above. E.g.
.. code-block:: python
def default_flag_handler(self, env, flag_val):
flags = []
if len(flag_val[1]) > 3:
env.append_flags(flag_val[0].upper(), ' '.join(flag_val[1][3:]))
flags = flag_val[1][:3]
else:
flags = flag_val[1]
flags.append('-flag')
return flags
Because these methods can pass values through environment variables,
it is important not to override these variables unnecessarily in other
package methods. In the ``setup_environment`` and
``setup_dependent_environment`` methods, use the ``append_flags``
method of the ``EnvironmentModifications`` class to append values to a
list of flags whenever there is no good reason to override the
existing value. In the ``install`` method and other methods that can
operate on the build environment directly through the ``env``
variable, test for environment variable existance before overriding
values to add compiler flags.
In rare circumstances such as compiling and running small unit tests, a
package developer may need to know what are the appropriate compiler
flags to enable features like ``OpenMP``, ``c++11``, ``c++14`` and

View File

@ -467,6 +467,19 @@ def setup_package(pkg, dirty=False):
for s in pkg.spec.traverse():
s.package.spec = s
# Trap spack-tracked compiler flags as appropriate.
# Must be before set_compiler_environment_variables
# Current implementation of default flag handler relies on this being
# the first thing to affect the spack_env (so there is no appending), or
# on no other build_environment methods trying to affect these variables
# (CFLAGS, CXXFLAGS, etc). Currently both are true, either is sufficient.
for flag in spack.spec.FlagMap.valid_compiler_flags():
trap_func = getattr(pkg, flag + '_handler',
getattr(pkg, 'default_flag_handler',
lambda x, y: y[1]))
flag_val = pkg.spec.compiler_flags[flag]
pkg.spec.compiler_flags[flag] = trap_func(spack_env, (flag, flag_val))
set_compiler_environment_variables(pkg, spack_env)
set_build_environment_variables(pkg, spack_env, dirty)
pkg.architecture.platform.setup_platform_environment(pkg, spack_env)

View File

@ -181,6 +181,29 @@ def build_directory(self):
"""Override to provide another place to build the package"""
return self.configure_directory
def default_flag_handler(self, spack_env, flag_val):
# Relies on being the first thing that can affect the spack_env
# EnvironmentModification after it is instantiated or no other
# method trying to affect these variables. Currently both are true
# flag_val is a tuple (flag, value_list).
spack_env.set(flag_val[0].upper(),
' '.join(flag_val[1]))
return []
def patch(self):
"""Patches config.guess if
:py:attr:``~.AutotoolsPackage.patch_config_guess`` is True
:raise RuntimeError: if something goes wrong when patching
``config.guess``
"""
if self.patch_config_guess and self.spec.satisfies(
'arch=linux-rhel7-ppc64le'
):
if not self._do_patch_config_guess():
raise RuntimeError('Failed to find suitable config.guess')
@run_before('autoreconf')
def delete_configure_to_force_update(self):
if self.force_autoreconf:

View File

@ -132,6 +132,15 @@ def build_directory(self):
"""
return join_path(self.stage.source_path, 'spack-build')
def default_flag_handler(self, spack_env, flag_val):
# Relies on being the first thing that can affect the spack_env
# EnvironmentModification after it is instantiated or no other
# method trying to affect these variables. Currently both are true
# flag_val is a tuple (flag, value_list)
spack_env.set(flag_val[0].upper(),
' '.join(flag_val[1]))
return []
def cmake_args(self):
"""Produces a list containing all the arguments that must be passed to
cmake, except:

View File

@ -63,6 +63,15 @@ def execute(self):
os.environ[self.name] = str(self.value)
class AppendFlagsEnv(NameValueModifier):
def execute(self):
if self.name in os.environ and os.environ[self.name]:
os.environ[self.name] += self.separator + str(self.value)
else:
os.environ[self.name] = str(self.value)
class UnsetEnv(NameModifier):
def execute(self):
@ -171,6 +180,20 @@ def set(self, name, value, **kwargs):
item = SetEnv(name, value, **kwargs)
self.env_modifications.append(item)
def append_flags(self, name, value, sep=' ', **kwargs):
"""
Stores in the current object a request to append to an env variable
Args:
name: name of the environment variable to be appended to
value: value to append to the environment variable
Appends with spaces separating different additions to the variable
"""
kwargs.update(self._get_outside_caller_attributes())
kwargs.update({'separator': sep})
item = AppendFlagsEnv(name, value, **kwargs)
self.env_modifications.append(item)
def unset(self, name, **kwargs):
"""
Stores in the current object a request to unset an environment variable

View File

@ -110,6 +110,19 @@ def test_set(env):
assert str(3) == os.environ['B']
def test_append_flags(env):
"""Tests appending to a value in the environment."""
# Store a couple of commands
env.append_flags('APPEND_TO_ME', 'flag1')
env.append_flags('APPEND_TO_ME', 'flag2')
# ... execute the commands
env.apply_modifications()
assert 'flag1 flag2' == os.environ['APPEND_TO_ME']
def test_unset(env):
"""Tests unsetting values in the environment."""

View File

@ -78,12 +78,18 @@ def cmake_args(self):
cmake_args = []
if '+cxx11' in spec:
env['CXXFLAGS'] = self.compiler.cxx11_flag
if 'CXXFLAGS' in env and env['CXXFLAGS']:
env['CXXFLAGS'] += ' ' + self.compiler.cxx11_flag
else:
env['CXXFLAGS'] = self.compiler.cxx11_flag
cmake_args.append('-DCLHEP_BUILD_CXXSTD=' +
self.compiler.cxx11_flag)
if '+cxx14' in spec:
env['CXXFLAGS'] = self.compiler.cxx14_flag
if 'CXXFLAGS' in env and env['CXXFLAGS']:
env['CXXFLAGS'] += ' ' + self.compiler.cxx14_flag
else:
env['CXXFLAGS'] = self.compiler.cxx14_flag
cmake_args.append('-DCLHEP_BUILD_CXXSTD=' +
self.compiler.cxx14_flag)

View File

@ -71,8 +71,8 @@ def setup_environment(self, spack_env, run_env):
spack_env.set('FC', spec['mpi'].mpifc)
spack_env.set('CXX', spec['mpi'].mpicxx)
spack_env.set('LDFLAGS', spec['lapack'].libs.search_flags)
spack_env.set('LIBS', spec['lapack'].libs.link_flags)
spack_env.append_flags('LDFLAGS', spec['lapack'].libs.search_flags)
spack_env.append_flags('LIBS', spec['lapack'].libs.link_flags)
spack_env.set('SCALAPACK_LDFLAGS', spec['scalapack'].libs.joined())
def configure_args(self):

View File

@ -98,7 +98,12 @@ def install(self, spec, prefix):
ln('-sf',
libz_prefix + '/lib',
libz_prefix + '/lib64')
os.environ['LDFLAGS'] = '-lquadmath'
if 'LDFLAGS' in env and env['LDFLAGS']:
env['LDFLAGS'] += ' ' + '-lquadmath'
else:
env['LDFLAGS'] = '-lquadmath'
with working_dir('FERRET', create=False):
os.environ['LD_X11'] = '-L/usr/lib/X11 -lX11'
os.environ['HOSTTYPE'] = 'x86_64-linux'

View File

@ -155,7 +155,7 @@ class Git(AutotoolsPackage):
depends_on('m4', type='build')
def setup_environment(self, spack_env, run_env):
spack_env.set('LDFLAGS', '-L{0} -lintl'.format(
spack_env.append_flags('LDFLAGS', '-L{0} -lintl'.format(
self.spec['gettext'].prefix.lib))
def configure_args(self):

View File

@ -85,6 +85,7 @@ def setup_environment(self, build_env, run_env):
def configure_args(self):
config_args = ['--enable-shared']
optflags = self.optflags
# Optimization flag names have changed in libint 2

View File

@ -71,8 +71,15 @@ def install(self, spec, prefix):
if which('xiar'):
env['AR'] = 'xiar'
env['CFLAGS'] = optflags
env['FCFLAGS'] = optflags
if 'CFLAGS' in env and env['CFLAGS']:
env['CFLAGS'] += ' ' + optflags
else:
env['CFLAGS'] = optflags
if 'FCFLAGS' in env and env['FCFLAGS']:
env['FCFLAGS'] += ' ' + optflags
else:
env['FCFLAGS'] = optflags
configure('--prefix={0}'.format(prefix),
'--enable-shared')

View File

@ -46,5 +46,5 @@ class Libxpm(AutotoolsPackage):
depends_on('util-macros', type='build')
def setup_environment(self, spack_env, run_env):
spack_env.set('LDFLAGS', '-L{0} -lintl'.format(
spack_env.append_flags('LDFLAGS', '-L{0} -lintl'.format(
self.spec['gettext'].prefix.lib))

View File

@ -38,7 +38,10 @@ class LlvmLld(Package):
depends_on('cmake', type='build')
def install(self, spec, prefix):
env['CXXFLAGS'] = self.compiler.cxx11_flag
if 'CXXFLAGS' in env and env['CXXFLAGS']:
env['CXXFLAGS'] += ' ' + self.compiler.cxx11_flag
else:
env['CXXFLAGS'] = self.compiler.cxx11_flag
with working_dir('spack-build', create=True):
cmake('..',

View File

@ -325,7 +325,7 @@ class Llvm(CMakePackage):
conflicts('+lldb', when='~clang')
def setup_environment(self, spack_env, run_env):
spack_env.set('CXXFLAGS', self.compiler.cxx11_flag)
spack_env.append_flags('CXXFLAGS', self.compiler.cxx11_flag)
def build_type(self):
if '+debug' in self.spec:

View File

@ -57,7 +57,7 @@ def libs(self):
def setup_environment(self, spack_env, run_env):
if '+pic' in self.spec:
spack_env.set('CFLAGS', self.compiler.pic_flag)
spack_env.append_flags('CFLAGS', self.compiler.pic_flag)
def install(self, spec, prefix):
config_args = []