From e7f4c2b49e38ceecb895b4ad6885ceea377a9737 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 15 Dec 2020 10:22:15 +0100 Subject: [PATCH] package sanity: ensure all variant defaults are allowed values (#20373) --- lib/spack/spack/build_systems/cuda.py | 4 ++-- lib/spack/spack/test/package_sanity.py | 14 +++++++++++++- lib/spack/spack/variant.py | 3 +-- .../builtin/packages/cbtf-argonavis/package.py | 2 +- .../repos/builtin/packages/cbtf-krell/package.py | 2 +- .../repos/builtin/packages/cbtf-lanl/package.py | 2 +- var/spack/repos/builtin/packages/cbtf/package.py | 2 +- var/spack/repos/builtin/packages/elsi/package.py | 2 +- .../repos/builtin/packages/fairlogger/package.py | 2 +- .../repos/builtin/packages/gpu-burn/package.py | 2 +- var/spack/repos/builtin/packages/hdf5/package.py | 2 +- var/spack/repos/builtin/packages/jube/package.py | 3 ++- .../builtin/packages/kokkos-legacy/package.py | 5 ++--- var/spack/repos/builtin/packages/kokkos/package.py | 6 +++--- .../repos/builtin/packages/libbeagle/package.py | 2 +- .../packages/openspeedshop-utils/package.py | 2 +- .../builtin/packages/openspeedshop/package.py | 2 +- var/spack/repos/builtin/packages/rr/package.py | 2 +- var/spack/repos/builtin/packages/vtk-m/package.py | 2 +- 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.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 @@ def isa_type(v): 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 0714d407e4..b368089b9b 100644 --- a/var/spack/repos/builtin/packages/vtk-m/package.py +++ b/var/spack/repos/builtin/packages/vtk-m/package.py @@ -67,7 +67,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