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

24 files changed, 585 insertions, 89 deletions

diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst
index 1c78a396e3..641bb74fea 100644
--- a/lib/spack/docs/packaging_guide.rst
+++ b/lib/spack/docs/packaging_guide.rst
@@ -1077,6 +1077,174 @@ Go cannot be used to fetch a particular commit or branch, it always
 downloads the head of the repository. This download method is untrusted,
 and is not recommended. Use another fetch strategy whenever possible.
 
+--------
+Variants
+--------
+
+Many software packages can be configured to enable optional
+features, which often come at the expense of additional dependencies or
+longer build-times. To be flexible enough and support a wide variety of
+use cases, Spack permits to expose to the end-user the ability to choose
+which features should be activated in a package at the time it is installed.
+The mechanism to be employed is the :py:func:`spack.directives.variant` directive.
+
+^^^^^^^^^^^^^^^^
+Boolean variants
+^^^^^^^^^^^^^^^^
+
+In their simplest form variants are boolean options specified at the package
+level:
+
+  .. code-block:: python
+
+    class Hdf5(AutotoolsPackage):
+        ...
+        variant(
+            'shared', default=True, description='Builds a shared version of the library'
+        )
+
+with a default value and a description of their meaning / use in the package.
+*Variants can be tested in any context where a spec constraint is expected.*
+In the example above the ``shared`` variant is tied to the build of shared dynamic
+libraries. To pass the right option at configure time we can branch depending on
+its value:
+
+  .. code-block:: python
+
+    def configure_args(self):
+        ...
+        if '+shared' in self.spec:
+            extra_args.append('--enable-shared')
+        else:
+            extra_args.append('--disable-shared')
+            extra_args.append('--enable-static-exec')
+
+As explained in :ref:`basic-variants` the constraint ``+shared`` means
+that the boolean variant is set to ``True``, while ``~shared`` means it is set
+to ``False``.
+Another common example is the optional activation of an extra dependency
+which requires to use the variant in the ``when`` argument of
+:py:func:`spack.directives.depends_on`:
+
+  ..  code-block:: python
+
+    class Hdf5(AutotoolsPackage):
+        ...
+        variant('szip', default=False, description='Enable szip support')
+        depends_on('szip', when='+szip')
+
+as shown in the snippet above where ``szip`` is modeled to be an optional
+dependency of ``hdf5``.
+
+^^^^^^^^^^^^^^^^^^^^^
+Multi-valued variants
+^^^^^^^^^^^^^^^^^^^^^
+
+If need be, Spack can go beyond Boolean variants and permit an arbitrary
+number of allowed values. This might be useful when modeling
+options that are tightly related to each other.
+The values in this case are passed to the :py:func:`spack.directives.variant`
+directive as a tuple:
+
+  .. code-block:: python
+
+    class Blis(Package):
+        ...
+        variant(
+            'threads', default='none', description='Multithreading support',
+            values=('pthreads', 'openmp', 'none'), multi=False
+        )
+
+In the example above the argument ``multi`` is set to ``False`` to indicate
+that only one among all the variant values can be active at any time. This
+constraint is enforced by the parser and an error is emitted if a user
+specifies two or more values at the same time:
+
+  .. code-block:: console
+
+    $ spack spec blis threads=openmp,pthreads
+    Input spec
+    --------------------------------
+    blis threads=openmp,pthreads
+
+    Concretized
+    --------------------------------
+    ==> Error: multiple values are not allowed for variant "threads"
+
+Another useful note is that *Python's* ``None`` *is not allowed as a default value*
+and therefore it should not be used to denote that no feature was selected.
+Users should instead select another value, like ``'none'``, and handle it explicitly
+within the package recipe if need be:
+
+  .. code-block:: python
+
+      if self.spec.variants['threads'].value == 'none':
+         options.append('--no-threads')
+
+In cases where multiple values can be selected at the same time ``multi`` should
+be set to ``True``:
+
+  .. code-block:: python
+
+    class Gcc(AutotoolsPackage):
+        ...
+        variant(
+            'languages', default='c,c++,fortran',
+            values=('ada', 'brig', 'c', 'c++', 'fortran',
+                    'go', 'java', 'jit', 'lto', 'objc', 'obj-c++'),
+            multi=True,
+            description='Compilers and runtime libraries to build'
+        )
+
+Within a package recipe a multi-valued variant is tested using a ``key=value`` syntax:
+
+  .. code-block:: python
+
+    if 'languages=jit' in spec:
+        options.append('--enable-host-shared')
+
+"""""""""""""""""""""""""""""""""""""""""""
+Complex validation logic for variant values
+"""""""""""""""""""""""""""""""""""""""""""
+To cover complex use cases, the :py:func:`spack.directives.variant` directive
+could accept as the ``values`` argument a full-fledged object which has
+``default`` and other arguments of the directive embedded as attributes.
+
+An example, already implemented in Spack's core, is :py:class:`spack.variant.DisjointSetsOfValues`.
+This class is used to implement a few convenience functions, like
+:py:func:`spack.variant.any_combination_of`:
+
+  ..  code-block:: python
+
+    class Adios(AutotoolsPackage):
+        ...
+        variant(
+            'staging',
+            values=any_combination_of('flexpath', 'dataspaces'),
+            description='Enable dataspaces and/or flexpath staging transports'
+        )
+
+that allows any combination of the specified values, and also allows the
+user to specify ``'none'`` (as a string) to choose none of them.
+The objects returned by these functions can be modified at will by chaining
+method calls to change the default value, customize the error message or
+other similar operations:
+
+  .. code-block:: python
+
+    class Mvapich2(AutotoolsPackage):
+        ...
+        variant(
+            'process_managers',
+            description='List of the process managers to activate',
+            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'),
+        )
+
 ------------------------------------
 Resources (expanding extra tarballs)
 ------------------------------------
diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py
index 12fe7ae210..3673fb7cce 100644
--- a/lib/spack/spack/build_systems/autotools.py
+++ b/lib/spack/spack/build_systems/autotools.py
@@ -359,9 +359,17 @@ class AutotoolsPackage(PackageBase):
             options = [(name, condition in spec)]
         else:
             condition = '{name}={value}'
+            # "feature_values" is used to track values which correspond to
+            # features which can be enabled or disabled as understood by the
+            # package's build system. It excludes values which have special
+            # meanings and do not correspond to features (e.g. "none")
+            feature_values = getattr(
+                self.variants[name].values, 'feature_values', None
+            ) or self.variants[name].values
+
             options = [
                 (value, condition.format(name=name, value=value) in spec)
-                for value in self.variants[name].values
+                for value in feature_values
             ]
 
         # For each allowed value in the list of values
diff --git a/lib/spack/spack/build_systems/cuda.py b/lib/spack/spack/build_systems/cuda.py
index 1cecdf2ccd..97dba30a16 100644
--- a/lib/spack/spack/build_systems/cuda.py
+++ b/lib/spack/spack/build_systems/cuda.py
@@ -5,8 +5,11 @@
 
 from spack.package import PackageBase
 from spack.directives import depends_on, variant, conflicts
+
 import platform
 
+import spack.variant
+
 
 class CudaPackage(PackageBase):
     """Auxiliary class which contains CUDA variant, dependencies and conflicts
@@ -19,11 +22,12 @@ class CudaPackage(PackageBase):
             description='Build with CUDA')
     # see http://docs.nvidia.com/cuda/cuda-compiler-driver-nvcc/index.html#gpu-feature-list
     # https://developer.nvidia.com/cuda-gpus
-    variant('cuda_arch', default=None,
+    variant('cuda_arch',
             description='CUDA architecture',
-            values=('20', '30', '32', '35', '50', '52', '53', '60', '61',
-                    '62', '70'),
-            multi=True)
+            values=spack.variant.any_combination_of(
+                '20', '30', '32', '35', '50', '52', '53', '60', '61',
+                '62', '70'
+            ))
 
     # see http://docs.nvidia.com/cuda/cuda-compiler-driver-nvcc/index.html#nvcc-examples
     # and http://llvm.org/docs/CompileCudaWithLLVM.html#compiling-cuda-code
diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py
index d167f856e2..08df20ae88 100644
--- a/lib/spack/spack/directives.py
+++ b/lib/spack/spack/directives.py
@@ -34,6 +34,7 @@ import re
 from six import string_types
 
 import llnl.util.lang
+import llnl.util.tty.color
 
 import spack.error
 import spack.patch
@@ -439,7 +440,7 @@ def variant(
         default=None,
         description='',
         values=None,
-        multi=False,
+        multi=None,
         validator=None):
     """Define a variant for the package. Packager can specify a default
     value as well as a text description.
@@ -456,23 +457,65 @@ def variant(
         multi (bool): if False only one value per spec is allowed for
             this variant
         validator (callable): optional group validator to enforce additional
-            logic. It receives a tuple of values and should raise an instance
-            of SpackError if the group doesn't meet the additional constraints
+            logic. It receives the package name, the variant name and a tuple
+            of values and should raise an instance of SpackError if the group
+            doesn't meet the additional constraints
+
+    Raises:
+        DirectiveError: if arguments passed to the directive are invalid
     """
+    def format_error(msg, pkg):
+        msg += " @*r{{[{0}, variant '{1}']}}"
+        return llnl.util.tty.color.colorize(msg.format(pkg.name, name))
+
     if name in reserved_names:
-        raise ValueError("The variant name '%s' is reserved by Spack" % name)
+        def _raise_reserved_name(pkg):
+            msg = "The name '%s' is reserved by Spack" % name
+            raise DirectiveError(format_error(msg, pkg))
+        return _raise_reserved_name
 
+    # Ensure we have a sequence of allowed variant values, or a
+    # predicate for it.
     if values is None:
-        if default in (True, False) or (type(default) is str and
-                                        default.upper() in ('TRUE', 'FALSE')):
+        if str(default).upper() in ('TRUE', 'FALSE'):
             values = (True, False)
         else:
             values = lambda x: True
 
-    if default is None:
-        default = False if values == (True, False) else ''
+    # The object defining variant values might supply its own defaults for
+    # all the other arguments. Ensure we have no conflicting definitions
+    # in place.
+    for argument in ('default', 'multi', 'validator'):
+        # TODO: we can consider treating 'default' differently from other
+        # TODO: attributes and let a packager decide whether to use the fluent
+        # TODO: interface or the directive argument
+        if hasattr(values, argument) and locals()[argument] is not None:
+            def _raise_argument_error(pkg):
+                msg = "Remove specification of {0} argument: it is handled " \
+                      "by an attribute of the 'values' argument"
+                raise DirectiveError(format_error(msg.format(argument), pkg))
+            return _raise_argument_error
+
+    # Allow for the object defining the allowed values to supply its own
+    # default value and group validator, say if it supports multiple values.
+    default = getattr(values, 'default', default)
+    validator = getattr(values, 'validator', validator)
+    multi = getattr(values, 'multi', bool(multi))
+
+    # Here we sanitize against a default value being either None
+    # or the empty string, as the former indicates that a default
+    # was not set while the latter will make the variant unparsable
+    # from the command line
+    if default is None or default == '':
+        def _raise_default_not_set(pkg):
+            if default is None:
+                msg = "either a default was not explicitly set, " \
+                      "or 'None' was used"
+            elif default == '':
+                msg = "the default cannot be an empty string"
+            raise DirectiveError(format_error(msg, pkg))
+        return _raise_default_not_set
 
-    default = default
     description = str(description).strip()
 
     def _execute_variant(pkg):
diff --git a/lib/spack/spack/pkgkit.py b/lib/spack/spack/pkgkit.py
index d1a39c1a31..85c1ee264e 100644
--- a/lib/spack/spack/pkgkit.py
+++ b/lib/spack/spack/pkgkit.py
@@ -47,3 +47,6 @@ from spack.util.executable import *
 from spack.package import \
     install_dependency_symlinks, flatten_dependencies, \
     DependencyConflictError, InstallError, ExternalPackageError
+
+from spack.variant import any_combination_of, auto_or_any_combination_of
+from spack.variant import disjoint_sets
diff --git a/lib/spack/spack/test/build_systems.py b/lib/spack/spack/test/build_systems.py
index a2e684aecf..1932d1bbc2 100644
--- a/lib/spack/spack/test/build_systems.py
+++ b/lib/spack/spack/test/build_systems.py
@@ -123,8 +123,11 @@ class TestAutotoolsPackage(object):
         s.concretize()
         pkg = spack.repo.get(s)
 
-        # Called without parameters
         options = pkg.with_or_without('foo')
+
+        # Ensure that values that are not representing a feature
+        # are not used by with_or_without
+        assert '--without-none' not in options
         assert '--with-bar' in options
         assert '--without-baz' in options
         assert '--no-fee' in options
@@ -133,14 +136,30 @@ class TestAutotoolsPackage(object):
             return 'something'
 
         options = pkg.with_or_without('foo', activation_value=activate)
+        assert '--without-none' not in options
         assert '--with-bar=something' in options
         assert '--without-baz' in options
         assert '--no-fee' in options
 
         options = pkg.enable_or_disable('foo')
+        assert '--disable-none' not in options
         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
+
+    def test_none_is_allowed(self):
+        s = Spec('a foo=none')
+        s.concretize()
+        pkg = spack.repo.get(s)
+
+        options = pkg.with_or_without('foo')
+
+        # Ensure that values that are not representing a feature
+        # are not used by with_or_without
+        assert '--with-none' not in options
+        assert '--without-bar' in options
+        assert '--without-baz' in options
+        assert '--no-fee' in options
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index 9ff5795e30..4ad9c5f9c7 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -3,7 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import spack.architecture
+import sys
 import pytest
 
 from spack.spec import Spec, UnsatisfiableSpecError
@@ -11,6 +11,10 @@ from spack.spec import substitute_abstract_variants, parse_anonymous_spec
 from spack.variant import InvalidVariantValueError
 from spack.variant import MultipleValuesInExclusiveVariantError
 
+import spack.architecture
+import spack.directives
+import spack.error
+
 
 def target_factory(spec_string, target_concrete):
     spec = Spec(spec_string) if spec_string else Spec()
@@ -785,3 +789,49 @@ class TestSpecSematics(object):
                 s.compiler_flags[x] == ['-O0', '-g']
                 for x in ('cflags', 'cxxflags', 'fflags')
             )
+
+    def test_any_combination_of(self):
+        # Test that using 'none' and another value raise during concretization
+        spec = Spec('multivalue_variant foo=none,bar')
+        with pytest.raises(spack.error.SpecError) as exc_info:
+            spec.concretize()
+
+        assert "is mutually exclusive with any of the" in str(exc_info.value)
+
+    @pytest.mark.skipif(
+        sys.version_info[0] == 2, reason='__wrapped__ requires python 3'
+    )
+    def test_errors_in_variant_directive(self):
+        variant = spack.directives.variant.__wrapped__
+
+        class Pkg(object):
+            name = 'PKG'
+
+        # We can't use names that are reserved by Spack
+        fn = variant('patches')
+        with pytest.raises(spack.directives.DirectiveError) as exc_info:
+            fn(Pkg())
+        assert "The name 'patches' is reserved" in str(exc_info.value)
+
+        # We can't have conflicting definitions for arguments
+        fn = variant(
+            'foo', values=spack.variant.any_combination_of('fee', 'foom'),
+            default='bar'
+        )
+        with pytest.raises(spack.directives.DirectiveError) as exc_info:
+            fn(Pkg())
+        assert " it is handled by an attribute of the 'values' " \
+               "argument" in str(exc_info.value)
+
+        # We can't leave None as a default value
+        fn = variant('foo', default=None)
+        with pytest.raises(spack.directives.DirectiveError) as exc_info:
+            fn(Pkg())
+        assert "either a default was not explicitly set, or 'None' was used"\
+               in str(exc_info.value)
+
+        # We can't use an empty string as a default value
+        fn = variant('foo', default='')
+        with pytest.raises(spack.directives.DirectiveError) as exc_info:
+            fn(Pkg())
+        assert "the default cannot be an empty string" in str(exc_info.value)
diff --git a/lib/spack/spack/test/variant.py b/lib/spack/spack/test/variant.py
index c28bdf7da8..2a937422d3 100644
--- a/lib/spack/spack/test/variant.py
+++ b/lib/spack/spack/test/variant.py
@@ -13,6 +13,9 @@ from spack.variant import UnsatisfiableVariantSpecError
 from spack.variant import InconsistentValidationError
 from spack.variant import MultipleValuesInExclusiveVariantError
 from spack.variant import InvalidVariantValueError, DuplicateVariantError
+from spack.variant import disjoint_sets
+
+import spack.error
 
 
 class TestMultiValuedVariant(object):
@@ -692,3 +695,59 @@ class TestVariantMapTest(object):
         c['feebar'] = SingleValuedVariant('feebar', 'foo')
         c['shared'] = BoolValuedVariant('shared', True)
         assert str(c) == ' feebar=foo foo=bar,baz foobar=fee +shared'
+
+
+def test_disjoint_set_initialization_errors():
+    # Constructing from non-disjoint sets should raise an exception
+    with pytest.raises(spack.error.SpecError) as exc_info:
+        disjoint_sets(('a', 'b'), ('b', 'c'))
+    assert 'sets in input must be disjoint' in str(exc_info.value)
+
+    # A set containing the reserved item 'none' along with other items
+    # should raise an exception
+    with pytest.raises(spack.error.SpecError) as exc_info:
+        disjoint_sets(('a', 'b'), ('none', 'c'))
+    assert "The value 'none' represents the empty set," in str(exc_info.value)
+
+
+def test_disjoint_set_initialization():
+    # Test that no error is thrown when the sets are disjoint
+    d = disjoint_sets(('a',), ('b', 'c'), ('e', 'f'))
+
+    assert d.default is 'none'
+    assert d.multi is True
+    assert set(x for x in d) == set(['none', 'a', 'b', 'c', 'e', 'f'])
+
+
+def test_disjoint_set_fluent_methods():
+    # Construct an object without the empty set
+    d = disjoint_sets(('a',), ('b', 'c'), ('e', 'f')).prohibit_empty_set()
+    assert set(('none',)) not in d.sets
+
+    # Call this 2 times to check that no matter whether
+    # the empty set was allowed or not before, the state
+    # returned is consistent.
+    for _ in range(2):
+        d = d.allow_empty_set()
+        assert set(('none',)) in d.sets
+        assert 'none' in d
+        assert 'none' in [x for x in d]
+        assert 'none' in d.feature_values
+
+    # Marking a value as 'non-feature' removes it from the
+    # list of feature values, but not for the items returned
+    # when iterating over the object.
+    d = d.with_non_feature_values('none')
+    assert 'none' in d
+    assert 'none' in [x for x in d]
+    assert 'none' not in d.feature_values
+
+    # Call this 2 times to check that no matter whether
+    # the empty set was allowed or not before, the state
+    # returned is consistent.
+    for _ in range(2):
+        d = d.prohibit_empty_set()
+        assert set(('none',)) not in d.sets
+        assert 'none' not in d
+        assert 'none' not in [x for x in d]
+        assert 'none' not in d.feature_values
diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py
index 05e9ac810d..2874eeb60a 100644
--- a/lib/spack/spack/variant.py
+++ b/lib/spack/spack/variant.py
@@ -9,14 +9,21 @@ variants both in packages and in specs.
 
 import functools
 import inspect
+import itertools
 import re
 from six import StringIO
 
+import llnl.util.tty.color
 import llnl.util.lang as lang
 
 import spack.directives
 import spack.error as error
 
+try:
+    from collections.abc import Sequence
+except ImportError:
+    from collections import Sequence
+
 
 class Variant(object):
     """Represents a variant in a package, as declared in the
@@ -71,8 +78,8 @@ class Variant(object):
 
         else:
             # Otherwise assume values is the set of allowed explicit values
-            self.values = tuple(values)
-            allowed = self.values + (self.default,)
+            self.values = values
+            allowed = tuple(self.values) + (self.default,)
             self.single_value_validator = lambda x: x in allowed
 
         self.multi = multi
@@ -118,7 +125,7 @@ class Variant(object):
 
         # Validate the group of values if needed
         if self.group_validator is not None:
-            self.group_validator(value)
+            self.group_validator(pkg.name, self.name, value)
 
     @property
     def allowed_values(self):
@@ -598,6 +605,172 @@ def substitute_abstract_variants(spec):
         spec.variants.substitute(new_variant)
 
 
+# The class below inherit from Sequence to disguise as a tuple and comply
+# with the semantic expected by the 'values' argument of the variant directive
+class DisjointSetsOfValues(Sequence):
+    """Allows combinations from one of many mutually exclusive sets.
+
+    The value ``('none',)`` is reserved to denote the empty set
+    and therefore no other set can contain the item ``'none'``.
+
+    Args:
+        *sets (list of tuples): mutually exclusive sets of values
+    """
+
+    _empty_set = set(('none',))
+
+    def __init__(self, *sets):
+        self.sets = [set(x) for x in sets]
+
+        # 'none' is a special value and can appear only in a set of
+        # a single element
+        if any('none' in s and s != set(('none',)) for s in self.sets):
+            raise error.SpecError("The value 'none' represents the empty set,"
+                                  " and must appear alone in a set. Use the "
+                                  "method 'allow_empty_set' to add it.")
+
+        # Sets should not intersect with each other
+        if any(s1 & s2 for s1, s2 in itertools.combinations(self.sets, 2)):
+            raise error.SpecError('sets in input must be disjoint')
+
+        #: Attribute used to track values which correspond to
+        #: features which can be enabled or disabled as understood by the
+        #: package's build system.
+        self.feature_values = tuple(itertools.chain.from_iterable(self.sets))
+        self.default = None
+        self.multi = True
+        self.error_fmt = "this variant accepts combinations of values from " \
+                         "exactly one of the following sets '{values}' " \
+                         "@*r{{[{package}, variant '{variant}']}}"
+
+    def with_default(self, default):
+        """Sets the default value and returns self."""
+        self.default = default
+        return self
+
+    def with_error(self, error_fmt):
+        """Sets the error message format and returns self."""
+        self.error_fmt = error_fmt
+        return self
+
+    def with_non_feature_values(self, *values):
+        """Marks a few values as not being tied to a feature."""
+        self.feature_values = tuple(
+            x for x in self.feature_values if x not in values
+        )
+        return self
+
+    def allow_empty_set(self):
+        """Adds the empty set to the current list of disjoint sets."""
+        if self._empty_set in self.sets:
+            return self
+
+        # Create a new object to be returned
+        object_with_empty_set = type(self)(('none',), *self.sets)
+        object_with_empty_set.error_fmt = self.error_fmt
+        object_with_empty_set.feature_values = self.feature_values + ('none', )
+        return object_with_empty_set
+
+    def prohibit_empty_set(self):
+        """Removes the empty set from the current list of disjoint sets."""
+        if self._empty_set not in self.sets:
+            return self
+
+        # Create a new object to be returned
+        sets = [s for s in self.sets if s != self._empty_set]
+        object_without_empty_set = type(self)(*sets)
+        object_without_empty_set.error_fmt = self.error_fmt
+        object_without_empty_set.feature_values = tuple(
+            x for x in self.feature_values if x != 'none'
+        )
+        return object_without_empty_set
+
+    def __getitem__(self, idx):
+        return tuple(itertools.chain.from_iterable(self.sets))[idx]
+
+    def __len__(self):
+        return len(itertools.chain.from_iterable(self.sets))
+
+    @property
+    def validator(self):
+        def _disjoint_set_validator(pkg_name, variant_name, values):
+            # If for any of the sets, all the values are in it return True
+            if any(all(x in s for x in values) for s in self.sets):
+                return
+
+            format_args = {
+                'variant': variant_name, 'package': pkg_name, 'values': values
+            }
+            msg = self.error_fmt + \
+                " @*r{{[{package}, variant '{variant}']}}"
+            msg = llnl.util.tty.color.colorize(msg.format(**format_args))
+            raise error.SpecError(msg)
+        return _disjoint_set_validator
+
+
+def _a_single_value_or_a_combination(single_value, *values):
+    error = "the value '" + single_value + \
+            "' is mutually exclusive with any of the other values"
+    return DisjointSetsOfValues(
+        (single_value,), values
+    ).with_default(single_value).with_error(error).\
+        with_non_feature_values(single_value)
+
+
+# TODO: The factories below are used by package writers to set values of
+# TODO: multi-valued variants. It could be worthwhile to gather them in
+# TODO: a common namespace (like 'multi') in the future.
+
+
+def any_combination_of(*values):
+    """Multi-valued variant that allows any combination of the specified
+    values, and also allows the user to specify 'none' (as a string) to choose
+    none of them.
+
+    It is up to the package implementation to handle the value 'none'
+    specially, if at all.
+
+    Args:
+        *values: allowed variant values
+
+    Returns:
+        a properly initialized instance of DisjointSetsOfValues
+    """
+    return _a_single_value_or_a_combination('none', *values)
+
+
+def auto_or_any_combination_of(*values):
+    """Multi-valued variant that allows any combination of a set of values
+    (but not the empty set) or 'auto'.
+
+    Args:
+        *values: allowed variant values
+
+    Returns:
+        a properly initialized instance of DisjointSetsOfValues
+    """
+    return _a_single_value_or_a_combination('auto', *values)
+
+
+#: Multi-valued variant that allows any combination picking
+#: from one of multiple disjoint sets
+def disjoint_sets(*sets):
+    """Multi-valued variant that allows any combination picking from one
+    of multiple disjoint sets of values, and also allows the user to specify
+    'none' (as a string) to choose none of them.
+
+    It is up to the package implementation to handle the value 'none'
+    specially, if at all.
+
+    Args:
+        *sets:
+
+    Returns:
+        a properly initialized instance of DisjointSetsOfValues
+    """
+    return DisjointSetsOfValues(*sets).allow_empty_set().with_default('none')
+
+
 class DuplicateVariantError(error.SpecError):
     """Raised when the same variant occurs in a spec twice."""
 
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