variant.py: make Variant.default is str | bool | tuple[str] (#49836)

Warn if variant default is not among those types

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"
+                msg = "either a default was not explicitly set, or 'None' was used"
-            elif default == "":
+            else:
                 msg = "the default cannot be an empty string"
             raise DirectiveError(format_error(msg, pkg))
 

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
 

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")

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")

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",

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(

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"

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,
     )

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(

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",

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")

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",

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",
     )
 

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",
     )
 

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",
     )
 

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",
     )

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,
     )

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"))
 

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")

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.",
     )

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,
     )

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",
         )

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",