Multi-valued variants: better support for combinations (#9481)

This enforces conventions that allow for correct handling of
multi-valued variants where specifying no value is an option,
and adds convenience functionality for specifying multi-valued
variants with conflicting sets of values. This also adds a notion
of "feature values" for variants, which are those that are understood
by the build system (e.g. those that would appear as configure
options). In more detail:

* Add documentation on variants to the packaging guide
* Forbid usage of '' or None as a possible variant value, in
  particular as a default. To indicate choosing no value, the user
  must explicitly define an option like 'none'. Without this,
  multi-valued variants with default set to None were not parsable
  from the command line (Fixes #6314)
* Add "disjoint_sets" function to support the declaration of
  multi-valued variants with conflicting sets of options. For example
  a variant "foo" with possible values "a", "b", and "c" where "c"
  is exclusive of the other values ("foo=a,b" and "foo=c" are
  valid but "foo=a,c" is not).
* Add "any_combination_of" function to support the declaration of
  multi-valued variants where it is valid to choose none of the
  values. This automatically defines "none" as an option (exclusive
  with all other choices); this value does not appear when iterating
  over the variant's values, for example in "with_or_without" (which
  constructs autotools option strings from variant values).
* The "disjoint_sets" and "any_combination_of" methods return an
  object which tracks the possible values. It is also possible to
  indicate that some of these values do not correspond to options
  understood by the package's build system, such that methods like
  "with_or_without" will not define options for those values (this
  occurs automatically for "none")
* Add documentation for usage of new functions for specifying
  multi-valued variants

15 files changed, 39 insertions, 70 deletions

diff --git a/var/spack/repos/builtin.mock/packages/a/package.py b/var/spack/repos/builtin.mock/packages/a/package.py
index 2474c0f61b..7e5a6f72e6 100644
--- a/var/spack/repos/builtin.mock/packages/a/package.py
+++ b/var/spack/repos/builtin.mock/packages/a/package.py
@@ -16,11 +16,8 @@ class A(AutotoolsPackage):
     version('2.0', '2.0_a_hash')
 
     variant(
-        'foo',
-        values=('bar', 'baz', 'fee'),
-        default='bar',
-        description='',
-        multi=True
+        'foo', description='',
+        values=any_combination_of('bar', 'baz', 'fee').with_default('bar'),
     )
 
     variant(
diff --git a/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py b/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py
index 442a756233..3ae05c5110 100644
--- a/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py
+++ b/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py
@@ -17,10 +17,8 @@ class MultivalueVariant(Package):
 
     variant('debug', default=False, description='Debug variant')
     variant(
-        'foo',
-        description='Multi-valued variant',
-        values=('bar', 'baz', 'barbaz'),
-        multi=True
+        'foo', description='Multi-valued variant',
+        values=any_combination_of('bar', 'baz', 'barbaz'),
     )
 
     variant(
diff --git a/var/spack/repos/builtin/packages/adios/package.py b/var/spack/repos/builtin/packages/adios/package.py
index 0aeb364c0f..d4ea3258e2 100644
--- a/var/spack/repos/builtin/packages/adios/package.py
+++ b/var/spack/repos/builtin/packages/adios/package.py
@@ -61,10 +61,7 @@ class Adios(AutotoolsPackage):
     variant('netcdf', default=False, description='Enable netcdf support')
 
     variant(
-        'staging',
-        default=None,
-        values=('flexpath', 'dataspaces'),
-        multi=True,
+        'staging', values=any_combination_of('flexpath', 'dataspaces'),
         description='Enable dataspaces and/or flexpath staging transports'
     )
 
diff --git a/var/spack/repos/builtin/packages/axl/package.py b/var/spack/repos/builtin/packages/axl/package.py
index 869232cfe7..a524ca0643 100644
--- a/var/spack/repos/builtin/packages/axl/package.py
+++ b/var/spack/repos/builtin/packages/axl/package.py
@@ -7,7 +7,7 @@ from spack import *
 from spack.error import SpackError
 
 
-def async_api_validator(values):
+def async_api_validator(pkg_name, variant_name, values):
     if 'none' in values and len(values) != 1:
         raise SpackError("The value 'none' is not usable"
                          " with other async_api values.")
diff --git a/var/spack/repos/builtin/packages/glib/package.py b/var/spack/repos/builtin/packages/glib/package.py
index 414b07c6ea..b36ec9362b 100644
--- a/var/spack/repos/builtin/packages/glib/package.py
+++ b/var/spack/repos/builtin/packages/glib/package.py
@@ -34,10 +34,7 @@ class Glib(AutotoolsPackage):
 
     variant('libmount', default=False, description='Build with libmount support')
     variant(
-        'tracing',
-        default='',
-        values=('dtrace', 'systemtap'),
-        multi=True,
+        'tracing', values=any_combination_of('dtrace', 'systemtap'),
         description='Enable tracing support'
     )
 
diff --git a/var/spack/repos/builtin/packages/kokkos/package.py b/var/spack/repos/builtin/packages/kokkos/package.py
index d889280b28..d73d0ae74a 100644
--- a/var/spack/repos/builtin/packages/kokkos/package.py
+++ b/var/spack/repos/builtin/packages/kokkos/package.py
@@ -71,7 +71,7 @@ class Kokkos(Package):
     # Host architecture variant
     variant(
         'host_arch',
-        default=None,
+        default='none',
         values=('AMDAVX', 'ARMv80', 'ARMv81', 'ARMv8-ThunderX',
                 'Power7', 'Power8', 'Power9',
                 'WSM', 'SNB', 'HSW', 'BDW', 'SKX', 'KNC', 'KNL'),
@@ -81,7 +81,7 @@ class Kokkos(Package):
     # GPU architecture variant
     variant(
         'gpu_arch',
-        default=None,
+        default='none',
         values=gpu_values,
         description='Set the GPU architecture to use'
     )
@@ -159,9 +159,9 @@ class Kokkos(Package):
             host_arch = spec.variants['host_arch'].value
             # GPU architectures
             gpu_arch  = spec.variants['gpu_arch'].value
-            if host_arch:
+            if host_arch != 'none':
                 arch_args.append(host_arch)
-            if gpu_arch:
+            if gpu_arch != 'none':
                 arch_args.append(gpu_arch)
             # Combined architecture flags
             if arch_args:
diff --git a/var/spack/repos/builtin/packages/matlab/package.py b/var/spack/repos/builtin/packages/matlab/package.py
index d0b242d518..639ec65890 100644
--- a/var/spack/repos/builtin/packages/matlab/package.py
+++ b/var/spack/repos/builtin/packages/matlab/package.py
@@ -35,7 +35,7 @@ class Matlab(Package):
 
     variant(
         'key',
-        default='',
+        default='<installation-key-here>',
         values=lambda x: True,  # Anything goes as a key
         description='The file installation key to use'
     )
diff --git a/var/spack/repos/builtin/packages/mvapich2/package.py b/var/spack/repos/builtin/packages/mvapich2/package.py
index 8493e58365..3cf2dba408 100644
--- a/var/spack/repos/builtin/packages/mvapich2/package.py
+++ b/var/spack/repos/builtin/packages/mvapich2/package.py
@@ -6,14 +6,6 @@
 import sys
 
 from spack import *
-from spack.error import SpackError
-
-
-def _process_manager_validator(values):
-    if len(values) > 1 and 'slurm' in values:
-        raise SpackError(
-            'slurm cannot be activated along with other process managers'
-        )
 
 
 class Mvapich2(AutotoolsPackage):
@@ -70,9 +62,12 @@ class Mvapich2(AutotoolsPackage):
     variant(
         'process_managers',
         description='List of the process managers to activate',
-        values=('slurm', 'hydra', 'gforker', 'remshell'),
-        multi=True,
-        validator=_process_manager_validator
+        values=disjoint_sets(
+            ('auto',), ('slurm',), ('hydra', 'gforker', 'remshell')
+        ).prohibit_empty_set().with_error(
+            "'slurm' or 'auto' cannot be activated along with "
+            "other process managers"
+        ).with_default('auto').with_non_feature_values('auto'),
     )
 
     variant(
@@ -94,8 +89,7 @@ class Mvapich2(AutotoolsPackage):
     variant(
         'file_systems',
         description='List of the ROMIO file systems to activate',
-        values=('lustre', 'gpfs', 'nfs', 'ufs'),
-        multi=True
+        values=auto_or_any_combination_of('lustre', 'gpfs', 'nfs', 'ufs'),
     )
 
     depends_on('findutils', type='build')
diff --git a/var/spack/repos/builtin/packages/omega-h/package.py b/var/spack/repos/builtin/packages/omega-h/package.py
index c28c10eea6..d2bee16733 100644
--- a/var/spack/repos/builtin/packages/omega-h/package.py
+++ b/var/spack/repos/builtin/packages/omega-h/package.py
@@ -3,8 +3,6 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-from spack import *
-
 
 class OmegaH(CMakePackage):
     """Omega_h is a C++11 library providing data structures and algorithms
@@ -28,8 +26,7 @@ class OmegaH(CMakePackage):
     variant('shared', default=True, description='Build shared libraries')
     variant('mpi', default=True, description='Activates MPI support')
     variant('zlib', default=True, description='Activates ZLib support')
-    variant('trilinos', default=False, description='Use Teuchos and Kokkos')
-    variant('build_type', default='')
+    variant('trilinos', default=True, description='Use Teuchos and Kokkos')
     variant('throw', default=False, description='Errors throw exceptions instead of abort')
     variant('examples', default=False, description='Compile examples')
     variant('optimize', default=True, description='Compile C++ with optimization')
diff --git a/var/spack/repos/builtin/packages/openblas/package.py b/var/spack/repos/builtin/packages/openblas/package.py
index 70ff12babf..ed8bba51ff 100644
--- a/var/spack/repos/builtin/packages/openblas/package.py
+++ b/var/spack/repos/builtin/packages/openblas/package.py
@@ -38,8 +38,8 @@ class Openblas(MakefilePackage):
     variant('ilp64', default=False, description='64 bit integers')
     variant('pic', default=True, description='Build position independent code')
 
-    variant('cpu_target', default='',
-                    description='Set CPU target architecture (leave empty for '
+    variant('cpu_target', default='auto',
+            description='Set CPU target architecture (leave empty for '
                         'autodetection; GENERIC, SSE_GENERIC, NEHALEM, ...)')
 
     variant(
@@ -150,7 +150,7 @@ class Openblas(MakefilePackage):
                 'NUM_THREADS=64',  # OpenBLAS stores present no of CPUs as max
             ]
 
-        if self.spec.variants['cpu_target'].value:
+        if self.spec.variants['cpu_target'].value != 'auto':
             make_defs += [
                 'TARGET={0}'.format(self.spec.variants['cpu_target'].value)
             ]
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index 2925048d39..5eda1d0658 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -7,8 +7,6 @@
 import os
 import sys
 
-from spack import *
-
 
 def _verbs_dir():
     """Try to find the directory where the OpenFabrics verbs package is
@@ -180,20 +178,17 @@ class Openmpi(AutotoolsPackage):
     patch('btl_vader.patch', when='@3.1.0:3.1.2')
 
     fabrics = ('psm', 'psm2', 'verbs', 'mxm', 'ucx', 'libfabric')
-
     variant(
-        'fabrics',
-        default=None if _verbs_dir() is None else 'verbs',
+        'fabrics', values=auto_or_any_combination_of(*fabrics).with_default(
+            'auto' if _verbs_dir() is None else 'verbs'
+        ),
         description="List of fabrics that are enabled",
-        values=fabrics,
-        multi=True
     )
 
+    schedulers = ('alps', 'lsf', 'tm', 'slurm', 'sge', 'loadleveler')
     variant(
-        'schedulers',
-        description='List of schedulers for which support is enabled',
-        values=('alps', 'lsf', 'tm', 'slurm', 'sge', 'loadleveler'),
-        multi=True
+        'schedulers', values=auto_or_any_combination_of(*schedulers),
+        description='List of schedulers for which support is enabled'
     )
 
     # Additional support options
diff --git a/var/spack/repos/builtin/packages/py-patsy/package.py b/var/spack/repos/builtin/packages/py-patsy/package.py
index 326825b1f5..6ffd4e19b9 100644
--- a/var/spack/repos/builtin/packages/py-patsy/package.py
+++ b/var/spack/repos/builtin/packages/py-patsy/package.py
@@ -3,8 +3,6 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-from spack import *
-
 
 class PyPatsy(PythonPackage):
     """A Python package for describing statistical models and for
@@ -15,7 +13,7 @@ class PyPatsy(PythonPackage):
 
     version('0.4.1', '9445f29e3426d1ed30d683a1e1453f84')
 
-    variant('splines', description="Offers spline related functions")
+    variant('splines', default=False, description="Offers spline related functions")
 
     depends_on('py-setuptools',  type='build')
     depends_on('py-numpy',       type=('build', 'run'))
diff --git a/var/spack/repos/builtin/packages/regcm/package.py b/var/spack/repos/builtin/packages/regcm/package.py
index a732f6c343..f9a1b7c65c 100644
--- a/var/spack/repos/builtin/packages/regcm/package.py
+++ b/var/spack/repos/builtin/packages/regcm/package.py
@@ -25,8 +25,10 @@ class Regcm(AutotoolsPackage):
     # producing a so-called fat binary. Unfortunately, gcc builds only the last
     # architecture provided (in the configure), so we allow a single arch.
     extensions = ('knl', 'skl', 'bdw', 'nhl')
-    variant('extension', default=None, values=extensions, multi=True,
-            description='Build extensions for a specific Intel architecture.')
+    variant(
+        'extension', values=any_combination_of(extensions),
+        description='Build extensions for a specific Intel architecture.'
+    )
 
     depends_on('netcdf')
     depends_on('netcdf-fortran')
diff --git a/var/spack/repos/builtin/packages/scr/package.py b/var/spack/repos/builtin/packages/scr/package.py
index 1f33bf7ca2..3cf94d7471 100644
--- a/var/spack/repos/builtin/packages/scr/package.py
+++ b/var/spack/repos/builtin/packages/scr/package.py
@@ -47,7 +47,7 @@ class Scr(CMakePackage):
     variant('scr_config', default='scr.conf',
             description='Location for SCR to find its system config file. '
             'May be either absolute or relative to the install prefix')
-    variant('copy_config', default=None,
+    variant('copy_config', default='none',
             description='Location from which to copy SCR system config file. '
             'Must be an absolute path.')
 
@@ -130,7 +130,7 @@ class Scr(CMakePackage):
     @run_after('install')
     def copy_config(self):
         spec = self.spec
-        if spec.variants['copy_config'].value:
+        if spec.variants['copy_config'].value != 'none':
             dest_path = self.get_abs_path_rel_prefix(
                 spec.variants['scr_config'].value)
             install(spec.variants['copy_config'].value, dest_path)
diff --git a/var/spack/repos/builtin/packages/yambo/package.py b/var/spack/repos/builtin/packages/yambo/package.py
index 04b8272aa6..5e70c358ea 100644
--- a/var/spack/repos/builtin/packages/yambo/package.py
+++ b/var/spack/repos/builtin/packages/yambo/package.py
@@ -26,19 +26,13 @@ class Yambo(AutotoolsPackage):
 
     variant('dp', default=False, description='Enable double precision')
     variant(
-        'profile',
-        values=('time', 'memory'),
-        default='',
-        description='Activate profiling of specific sections',
-        multi=True
+        'profile', values=any_combination_of('time', 'memory'),
+        description='Activate profiling of specific sections'
     )
 
     variant(
-        'io',
-        values=('iotk', 'etsf-io'),
-        default='',
+        'io', values=any_combination_of('iotk', 'etsf-io'),
         description='Activate support for different io formats (requires network access)',  # noqa
-        multi=True
     )
 
     # MPI + OpenMP parallelism