From 32117c22deb97c0be06ef073c432e45569b138c3 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 12 Sep 2017 01:20:49 +0200 Subject: 'with_or_without' accepts bool variants Fixes #4112 This commit extends the support of the AutotoolsPackage methods `with_or_without` and `enable_or_disable` to bool-valued variants. It also defines for those functions a convenience short-cut if the activation parameter is the prefix of a spec (like in `--with-{pkg}={prefix}`). This commit also includes: * Updates to viennarna and adios accordingly: they have been modified to use `enable_or_disable` and `with_or_without` * Improved docstrings in `autotools.py`. Raise `KeyError` if name is not a variant. --- lib/spack/spack/build_systems/autotools.py | 160 +++++++++++++++++---- lib/spack/spack/test/build_systems.py | 27 ++-- var/spack/repos/builtin.mock/packages/a/package.py | 2 + var/spack/repos/builtin/packages/adios/package.py | 98 ++++++------- .../repos/builtin/packages/viennarna/package.py | 24 ++-- 5 files changed, 202 insertions(+), 109 deletions(-) diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py index afabb90820..b78d3ed026 100644 --- a/lib/spack/spack/build_systems/autotools.py +++ b/lib/spack/spack/build_systems/autotools.py @@ -291,49 +291,161 @@ class AutotoolsPackage(PackageBase): self._if_make_target_execute('test') self._if_make_target_execute('check') - def _activate_or_not(self, active, inactive, name, active_parameters=None): + def _activate_or_not( + self, + name, + activation_word, + deactivation_word, + activation_value=None + ): + """This function contains the current implementation details of + :py:meth:`~.AutotoolsPackage.with_or_without` and + :py:meth:`~.AutotoolsPackage.enable_or_disable`. + + Args: + name (str): name of the variant that is being processed + activation_word (str): the default activation word ('with' in the + case of ``with_or_without``) + deactivation_word (str): the default deactivation word ('without' + in the case of ``with_or_without``) + activation_value (callable): callable that accepts a single + value. This value is either one of the allowed values for a + multi-valued variant or the name of a bool-valued variant. + Returns the parameter to be used when the value is activated. + + The special value 'prefix' can also be assigned and will return + ``spec[name].prefix`` as activation parameter. + + Examples: + + Given a package with: + + .. code-block:: python + + variant('foo', values=('x', 'y'), description='') + variant('bar', default=True, description='') + + calling this function like: + + .. code-block:: python + + _activate_or_not( + 'foo', 'with', 'without', activation_value='prefix' + ) + _activate_or_not('bar', 'with', 'without') + + will generate the following configuration options: + + .. code-block:: console + + --with-x= --without-y --with-bar + + for `` foo=x +bar`` + + Returns: + list of strings that corresponds to the activation/deactivation + of the variant that has been processed + + Raises: + KeyError: if name is not among known variants + """ spec = self.spec args = [] + + if activation_value == 'prefix': + activation_value = lambda x: spec[x].prefix + + # Defensively look that the name passed as argument is among + # variants + if name not in self.variants: + msg = '"{0}" is not a variant of "{1}"' + raise KeyError(msg.format(name, self.name)) + + # Create a list of pairs. Each pair includes a configuration + # option and whether or not that option is activated + if set(self.variants[name].values) == set((True, False)): + # BoolValuedVariant carry information about a single option. + # Nonetheless, for uniformity of treatment we'll package them + # in an iterable of one element. + condition = '+{name}'.format(name=name) + options = [(name, condition in spec)] + else: + condition = '{name}={value}' + options = [ + (value, condition.format(name=name, value=value) in spec) + for value in self.variants[name].values + ] + # For each allowed value in the list of values - for value in self.variants[name].values: - # Check if the value is active in the current spec - condition = '{name}={value}'.format(name=name, value=value) - activated = condition in spec + for option_value, activated in options: # Search for an override in the package for this value - override_name = '{0}_or_{1}_{2}'.format(active, inactive, value) + override_name = '{0}_or_{1}_{2}'.format( + activation_word, deactivation_word, option_value + ) line_generator = getattr(self, override_name, None) # If not available use a sensible default if line_generator is None: def _default_generator(is_activated): if is_activated: - line = '--{0}-{1}'.format(active, value) - if active_parameters is not None and active_parameters(value): # NOQA=ignore=E501 - line += '={0}'.format(active_parameters(value)) + line = '--{0}-{1}'.format( + activation_word, option_value + ) + if activation_value is not None and activation_value(option_value): # NOQA=ignore=E501 + line += '={0}'.format( + activation_value(option_value) + ) return line - return '--{0}-{1}'.format(inactive, value) + return '--{0}-{1}'.format(deactivation_word, option_value) line_generator = _default_generator args.append(line_generator(activated)) return args - def with_or_without(self, name, active_parameters=None): - """Inspects the multi-valued variant 'name' and returns the configure - arguments that activate / deactivate the selected feature. + def with_or_without(self, name, activation_value=None): + """Inspects a variant and returns the arguments that activate + or deactivate the selected feature(s) for the configure options. + + This function works on all type of variants. For bool-valued variants + it will return by default ``--with-{name}`` or ``--without-{name}``. + For other kinds of variants it will cycle over the allowed values and + return either ``--with-{value}`` or ``--without-{value}``. + + If activation_value is given, then for each possible value of the + variant, the option ``--with-{value}=activation_value(value)`` or + ``--without-{value}`` will be added depending on whether or not + ``variant=value`` is in the spec. - :param str name: name of a valid multi-valued variant - :param callable active_parameters: if present accepts a single value - and returns the parameter to be used leading to an entry of the - type '--with-{name}={parameter} + Args: + name (str): name of a valid multi-valued variant + activation_value (callable): callable that accepts a single + value and returns the parameter to be used leading to an entry + of the type ``--with-{name}={parameter}``. + + The special value 'prefix' can also be assigned and will return + ``spec[name].prefix`` as activation parameter. + + Returns: + list of arguments to configure """ - return self._activate_or_not( - 'with', 'without', name, active_parameters - ) + return self._activate_or_not(name, 'with', 'without', activation_value) + + def enable_or_disable(self, name, activation_value=None): + """Same as :py:meth:`~.AutotoolsPackage.with_or_without` but substitute + ``with`` with ``enable`` and ``without`` with ``disable``. + + Args: + name (str): name of a valid multi-valued variant + activation_value (callable): if present accepts a single value + and returns the parameter to be used leading to an entry of the + type ``--enable-{name}={parameter}`` + + The special value 'prefix' can also be assigned and will return + ``spec[name].prefix`` as activation parameter. - def enable_or_disable(self, name, active_parameters=None): - """Inspects the multi-valued variant 'name' and returns the configure - arguments that activate / deactivate the selected feature. + Returns: + list of arguments to configure """ return self._activate_or_not( - 'enable', 'disable', name, active_parameters + name, 'enable', 'disable', activation_value ) run_after('install')(PackageBase._run_default_install_time_test_callbacks) diff --git a/lib/spack/spack/test/build_systems.py b/lib/spack/spack/test/build_systems.py index 114497bccd..9960966e2f 100644 --- a/lib/spack/spack/test/build_systems.py +++ b/lib/spack/spack/test/build_systems.py @@ -53,20 +53,23 @@ class TestAutotoolsPackage(object): pkg = spack.repo.get(s) # Called without parameters - l = pkg.with_or_without('foo') - assert '--with-bar' in l - assert '--without-baz' in l - assert '--no-fee' in l + options = pkg.with_or_without('foo') + assert '--with-bar' in options + assert '--without-baz' in options + assert '--no-fee' in options def activate(value): return 'something' - l = pkg.with_or_without('foo', active_parameters=activate) - assert '--with-bar=something' in l - assert '--without-baz' in l - assert '--no-fee' in l + options = pkg.with_or_without('foo', activation_value=activate) + assert '--with-bar=something' in options + assert '--without-baz' in options + assert '--no-fee' in options - l = pkg.enable_or_disable('foo') - assert '--enable-bar' in l - assert '--disable-baz' in l - assert '--disable-fee' in l + options = pkg.enable_or_disable('foo') + assert '--enable-bar' in options + assert '--disable-baz' in options + assert '--disable-fee' in options + + options = pkg.with_or_without('bvv') + assert '--with-bvv' in options diff --git a/var/spack/repos/builtin.mock/packages/a/package.py b/var/spack/repos/builtin.mock/packages/a/package.py index dc078d2434..463ad10055 100644 --- a/var/spack/repos/builtin.mock/packages/a/package.py +++ b/var/spack/repos/builtin.mock/packages/a/package.py @@ -50,6 +50,8 @@ class A(AutotoolsPackage): multi=False ) + variant('bvv', default=True, description='The good old BV variant') + depends_on('b', when='foobar=bar') def with_or_without_fee(self, activated): diff --git a/var/spack/repos/builtin/packages/adios/package.py b/var/spack/repos/builtin/packages/adios/package.py index 88721a477a..91188b8e4e 100644 --- a/var/spack/repos/builtin/packages/adios/package.py +++ b/var/spack/repos/builtin/packages/adios/package.py @@ -61,9 +61,14 @@ class Adios(AutotoolsPackage): # transports and serial file converters variant('hdf5', default=False, description='Enable parallel HDF5 transport and serial bp2h5 converter') variant('netcdf', default=False, description='Enable netcdf support') - variant('flexpath', default=False, description='Enable flexpath transport') - variant('dataspaces', default=False, description='Enable dataspaces transport') - variant('staging', default=False, description='Enable dataspaces and flexpath staging transports') + + variant( + 'staging', + default=None, + values=('flexpath', 'dataspaces'), + multi=True, + description='Enable dataspaces and/or flexpath staging transports' + ) depends_on('autoconf', type='build') depends_on('automake', type='build') @@ -100,15 +105,27 @@ class Adios(AutotoolsPackage): patch('adios_1100.patch', when='@:1.10.0^hdf5@1.10:') def validate(self, spec): - """ - Checks if incompatible variants have been activated at the same time - :param spec: spec of the package - :raises RuntimeError: in case of inconsistencies + """Checks if incompatible variants have been activated at the same time + + Args: + spec: spec of the package + + Raises: + RuntimeError: in case of inconsistencies """ if '+fortran' in spec and not self.compiler.fc: msg = 'cannot build a fortran variant without a fortran compiler' raise RuntimeError(msg) + def with_or_without_hdf5(self, activated): + + if activated: + return '--with-phdf5={0}'.format( + self.spec['hdf5'].prefix + ) + + return '--without-phdf5' + def configure_args(self): spec = self.spec self.validate(spec) @@ -118,66 +135,31 @@ class Adios(AutotoolsPackage): 'CFLAGS={0}'.format(self.compiler.pic_flag) ] - if '+shared' in spec: - extra_args.append('--enable-shared') + extra_args += self.enable_or_disable('shared') + extra_args += self.enable_or_disable('fortran') if '+mpi' in spec: env['MPICC'] = spec['mpi'].mpicc env['MPICXX'] = spec['mpi'].mpicxx - extra_args.append('--with-mpi=%s' % spec['mpi'].prefix) - else: - extra_args.append('--without-mpi') - if '+infiniband' in spec: - extra_args.append('--with-infiniband') - else: - extra_args.append('--with-infiniband=no') - - if '+fortran' in spec: - extra_args.append('--enable-fortran') - else: - extra_args.append('--disable-fortran') + + extra_args += self.with_or_without('mpi', activation='prefix') + extra_args += self.with_or_without('infiniband') # Transforms - if '+zlib' in spec: - extra_args.append('--with-zlib=%s' % spec['zlib'].prefix) - else: - extra_args.append('--without-zlib') - if '+bzip2' in spec: - extra_args.append('--with-bzip2=%s' % spec['bzip2'].prefix) - else: - extra_args.append('--without-bzip2') - if '+szip' in spec: - extra_args.append('--with-szip=%s' % spec['szip'].prefix) - else: - extra_args.append('--without-szip') - if '+zfp' in spec: - extra_args.append('--with-zfp=%s' % spec['zfp'].prefix) - else: - extra_args.append('--without-zfp') - if '+sz' in spec: - extra_args.append('--with-sz=%s' % spec['sz'].prefix) - else: - extra_args.append('--without-sz') + variants = ['zlib', 'bzip2', 'szip', 'zfp', 'sz'] # External I/O libraries - if '+hdf5' in spec: - extra_args.append('--with-phdf5=%s' % spec['hdf5'].prefix) - else: - extra_args.append('--without-phdf5') - if '+netcdf' in spec: - extra_args.append('--with-netcdf=%s' % spec['netcdf'].prefix) - else: - extra_args.append('--without-netcdf') + variants += ['hdf5', 'netcdf'] + + for x in variants: + extra_args += self.with_or_without(x, activation='prefix') # Staging transports - if '+flexpath' in spec or '+staging' in spec: - extra_args.append('--with-flexpath=%s' % spec['libevpath'].prefix) - else: - extra_args.append('--without-flexpath') - if '+dataspaces' in spec or '+staging' in spec: - extra_args.append('--with-dataspaces=%s' - % spec['dataspaces'].prefix) - else: - extra_args.append('--without-dataspaces') + def with_staging(name): + if name == 'flexpath': + return spec['libevpath'].prefix + return spec[name].prefix + + extra_args += self.with_or_without('staging', activation=with_staging) return extra_args diff --git a/var/spack/repos/builtin/packages/viennarna/package.py b/var/spack/repos/builtin/packages/viennarna/package.py index dbaff25eec..7f8f2cb18c 100644 --- a/var/spack/repos/builtin/packages/viennarna/package.py +++ b/var/spack/repos/builtin/packages/viennarna/package.py @@ -27,8 +27,9 @@ from spack import * class Viennarna(AutotoolsPackage): """The ViennaRNA Package consists of a C code library and several - stand-alone programs for the prediction and comparison of RNA secondary - structures.""" + stand-alone programs for the prediction and comparison of RNA secondary + structures. + """ homepage = "https://www.tbi.univie.ac.at/RNA/" url = "https://www.tbi.univie.ac.at/RNA/download/sourcecode/2_3_x/ViennaRNA-2.3.5.tar.gz" @@ -49,19 +50,12 @@ class Viennarna(AutotoolsPackage): return url.format(version.up_to(2).underscored, version) def configure_args(self): - args = [] - if '+sse' in self.spec: - args.append('--enable-sse') - else: - args.append('--disable-sse') - if '~python' in self.spec: - args.append('--without-python') - else: - args.append('--with-python') - if '~perl' in self.spec: - args.append('--without-perl') - else: - args.append('--with-perl') + + args = self.enable_or_disable('sse') + args += self.with_or_without('python') + args += self.with_or_without('perl') + if 'python@3:' in self.spec: args.append('--with-python3') + return args -- cgit v1.2.3-70-g09d2