'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.

3 files changed, 51 insertions, 73 deletions

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