package sanity: ensure all variant defaults are allowed values (#20373)

19 files changed, 36 insertions, 25 deletions

diff --git a/lib/spack/spack/build_systems/cuda.py b/lib/spack/spack/build_systems/cuda.py
index ed574260e7..61007431a4 100644
--- a/lib/spack/spack/build_systems/cuda.py
+++ b/lib/spack/spack/build_systems/cuda.py
@@ -19,7 +19,7 @@ class CudaPackage(PackageBase):
     # https://docs.nvidia.com/cuda/cuda-compiler-driver-nvcc/index.html#gpu-feature-list
     # https://developer.nvidia.com/cuda-gpus
     # https://en.wikipedia.org/wiki/CUDA#GPUs_supported
-    cuda_arch_values = [
+    cuda_arch_values = (
         '10', '11', '12', '13',
         '20', '21',
         '30', '32', '35', '37',
@@ -27,7 +27,7 @@ class CudaPackage(PackageBase):
         '60', '61', '62',
         '70', '72', '75',
         '80', '86'
-    ]
+    )
 
     # FIXME: keep cuda and cuda_arch separate to make usage easier until
     # Spack has depends_on(cuda, when='cuda_arch!=None') or alike
diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py
index 5c18544a56..d50169a1a4 100644
--- a/lib/spack/spack/test/package_sanity.py
+++ b/lib/spack/spack/test/package_sanity.py
@@ -2,7 +2,6 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
 """This test does sanity checks on Spack's builtin package database."""
 import os.path
 import re
@@ -14,6 +13,7 @@ import spack.package
 import spack.paths
 import spack.repo
 import spack.util.executable as executable
+import spack.variant
 # A few functions from this module are used to
 # do sanity checks only on packagess modified by a PR
 import spack.cmd.flake8 as flake8
@@ -257,3 +257,15 @@ def test_variant_defaults_are_parsable_from_cli():
             if not default_is_parsable:
                 failing.append((pkg.name, variant_name))
     assert not failing
+
+
+def test_variant_defaults_listed_explicitly_in_values():
+    failing = []
+    for pkg in spack.repo.path.all_packages():
+        for variant_name, variant in pkg.variants.items():
+            vspec = variant.make_default()
+            try:
+                variant.validate_or_raise(vspec, pkg=pkg)
+            except spack.variant.InvalidVariantValueError:
+                failing.append((pkg.name, variant.name))
+    assert not failing
diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py
index e50c34a07d..8fa52f9738 100644
--- a/lib/spack/spack/variant.py
+++ b/lib/spack/spack/variant.py
@@ -82,8 +82,7 @@ class Variant(object):
         else:
             # Otherwise assume values is the set of allowed explicit values
             self.values = values
-            allowed = tuple(self.values) + (self.default,)
-            self.single_value_validator = lambda x: x in allowed
+            self.single_value_validator = lambda x: x in tuple(self.values)
 
         self.multi = multi
         self.group_validator = validator
diff --git a/var/spack/repos/builtin/packages/cbtf-argonavis/package.py b/var/spack/repos/builtin/packages/cbtf-argonavis/package.py
index 45fa4e09d3..371d2eb809 100644
--- a/var/spack/repos/builtin/packages/cbtf-argonavis/package.py
+++ b/var/spack/repos/builtin/packages/cbtf-argonavis/package.py
@@ -27,7 +27,7 @@ class CbtfArgonavis(CMakePackage):
                          to point to target build.")
     variant('runtime', default=False,
             description="build only the runtime libraries and collectors.")
-    variant('build_type', default='None', values=('None'),
+    variant('build_type', default='None', values=('None',),
             description='CMake build type')
 
     depends_on("cmake@3.0.2:", type='build')
diff --git a/var/spack/repos/builtin/packages/cbtf-krell/package.py b/var/spack/repos/builtin/packages/cbtf-krell/package.py
index fb43c22f5c..ea238bc439 100644
--- a/var/spack/repos/builtin/packages/cbtf-krell/package.py
+++ b/var/spack/repos/builtin/packages/cbtf-krell/package.py
@@ -39,7 +39,7 @@ class CbtfKrell(CMakePackage):
             description="Build mpi experiment collector for mpich MPI.")
     variant('runtime', default=False,
             description="build only the runtime libraries and collectors.")
-    variant('build_type', default='None', values=('None'),
+    variant('build_type', default='None', values=('None',),
             description='CMake build type')
     variant('cti', default=False,
             description="Build MRNet with the CTI startup option")
diff --git a/var/spack/repos/builtin/packages/cbtf-lanl/package.py b/var/spack/repos/builtin/packages/cbtf-lanl/package.py
index 719ddc45ff..857ef6c7f9 100644
--- a/var/spack/repos/builtin/packages/cbtf-lanl/package.py
+++ b/var/spack/repos/builtin/packages/cbtf-lanl/package.py
@@ -20,7 +20,7 @@ class CbtfLanl(CMakePackage):
     version('1.9.1.1', branch='1.9.1.1')
     version('1.9.1.0', branch='1.9.1.0')
 
-    variant('build_type', default='None', values=('None'),
+    variant('build_type', default='None', values=('None',),
             description='CMake build type')
 
     variant('runtime', default=False,
diff --git a/var/spack/repos/builtin/packages/cbtf/package.py b/var/spack/repos/builtin/packages/cbtf/package.py
index eca4263858..4c64732166 100644
--- a/var/spack/repos/builtin/packages/cbtf/package.py
+++ b/var/spack/repos/builtin/packages/cbtf/package.py
@@ -29,7 +29,7 @@ class Cbtf(CMakePackage):
     variant('runtime', default=False,
             description="build only the runtime libraries and collectors.")
 
-    variant('build_type', default='None', values=('None'),
+    variant('build_type', default='None', values=('None',),
             description='CMake build type')
 
     depends_on("cmake@3.0.2:", type='build')
diff --git a/var/spack/repos/builtin/packages/elsi/package.py b/var/spack/repos/builtin/packages/elsi/package.py
index 578bc73c91..f800b57943 100644
--- a/var/spack/repos/builtin/packages/elsi/package.py
+++ b/var/spack/repos/builtin/packages/elsi/package.py
@@ -25,7 +25,7 @@ class Elsi(CMakePackage):
     )
     variant(
         'elpa2_kernel', default="none", description="ELPA2 Kernel",
-        values=('AVX', 'AVX2', 'AVX512'), multi=False
+        values=('none', 'AVX', 'AVX2', 'AVX512'), multi=False
     )
     variant(
         'enable_pexsi', default=False, description='Enable PEXSI support'
diff --git a/var/spack/repos/builtin/packages/fairlogger/package.py b/var/spack/repos/builtin/packages/fairlogger/package.py
index d90a601ddb..08108f557a 100644
--- a/var/spack/repos/builtin/packages/fairlogger/package.py
+++ b/var/spack/repos/builtin/packages/fairlogger/package.py
@@ -36,7 +36,7 @@ class Fairlogger(CMakePackage):
             multi=False,
             description='CMake build type')
     variant('cxxstd', default='default',
-            values=('11', '14', '17'),
+            values=('default', '11', '14', '17'),
             multi=False,
             description='Use the specified C++ standard when building.')
     variant('pretty',
diff --git a/var/spack/repos/builtin/packages/gpu-burn/package.py b/var/spack/repos/builtin/packages/gpu-burn/package.py
index 47bef447ea..2d367ad3fe 100644
--- a/var/spack/repos/builtin/packages/gpu-burn/package.py
+++ b/var/spack/repos/builtin/packages/gpu-burn/package.py
@@ -31,7 +31,7 @@ class GpuBurn(MakefilePackage, CudaPackage):
         'cuda_arch',
         description='CUDA architecture',
         default='none',
-        values=cuda_arch_values,
+        values=('none',) + cuda_arch_values,
         multi=False
     )
 
diff --git a/var/spack/repos/builtin/packages/hdf5/package.py b/var/spack/repos/builtin/packages/hdf5/package.py
index 97ce271f79..abad1666bb 100644
--- a/var/spack/repos/builtin/packages/hdf5/package.py
+++ b/var/spack/repos/builtin/packages/hdf5/package.py
@@ -67,7 +67,7 @@ class Hdf5(AutotoolsPackage):
     variant('pic', default=True,
             description='Produce position-independent code (for shared libs)')
     # Build HDF5 with API compaitibility.
-    variant('api', default='none', description='choose api compatibility', values=('v114', 'v112', 'v110', 'v18', 'v16'), multi=False)
+    variant('api', default='none', description='choose api compatibility', values=('none', 'v114', 'v112', 'v110', 'v18', 'v16'), multi=False)
 
     conflicts('api=v114', when='@1.6:1.12.99', msg='v114 is not compatible with this release')
     conflicts('api=v112', when='@1.6:1.10.99', msg='v112 is not compatible with this release')
diff --git a/var/spack/repos/builtin/packages/jube/package.py b/var/spack/repos/builtin/packages/jube/package.py
index aff47bc55e..edbd6fc151 100644
--- a/var/spack/repos/builtin/packages/jube/package.py
+++ b/var/spack/repos/builtin/packages/jube/package.py
@@ -30,7 +30,8 @@ class Jube(PythonPackage):
     variant(
         'resource_manager', default='none',
         description='Select resource manager templates',
-        values=('loadleveler', 'lsf', 'moab', 'pbs', 'slurm'), multi=False
+        values=('none', 'loadleveler', 'lsf', 'moab', 'pbs', 'slurm'),
+        multi=False
     )
 
     depends_on('py-setuptools', type='build')
diff --git a/var/spack/repos/builtin/packages/kokkos-legacy/package.py b/var/spack/repos/builtin/packages/kokkos-legacy/package.py
index 3195e06fd6..4b569967a6 100644
--- a/var/spack/repos/builtin/packages/kokkos-legacy/package.py
+++ b/var/spack/repos/builtin/packages/kokkos-legacy/package.py
@@ -77,9 +77,8 @@ class KokkosLegacy(Package):
                   'Volta70', 'Volta72')
 
     # C++ standard variant
-    variant('cxxstd', default='none',
-            values=('c++11', 'c++14', 'c++17', 'c++1y', 'c++1z', 'c++2a'),
-            multi=False,
+    cxx_stds = ('none', 'c++11', 'c++14', 'c++17', 'c++1y', 'c++1z', 'c++2a')
+    variant('cxxstd', default='none', values=cxx_stds, multi=False,
             description='set cxxstandard Kokkos option')
 
     # Host architecture variant
diff --git a/var/spack/repos/builtin/packages/kokkos/package.py b/var/spack/repos/builtin/packages/kokkos/package.py
index ff666dbe0b..b3fdca7821 100644
--- a/var/spack/repos/builtin/packages/kokkos/package.py
+++ b/var/spack/repos/builtin/packages/kokkos/package.py
@@ -72,13 +72,13 @@ class Kokkos(CMakePackage, CudaPackage):
         'tests': [False, 'Build for tests'],
     }
 
-    amd_gpu_arches = [
+    amd_gpu_arches = (
         'fiji',
         'gfx901',
         'vega900',
         'vega906',
-    ]
-    variant("amd_gpu_arch", default='none', values=amd_gpu_arches,
+    )
+    variant("amd_gpu_arch", default='none', values=('none',) + amd_gpu_arches,
             description="AMD GPU architecture")
     conflicts("+hip", when="amd_gpu_arch=none")
 
diff --git a/var/spack/repos/builtin/packages/libbeagle/package.py b/var/spack/repos/builtin/packages/libbeagle/package.py
index 9f06d01001..871dc5d2c6 100644
--- a/var/spack/repos/builtin/packages/libbeagle/package.py
+++ b/var/spack/repos/builtin/packages/libbeagle/package.py
@@ -31,7 +31,7 @@ class Libbeagle(AutotoolsPackage, CudaPackage):
         'cuda_arch',
         description='CUDA architecture',
         default='none',
-        values=cuda_arch_values,
+        values=('none',) + cuda_arch_values,
         multi=False
     )
     conflicts('cuda_arch=none', when='+cuda',
diff --git a/var/spack/repos/builtin/packages/openspeedshop-utils/package.py b/var/spack/repos/builtin/packages/openspeedshop-utils/package.py
index 33bd293fa2..6b3645148e 100644
--- a/var/spack/repos/builtin/packages/openspeedshop-utils/package.py
+++ b/var/spack/repos/builtin/packages/openspeedshop-utils/package.py
@@ -51,7 +51,7 @@ class OpenspeedshopUtils(CMakePackage):
     variant('cuda', default=False,
             description="build with cuda packages included.")
 
-    variant('build_type', default='None', values=('None'),
+    variant('build_type', default='None', values=('None',),
             description='CMake build type')
 
     # MPI variants
diff --git a/var/spack/repos/builtin/packages/openspeedshop/package.py b/var/spack/repos/builtin/packages/openspeedshop/package.py
index a0cf6e46cd..adb1d59ca0 100644
--- a/var/spack/repos/builtin/packages/openspeedshop/package.py
+++ b/var/spack/repos/builtin/packages/openspeedshop/package.py
@@ -46,7 +46,7 @@ class Openspeedshop(CMakePackage):
     variant('gui', default='qt3', values=('none', 'qt3', 'qt4'),
             description='Build or not build a GUI of choice')
 
-    variant('build_type', default='None', values=('None'),
+    variant('build_type', default='None', values=('None',),
             description='CMake build type')
 
     # MPI variants
diff --git a/var/spack/repos/builtin/packages/rr/package.py b/var/spack/repos/builtin/packages/rr/package.py
index 6b3bb93a51..f96d336533 100644
--- a/var/spack/repos/builtin/packages/rr/package.py
+++ b/var/spack/repos/builtin/packages/rr/package.py
@@ -29,7 +29,7 @@ class Rr(CMakePackage):
     # Only 'Release' is supported at the moment
     variant('build_type', default='Release',
             description='The build type to build',
-            values=('Release'))
+            values=('Release',))
 
     def patch(self):
         # because otherwise CMake would try and fail to set RPATH of
diff --git a/var/spack/repos/builtin/packages/vtk-m/package.py b/var/spack/repos/builtin/packages/vtk-m/package.py
index 7c31160df7..06d38cfc28 100644
--- a/var/spack/repos/builtin/packages/vtk-m/package.py
+++ b/var/spack/repos/builtin/packages/vtk-m/package.py
@@ -66,7 +66,7 @@ class VtkM(CMakePackage, CudaPackage):
         'gfx908': 'vega908'
     }
 
-    variant('amdgpu_target', default='none', multi=True, values=amdgpu_targets)
+    variant('amdgpu_target', default='none', multi=True, values=('none',) + amdgpu_targets)
     conflicts("+hip", when="amdgpu_target=none")
 
     depends_on("cmake@3.12:", type="build")         # CMake >= 3.12