Skip to content

Commit a60ff27

Browse files
tgamblintgamblin
authored
Merge f6ed5a8 into 67b5f6b 
2 parents 67b5f6b + f6ed5a8 commit a60ff27

File tree

25 files changed

+789
-471
lines changed

25 files changed

+789
-471
lines changed

‎lib/spack/spack/audit.py‎

Lines changed: 85 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ def _search_duplicate_compilers(error_cls):
6464
#: Map a group of checks to the list of related audit tags
6565
GROUPS = collections.defaultdict(list)
6666

67+
# TODO: turn this on (or remove the strict option entirely and make
68+
# prevalidate_variant_value always strict) when we're ready to correct packages with
69+
# invalid variant specifications, e.g. @9:+foo when foo only exists for pkg@:8, or
70+
# ~cuda~cudnn, where cudnn doesn't exist when ~cuda.
71+
strict_variants = False
72+
6773

6874
class Error:
6975
"""Information on an error reported in a test."""
@@ -315,9 +321,10 @@ def _avoid_mismatched_variants(error_cls):
315321
continue
316322

317323
# Variant cannot accept this value
318-
s = spack.spec.Spec(pkg_name)
319324
try:
320-
s.update_variant_validate(variant.name, variant.value)
325+
spack.variant.prevalidate_variant_value(
326+
pkg_cls, variant, variant.value, strict=strict_variants
327+
)
321328
except Exception:
322329
summary = (
323330
f"Setting the variant '{variant.name}' of the '{pkg_name}' package "
@@ -650,9 +657,14 @@ def _ensure_env_methods_are_ported_to_builders(pkgs, error_cls):
650657
errors = []
651658
for pkg_name in pkgs:
652659
pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name)
653-
buildsystem_variant, _ = pkg_cls.variants["build_system"]
654-
buildsystem_names = [getattr(x, "value", x) for x in buildsystem_variant.values]
660+
661+
buildsystem_names = set()
662+
build_system_variants = [vdef for _, vdef in pkg_cls.variant_definitions("build_system")]
663+
buildsystem_names = set(
664+
getattr(v, "value", v) for variant in build_system_variants for v in variant.values
665+
)
655666
builder_cls_names = [spack.builder.BUILDER_CLS[x].__name__ for x in buildsystem_names]
667+
656668
module = pkg_cls.module
657669
has_builders_in_package_py = any(
658670
getattr(module, name, False) for name in builder_cls_names
@@ -837,20 +849,26 @@ def check_virtual_with_variants(spec, msg):
837849

838850
# check variants
839851
dependency_variants = dep.spec.variants
840-
for name, value in dependency_variants.items():
852+
for name, variant in dependency_variants.items():
841853
try:
842-
v, _ = dependency_pkg_cls.variants[name]
843-
v.validate_or_raise(value, pkg_cls=dependency_pkg_cls)
854+
spack.variant.prevalidate_variant_value(
855+
dependency_pkg_cls,
856+
variant,
857+
variant.value,
858+
dep.spec,
859+
strict=strict_variants,
860+
)
844861
except Exception as e:
845862
summary = (
846863
f"{pkg_name}: wrong variant used for dependency in 'depends_on()'"
847864
)
848865

866+
error_msg = str(e)
849867
if isinstance(e, KeyError):
850868
error_msg = (
851869
f"variant {str(e).strip()} does not exist in package {dep_name}"
870+
f" in package '{dep_name}'"
852871
)
853-
error_msg += f" in package '{dep_name}'"
854872

855873
errors.append(
856874
error_cls(summary=summary, details=[error_msg, f"in {filename}"])
@@ -862,38 +880,40 @@ def check_virtual_with_variants(spec, msg):
862880
@package_directives
863881
def _ensure_variant_defaults_are_parsable(pkgs, error_cls):
864882
"""Ensures that variant defaults are present and parsable from cli"""
883+
884+
def check_variant(pkg_name, variant):
885+
default_is_parsable = (
886+
# Permitting a default that is an instance on 'int' permits
887+
# to have foo=false or foo=0. Other falsish values are
888+
# not allowed, since they can't be parsed from cli ('foo=')
889+
isinstance(variant.default, int)
890+
or variant.default
891+
)
892+
if not default_is_parsable:
893+
msg = f"Variant '{vname}' of package '{pkg_name}' has a bad default value"
894+
errors.append(error_cls(msg, []))
895+
return
896+
897+
try:
898+
vspec = variant.make_default()
899+
except spack.variant.MultipleValuesInExclusiveVariantError:
900+
msg = f"Can't create default value for variant '{vname}' in package '{pkg_name}'"
901+
errors.append(error_cls(msg, []))
902+
return
903+
904+
try:
905+
variant.validate_or_raise(vspec, pkg_cls=pkg_cls)
906+
except spack.variant.InvalidVariantValueError:
907+
msg = "Default value of variant '{vname}' in package '{pkg.name}' is invalid"
908+
question = "Is it among the allowed values?"
909+
errors.append(error_cls(msg, [question]))
910+
865911
errors = []
866912
for pkg_name in pkgs:
867913
pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name)
868-
for variant_name, entry in pkg_cls.variants.items():
869-
variant, _ = entry
870-
default_is_parsable = (
871-
# Permitting a default that is an instance on 'int' permits
872-
# to have foo=false or foo=0. Other falsish values are
873-
# not allowed, since they can't be parsed from cli ('foo=')
874-
isinstance(variant.default, int)
875-
or variant.default
876-
)
877-
if not default_is_parsable:
878-
error_msg = "Variant '{}' of package '{}' has a bad default value"
879-
errors.append(error_cls(error_msg.format(variant_name, pkg_name), []))
880-
continue
881-
882-
try:
883-
vspec = variant.make_default()
884-
except spack.variant.MultipleValuesInExclusiveVariantError:
885-
error_msg = "Cannot create a default value for the variant '{}' in package '{}'"
886-
errors.append(error_cls(error_msg.format(variant_name, pkg_name), []))
887-
continue
888-
889-
try:
890-
variant.validate_or_raise(vspec, pkg_cls=pkg_cls)
891-
except spack.variant.InvalidVariantValueError:
892-
error_msg = (
893-
"The default value of the variant '{}' in package '{}' failed validation"
894-
)
895-
question = "Is it among the allowed values?"
896-
errors.append(error_cls(error_msg.format(variant_name, pkg_name), [question]))
914+
for vname in pkg_cls.variant_names():
915+
for _, variant_def in pkg_cls.variant_definitions(vname):
916+
check_variant(pkg_cls, variant_def)
897917

898918
return errors
899919

@@ -904,11 +924,11 @@ def _ensure_variants_have_descriptions(pkgs, error_cls):
904924
errors = []
905925
for pkg_name in pkgs:
906926
pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name)
907-
for variant_name, entry in pkg_cls.variants.items():
908-
variant, _ = entry
909-
if not variant.description:
910-
error_msg = "Variant '{}' in package '{}' is missing a description"
911-
errors.append(error_cls(error_msg.format(variant_name, pkg_name), []))
927+
for name in pkg_cls.variant_names():
928+
for when, variant in pkg_cls.variant_definitions(name):
929+
if not variant.description:
930+
msg = f"Variant '{name}' in package '{pkg_name}' is missing a description"
931+
errors.append(error_cls(msg, []))
912932

913933
return errors
914934

@@ -965,29 +985,29 @@ def _version_constraints_are_satisfiable_by_some_version_in_repo(pkgs, error_cls
965985

966986

967987
def _analyze_variants_in_directive(pkg, constraint, directive, error_cls):
968-
variant_exceptions = (
969-
spack.variant.InconsistentValidationError,
970-
spack.variant.MultipleValuesInExclusiveVariantError,
971-
spack.variant.InvalidVariantValueError,
972-
KeyError,
973-
)
974988
errors = []
975-
for name, v in constraint.variants.items():
976-
try:
977-
variant, _ = pkg.variants[name]
978-
variant.validate_or_raise(v, pkg_cls=pkg)
979-
except variant_exceptions as e:
980-
summary = pkg.name + ': wrong variant in "{0}" directive'
981-
summary = summary.format(directive)
982-
filename = spack.repo.PATH.filename_for_package_name(pkg.name)
989+
variant_names = pkg.variant_names()
983990

984-
error_msg = str(e).strip()
985-
if isinstance(e, KeyError):
986-
error_msg = "the variant {0} does not exist".format(error_msg)
991+
for name, v in constraint.variants.items():
992+
summary = f"{pkg.name}: wrong variant in '{directive}' directive"
993+
filename = spack.repo.PATH.filename_for_package_name(pkg.name)
987994

988-
err = error_cls(summary=summary, details=[error_msg, "in " + filename])
995+
if name not in variant_names:
996+
msg = f"variant {name} does not exist in {pkg.name}"
997+
errors.append(error_cls(summary=summary, details=[msg, f"in {filename}"]))
998+
continue
989999

990-
errors.append(err)
1000+
try:
1001+
spack.variant.prevalidate_variant_value(
1002+
pkg, v, v.value, constraint, strict=strict_variants
1003+
)
1004+
except (
1005+
spack.variant.InconsistentValidationError,
1006+
spack.variant.MultipleValuesInExclusiveVariantError,
1007+
spack.variant.InvalidVariantValueError,
1008+
) as e:
1009+
msg = str(e).strip()
1010+
errors.append(error_cls(summary=summary, details=[msg, f"in {filename}"]))
9911011

9921012
return errors
9931013

@@ -1025,9 +1045,10 @@ def _extracts_errors(triggers, summary):
10251045
for dname in dnames
10261046
)
10271047

1028-
for vname, (variant, triggers) in pkg_cls.variants.items():
1029-
summary = f"{pkg_name}: wrong 'when=' condition for the '{vname}' variant"
1030-
errors.extend(_extracts_errors(triggers, summary))
1048+
for when, variants_by_name in pkg_cls.variants.items():
1049+
for vname, variant in variants_by_name.items():
1050+
summary = f"{pkg_name}: wrong 'when=' condition for the '{vname}' variant"
1051+
errors.extend(_extracts_errors([when], summary))
10311052

10321053
for when, providers, details in _error_items(pkg_cls.provided):
10331054
errors.extend(

‎lib/spack/spack/build_systems/autotools.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ def _activate_or_not(
690690

691691
# Defensively look that the name passed as argument is among
692692
# variants
693-
if variant not in self.pkg.variants:
693+
if not self.pkg.has_variant(variant):
694694
msg = '"{0}" is not a variant of "{1}"'
695695
raise KeyError(msg.format(variant, self.pkg.name))
696696

@@ -699,7 +699,7 @@ def _activate_or_not(
699699

700700
# Create a list of pairs. Each pair includes a configuration
701701
# option and whether or not that option is activated
702-
variant_desc, _ = self.pkg.variants[variant]
702+
variant_desc = self.pkg.variant_descriptor(variant)
703703
if set(variant_desc.values) == set((True, False)):
704704
# BoolValuedVariant carry information about a single option.
705705
# Nonetheless, for uniformity of treatment we'll package them

‎lib/spack/spack/build_systems/cached_cmake.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ def define_cmake_cache_from_variant(self, cmake_var, variant=None, comment=""):
8989
if variant is None:
9090
variant = cmake_var.lower()
9191

92-
if variant not in self.pkg.variants:
92+
if not self.pkg.has_variant(variant):
9393
raise KeyError('"{0}" is not a variant of "{1}"'.format(variant, self.pkg.name))
9494

9595
if variant not in self.pkg.spec.variants:

‎lib/spack/spack/build_systems/cmake.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ def _values(x):
141141
default=default,
142142
values=_values,
143143
description="the build system generator to use",
144+
when="build_system=cmake",
144145
)
145146
for x in not_used:
146147
conflicts(f"generator={x}")
@@ -500,7 +501,7 @@ def define_from_variant(self, cmake_var, variant=None):
500501
if variant is None:
501502
variant = cmake_var.lower()
502503

503-
if variant not in self.pkg.variants:
504+
if not self.pkg.has_variant(variant):
504505
raise KeyError('"{0}" is not a variant of "{1}"'.format(variant, self.pkg.name))
505506

506507
if variant not in self.pkg.spec.variants:

‎lib/spack/spack/cmd/audit.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ def setup_parser(subparser):
4141
# Audit package recipes
4242
pkg_parser = sp.add_parser("packages", help="audit package recipes")
4343

44+
# TODO: this should be set to true and removed as an option eventually
45+
pkg_parser.add_argument(
46+
"--strict-variants",
47+
action="store_true",
48+
default=False,
49+
help="report if variant cannot exist in context where it's referenced",
50+
)
51+
4452
for group in [pkg_parser, https_parser, external_parser]:
4553
group.add_argument(
4654
"name",
@@ -61,6 +69,7 @@ def configs(parser, args):
6169

6270

6371
def packages(parser, args):
72+
spack.audit.strict_variants = args.strict_variants
6473
pkgs = args.name or spack.repo.PATH.all_package_names()
6574
reports = spack.audit.run_group(args.subcommand, pkgs=pkgs)
6675
_process_reports(reports)

‎lib/spack/spack/cmd/config.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,10 +537,10 @@ def config_prefer_upstream(args):
537537
# Get and list all the variants that differ from the default.
538538
variants = []
539539
for var_name, variant in spec.variants.items():
540-
if var_name in ["patches"] or var_name not in spec.package.variants:
540+
if var_name in ["patches"] or not spec.package.has_variant(var_name):
541541
continue
542542

543-
variant_desc, _ = spec.package.variants[var_name]
543+
variant_desc = spec.package.variant_descriptor(var_name)
544544
if variant.value != variant_desc.default:
545545
variants.append(str(variant))
546546
variants.sort()

‎lib/spack/spack/cmd/info.py‎

Lines changed: 10 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -323,26 +323,6 @@ def _fmt_variant(variant, max_name_default_len, indent, when=None, out=None):
323323
out.write("\n")
324324

325325

326-
def _variants_by_name_when(pkg):
327-
"""Adaptor to get variants keyed by { name: { when: { [Variant...] } }."""
328-
# TODO: replace with pkg.variants_by_name(when=True) when unified directive dicts are merged.
329-
variants = {}
330-
for name, (variant, whens) in sorted(pkg.variants.items()):
331-
for when in whens:
332-
variants.setdefault(name, {}).setdefault(when, []).append(variant)
333-
return variants
334-
335-
336-
def _variants_by_when_name(pkg):
337-
"""Adaptor to get variants keyed by { when: { name: Variant } }"""
338-
# TODO: replace with pkg.variants when unified directive dicts are merged.
339-
variants = {}
340-
for name, (variant, whens) in pkg.variants.items():
341-
for when in whens:
342-
variants.setdefault(when, {})[name] = variant
343-
return variants
344-
345-
346326
def _print_variants_header(pkg):
347327
"""output variants"""
348328

@@ -353,32 +333,22 @@ def _print_variants_header(pkg):
353333
color.cprint("")
354334
color.cprint(section_title("Variants:"))
355335

356-
variants_by_name = _variants_by_name_when(pkg)
357-
358336
# Calculate the max length of the "name [default]" part of the variant display
359337
# This lets us know where to print variant values.
360338
max_name_default_len = max(
361339
color.clen(_fmt_name_and_default(variant))
362-
for name, when_variants in variants_by_name.items()
363-
for variants in when_variants.values()
364-
for variant in variants
340+
for name in pkg.variant_names()
341+
for _, variant in pkg.variant_definitions(name)
365342
)
366343

367-
return max_name_default_len, variants_by_name
368-
369-
370-
def _unconstrained_ver_first(item):
371-
"""sort key that puts specs with open version ranges first"""
372-
spec, _ = item
373-
return (spack.version.any_version not in spec.versions, spec)
344+
return max_name_default_len
374345

375346

376347
def print_variants_grouped_by_when(pkg):
377-
max_name_default_len, _ = _print_variants_header(pkg)
348+
max_name_default_len = _print_variants_header(pkg)
378349

379350
indent = 4
380-
variants = _variants_by_when_name(pkg)
381-
for when, variants_by_name in sorted(variants.items(), key=_unconstrained_ver_first):
351+
for when, variants_by_name in pkg.variants.items():
382352
padded_values = max_name_default_len + 4
383353
start_indent = indent
384354

@@ -396,15 +366,14 @@ def print_variants_grouped_by_when(pkg):
396366

397367

398368
def print_variants_by_name(pkg):
399-
max_name_default_len, variants_by_name = _print_variants_header(pkg)
369+
max_name_default_len = _print_variants_header(pkg)
400370
max_name_default_len += 4
401371

402372
indent = 4
403-
for name, when_variants in variants_by_name.items():
404-
for when, variants in sorted(when_variants.items(), key=_unconstrained_ver_first):
405-
for variant in variants:
406-
_fmt_variant(variant, max_name_default_len, indent, when, out=sys.stdout)
407-
sys.stdout.write("\n")
373+
for name in pkg.variant_names():
374+
for when, variant in pkg.variant_definitions(name):
375+
_fmt_variant(variant, max_name_default_len, indent, when, out=sys.stdout)
376+
sys.stdout.write("\n")
408377

409378

410379
def print_variants(pkg, args):

0 commit comments

Comments
 (0)