diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index a70dbdb7b30..2cff2678a04 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -34,11 +34,13 @@ class OpenMpi(Package): import collections.abc import os import re +import warnings from typing import Any, Callable, List, Optional, Tuple, Type, Union import llnl.util.tty.color import spack.deptypes as dt +import spack.error import spack.fetch_strategy import spack.package_base import spack.patch @@ -620,7 +622,7 @@ def conditional(*values: List[Any], when: Optional[WhenType] = None): @directive("variants") def variant( name: str, - default: Optional[Any] = None, + default: Optional[Union[bool, str, Tuple[str, ...]]] = None, description: str = "", values: Optional[Union[collections.abc.Sequence, Callable[[Any], bool]]] = None, multi: Optional[bool] = None, @@ -650,6 +652,24 @@ def variant( DirectiveError: If arguments passed to the directive are invalid """ + # This validation can be removed at runtime and enforced with an audit in Spack v1.0. + # For now it's a warning to let people migrate faster. + if not ( + default is None + or type(default) in (bool, str) + or (type(default) is tuple and all(type(x) is str for x in default)) + ): + if isinstance(default, (list, tuple)): + did_you_mean = f"default={','.join(str(x) for x in default)!r}" + else: + did_you_mean = f"default={str(default)!r}" + warnings.warn( + f"default value for variant '{name}' is not a boolean or string: default={default!r}. " + f"Did you mean {did_you_mean}?", + stacklevel=3, + category=spack.error.SpackAPIWarning, + ) + def format_error(msg, pkg): msg += " @*r{{[{0}, variant '{1}']}}" return llnl.util.tty.color.colorize(msg.format(pkg.name, name)) @@ -665,7 +685,11 @@ def _raise_reserved_name(pkg): # Ensure we have a sequence of allowed variant values, or a # predicate for it. if values is None: - if str(default).upper() in ("TRUE", "FALSE"): + if ( + default in (True, False) + or type(default) is str + and default.upper() in ("TRUE", "FALSE") + ): values = (True, False) else: values = lambda x: True @@ -698,12 +722,15 @@ def _raise_argument_error(pkg): # 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 isinstance(default, tuple): + default = ",".join(default) + 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 = "either a default was not explicitly set, or 'None' was used" + else: msg = "the default cannot be an empty string" raise DirectiveError(format_error(msg, pkg)) diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index 991dc49c8e4..5fa2251f127 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -64,7 +64,7 @@ class Variant: """ name: str - default: Any + default: Union[bool, str] description: str values: Optional[Collection] #: if None, valid values are defined only by validators multi: bool @@ -77,7 +77,7 @@ def __init__( self, name: str, *, - default: Any, + default: Union[bool, str], description: str, values: Union[Collection, Callable] = (True, False), multi: bool = False, @@ -200,7 +200,7 @@ def make_default(self): """ return self.make_variant(self.default) - def make_variant(self, value) -> "AbstractVariant": + def make_variant(self, value: Union[str, bool]) -> "AbstractVariant": """Factory that creates a variant holding the value passed as a parameter. @@ -298,7 +298,7 @@ class AbstractVariant: _value: ValueType _original_value: Any - def __init__(self, name: str, value: Any, propagate: bool = False): + def __init__(self, name: str, value: ValueType, propagate: bool = False) -> None: self.name = name self.propagate = propagate diff --git a/var/spack/repos/builtin/packages/abyss/package.py b/var/spack/repos/builtin/packages/abyss/package.py index 54a42624e9f..d84beefe483 100644 --- a/var/spack/repos/builtin/packages/abyss/package.py +++ b/var/spack/repos/builtin/packages/abyss/package.py @@ -37,7 +37,7 @@ class Abyss(AutotoolsPackage): version("1.5.2", sha256="8a52387f963afb7b63db4c9b81c053ed83956ea0a3981edcad554a895adf84b1") variant( - "maxk", default=128, values=is_multiple_32, description="set the maximum k-mer length." + "maxk", default="128", values=is_multiple_32, description="set the maximum k-mer length." ) depends_on("c", type="build") diff --git a/var/spack/repos/builtin/packages/asagi/package.py b/var/spack/repos/builtin/packages/asagi/package.py index 862ee409217..b64e8c3f3ff 100644 --- a/var/spack/repos/builtin/packages/asagi/package.py +++ b/var/spack/repos/builtin/packages/asagi/package.py @@ -32,7 +32,7 @@ class Asagi(CMakePackage): ) variant("fortran", default=True, description="enable fortran support") - variant("max_dimensions", default=4, description="max. number of dimensions supported") + variant("max_dimensions", default="4", description="max. number of dimensions supported") variant("numa", default=True, description="enable NUMA support") variant("mpi", default=True, description="enable MPI") variant("threadsafe", default=True, description="enable threadsafe ASAGI-functions") diff --git a/var/spack/repos/builtin/packages/athena/package.py b/var/spack/repos/builtin/packages/athena/package.py index 53ed85e83d1..e7879274c49 100644 --- a/var/spack/repos/builtin/packages/athena/package.py +++ b/var/spack/repos/builtin/packages/athena/package.py @@ -85,7 +85,7 @@ class Athena(AutotoolsPackage): description="Equation of state", values=["adiabatic", "isothermal"], ) - variant("nscalars", default=0, description="Number of advected scalars") + variant("nscalars", default="0", description="Number of advected scalars") variant( "gravity", default="none", diff --git a/var/spack/repos/builtin/packages/atlas/package.py b/var/spack/repos/builtin/packages/atlas/package.py index 2691f67d911..24c4ba9cf23 100644 --- a/var/spack/repos/builtin/packages/atlas/package.py +++ b/var/spack/repos/builtin/packages/atlas/package.py @@ -59,9 +59,9 @@ class Atlas(Package): variant( "tune_cpu", - default=-1, + default="-1", multi=False, - description="Number of threads to tune to, " "-1 for autodetect, 0 for no threading", + description="Number of threads to tune to, -1 for autodetect, 0 for no threading", ) conflicts( diff --git a/var/spack/repos/builtin/packages/ccs-qcd/package.py b/var/spack/repos/builtin/packages/ccs-qcd/package.py index d038525dc9a..c8a2f900a3c 100644 --- a/var/spack/repos/builtin/packages/ccs-qcd/package.py +++ b/var/spack/repos/builtin/packages/ccs-qcd/package.py @@ -41,7 +41,7 @@ class CcsQcd(MakefilePackage): variant( "class", - default=1, + default="1", values=class_validator, description="This miniapp has five problem classes, for which the" " first three are relatively small problems just for testing" diff --git a/var/spack/repos/builtin/packages/fenics-dolfinx/package.py b/var/spack/repos/builtin/packages/fenics-dolfinx/package.py index db928629779..9836f23bd14 100644 --- a/var/spack/repos/builtin/packages/fenics-dolfinx/package.py +++ b/var/spack/repos/builtin/packages/fenics-dolfinx/package.py @@ -25,7 +25,7 @@ class FenicsDolfinx(CMakePackage): variant( "partitioners", description="Graph partioning", - default=("parmetis",), + default="parmetis", values=("kahip", "parmetis", "scotch"), multi=True, ) diff --git a/var/spack/repos/builtin/packages/g2/package.py b/var/spack/repos/builtin/packages/g2/package.py index eb02f22aa57..627ce4b3d3d 100644 --- a/var/spack/repos/builtin/packages/g2/package.py +++ b/var/spack/repos/builtin/packages/g2/package.py @@ -38,7 +38,7 @@ class G2(CMakePackage): when="@3.4.6:", ) variant("w3emc", default=True, description="Enable GRIB1 through w3emc", when="@3.4.6:") - variant("shared", default="False", description="Build shared library", when="@3.4.7:") + variant("shared", default=False, description="Build shared library", when="@3.4.7:") variant("openmp", default=False, description="Use OpenMP multithreading", when="@develop") variant("utils", default=False, description="Build grib utilities", when="@develop") variant( diff --git a/var/spack/repos/builtin/packages/g2c/package.py b/var/spack/repos/builtin/packages/g2c/package.py index bc56586e0e5..658664a1c9a 100644 --- a/var/spack/repos/builtin/packages/g2c/package.py +++ b/var/spack/repos/builtin/packages/g2c/package.py @@ -31,7 +31,7 @@ class G2c(CMakePackage): variant("pic", default=True, description="Build with position-independent-code") variant( "libs", - default=("shared", "static"), + default="shared,static", values=("shared", "static"), multi=True, description="Build shared libs, static libs or both", diff --git a/var/spack/repos/builtin/packages/legion/package.py b/var/spack/repos/builtin/packages/legion/package.py index 07dbc08f22e..174a5e35c42 100644 --- a/var/spack/repos/builtin/packages/legion/package.py +++ b/var/spack/repos/builtin/packages/legion/package.py @@ -344,19 +344,19 @@ def validate_gasnet_root(value): variant( "max_dims", values=int, - default=3, + default="3", description="Set max number of dimensions for logical regions.", ) variant( "max_fields", values=int, - default=512, + default="512", description="Maximum number of fields allowed in a logical region.", ) variant( "max_num_nodes", values=int, - default=1024, + default="1024", description="Maximum number of nodes supported by Legion.", ) variant("prof", default=False, description="Install Rust Legion prof") diff --git a/var/spack/repos/builtin/packages/libjpeg-turbo/package.py b/var/spack/repos/builtin/packages/libjpeg-turbo/package.py index d4a062c9f04..33f3adb3d5d 100644 --- a/var/spack/repos/builtin/packages/libjpeg-turbo/package.py +++ b/var/spack/repos/builtin/packages/libjpeg-turbo/package.py @@ -68,7 +68,7 @@ class LibjpegTurbo(CMakePackage, AutotoolsPackage): variant( "libs", - default=("shared", "static"), + default="shared,static", values=("shared", "static"), multi=True, description="Build shared libs, static libs, or both", diff --git a/var/spack/repos/builtin/packages/mpi-serial/package.py b/var/spack/repos/builtin/packages/mpi-serial/package.py index 4fcfd832f34..1226a16446d 100644 --- a/var/spack/repos/builtin/packages/mpi-serial/package.py +++ b/var/spack/repos/builtin/packages/mpi-serial/package.py @@ -24,14 +24,14 @@ class MpiSerial(AutotoolsPackage): variant( "fort-real-size", values=int, - default=4, + default="4", description="Specify the size of Fortran real variables", ) variant( "fort-double-size", values=int, - default=8, + default="8", description="Specify the size of Fortran double precision variables", ) diff --git a/var/spack/repos/builtin/packages/mpileaks/package.py b/var/spack/repos/builtin/packages/mpileaks/package.py index 4ef58676b03..9557b9785bc 100644 --- a/var/spack/repos/builtin/packages/mpileaks/package.py +++ b/var/spack/repos/builtin/packages/mpileaks/package.py @@ -17,7 +17,7 @@ class Mpileaks(AutotoolsPackage): variant( "stackstart", values=int, - default=0, + default="0", description="Specify the number of stack frames to truncate", ) diff --git a/var/spack/repos/builtin/packages/mpip/package.py b/var/spack/repos/builtin/packages/mpip/package.py index 8eefc0b0ed6..21d259b0a79 100644 --- a/var/spack/repos/builtin/packages/mpip/package.py +++ b/var/spack/repos/builtin/packages/mpip/package.py @@ -35,18 +35,21 @@ class Mpip(AutotoolsPackage): variant( "maxargs", values=int, - default=32, + default="32", description="Set number of command line arguments in report", ) variant( - "stackdepth", values=int, default=8, description="Specify maximum report stacktrace depth" + "stackdepth", + values=int, + default="8", + description="Specify maximum report stacktrace depth", ) variant( "internal_stackdepth", values=int, - default=3, + default="3", description="Specify number of internal stack frames", ) diff --git a/var/spack/repos/builtin/packages/nalu-wind/package.py b/var/spack/repos/builtin/packages/nalu-wind/package.py index 4b592f48e63..7b66312a967 100644 --- a/var/spack/repos/builtin/packages/nalu-wind/package.py +++ b/var/spack/repos/builtin/packages/nalu-wind/package.py @@ -43,13 +43,13 @@ class NaluWind(CMakePackage, CudaPackage, ROCmPackage): variant("pic", default=True, description="Position independent code") variant( "abs_tol", - default=1.0e-15, + default="1.0e-15", values=_parse_float, description="Absolute tolerance for regression tests", ) variant( "rel_tol", - default=1.0e-12, + default="1.0e-12", values=_parse_float, description="Relative tolerance for regression tests", ) diff --git a/var/spack/repos/builtin/packages/nektools/package.py b/var/spack/repos/builtin/packages/nektools/package.py index e8af839a258..1ddb0a40ca8 100644 --- a/var/spack/repos/builtin/packages/nektools/package.py +++ b/var/spack/repos/builtin/packages/nektools/package.py @@ -42,7 +42,7 @@ class Nektools(Package): # Variant for MAXNEL, we need to read this from user variant( "MAXNEL", - default=150000, + default="150000", description="Maximum number of elements for Nek5000 tools.", values=is_integral, ) diff --git a/var/spack/repos/builtin/packages/openloops/package.py b/var/spack/repos/builtin/packages/openloops/package.py index ce87fd30546..da83a4e056c 100644 --- a/var/spack/repos/builtin/packages/openloops/package.py +++ b/var/spack/repos/builtin/packages/openloops/package.py @@ -247,7 +247,7 @@ class Openloops(Package): description="Number of parallel jobs to run. " + "Set to 1 if compiling a large number" + "of processes (e.g. lcg.coll)", - default=0, + default="0", ) depends_on("python", type=("build", "run")) diff --git a/var/spack/repos/builtin/packages/psblas/package.py b/var/spack/repos/builtin/packages/psblas/package.py index 79eb025908a..6baa8e76d6a 100644 --- a/var/spack/repos/builtin/packages/psblas/package.py +++ b/var/spack/repos/builtin/packages/psblas/package.py @@ -37,9 +37,11 @@ class Psblas(AutotoolsPackage): # Variants: # LPK/IPK: Integer precision variants - variant("LPK", default=8, values=int, description="Length in bytes for long integers (8 or 4)") variant( - "IPK", default=4, values=int, description="Length in bytes for short integers (8 or 4)" + "LPK", default="8", values=int, description="Length in bytes for long integers (8 or 4)" + ) + variant( + "IPK", default="4", values=int, description="Length in bytes for short integers (8 or 4)" ) # MPI variant("mpi", default=True, description="Activates MPI support") diff --git a/var/spack/repos/builtin/packages/pythia6/package.py b/var/spack/repos/builtin/packages/pythia6/package.py index e8c06635dc5..472000b759b 100644 --- a/var/spack/repos/builtin/packages/pythia6/package.py +++ b/var/spack/repos/builtin/packages/pythia6/package.py @@ -49,7 +49,7 @@ class Pythia6(CMakePackage): # intended to be used with other code with different requirements. variant( "nmxhep", - default=4000, + default="4000", values=_is_integral, description="Extent of particle arrays in the /HEPEVT/ COMMON block.", ) diff --git a/var/spack/repos/builtin/packages/qthreads/package.py b/var/spack/repos/builtin/packages/qthreads/package.py index b7945ee440e..4e49b992a55 100644 --- a/var/spack/repos/builtin/packages/qthreads/package.py +++ b/var/spack/repos/builtin/packages/qthreads/package.py @@ -59,7 +59,7 @@ class Qthreads(AutotoolsPackage): variant("static", default=True, description="Build static library") variant( "stack_size", - default=4096, + default="4096", description="Specify number of bytes to use in a stack", values=is_integer, ) diff --git a/var/spack/repos/builtin/packages/quda/package.py b/var/spack/repos/builtin/packages/quda/package.py index 16f3b271621..102020792aa 100644 --- a/var/spack/repos/builtin/packages/quda/package.py +++ b/var/spack/repos/builtin/packages/quda/package.py @@ -88,7 +88,7 @@ class Quda(CMakePackage, CudaPackage, ROCmPackage): with when("+multigrid"): variant( "mg_mrhs_list", - default=16, + default="16", multi=True, description="The list of multi-rhs sizes that get compiled", ) diff --git a/var/spack/repos/builtin/packages/soci/package.py b/var/spack/repos/builtin/packages/soci/package.py index cbe0bc3de2e..fd8a34c9d57 100644 --- a/var/spack/repos/builtin/packages/soci/package.py +++ b/var/spack/repos/builtin/packages/soci/package.py @@ -29,7 +29,7 @@ class Soci(CMakePackage): variant( "cxxstd", - default=11, + default="11", values=("98", "11", "14", "17", "20"), multi=False, description="Use the specified C++ standard when building",