From 1343a815c02421fb94ddb2b80af76d35760bf4c4 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sat, 5 Dec 2020 21:22:20 -0800 Subject: concretizer: refactor handling of special variants dev_build and patches Other parts of the concretizer code build up lists of things we can't know without traversing all specs and packages, and they output these list at the very end. The code for this for variant values from spec literals was intertwined with the code for traversing the input specs. This only covers the input specs and misses variant values that might come from directives in packages. - [x] move ad-hoc value handling code into spec_clauses so we do it in one place for CLI and packages - [x] move handling of `variant_possible_value`, etc. into `concretize.lp`, where we can automatically infer variant existence more concisely. - [x] simplify/clarify some of the code for variants in `spec_clauses()` --- lib/spack/spack/solver/asp.py | 95 +++++++++++++++++------------------- lib/spack/spack/solver/concretize.lp | 10 ++++ 2 files changed, 55 insertions(+), 50 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 7810d35445..d5f288b3b6 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -34,6 +34,7 @@ import spack.cmd import spack.compilers import spack.config import spack.dependency +import spack.directives import spack.error import spack.spec import spack.package @@ -696,6 +697,7 @@ class SpackSolverSetup(object): self.possible_versions = {} self.possible_virtuals = None self.possible_compilers = [] + self.variant_values_from_specs = set() self.version_constraints = set() self.target_constraints = set() self.providers_by_vspec_name = collections.defaultdict(list) @@ -1161,7 +1163,7 @@ class SpackSolverSetup(object): node_platform = fn.node_platform_set node_os = fn.node_os_set node_target = fn.node_target_set - variant = fn.variant_set + variant_value = fn.variant_set node_compiler = fn.node_compiler_hard node_compiler_version = fn.node_compiler_version_hard node_flag = fn.node_flag_set @@ -1171,7 +1173,7 @@ class SpackSolverSetup(object): node_platform = fn.node_platform node_os = fn.node_os node_target = fn.node_target - variant = fn.variant_value + variant_value = fn.variant_value node_compiler = fn.node_compiler node_compiler_version = fn.node_compiler_version node_flag = fn.node_flag @@ -1196,14 +1198,26 @@ class SpackSolverSetup(object): # variants for vname, variant in sorted(spec.variants.items()): - value = variant.value - if isinstance(value, tuple): - for v in value: - if v == '*': - continue - clauses.append(f.variant(spec.name, vname, v)) - elif value != '*': - clauses.append(f.variant(spec.name, vname, variant.value)) + values = variant.value + if not isinstance(values, (list, tuple)): + values = [values] + + for value in values: + # * is meaningless for concretization -- just for matching + if value == '*': + continue + + # validate variant value + if vname not in spack.directives.reserved_names: + variant_def = spec.package.variants[vname] + variant_def.validate_or_raise(variant, spec.package) + + clauses.append(f.variant_value(spec.name, vname, value)) + + # Tell the concretizer that this is a possible value for the + # variant, to account for things like int/str values where we + # can't enumerate the valid values + self.variant_values_from_specs.add((spec.name, vname, value)) # compiler and compiler version if spec.compiler: @@ -1512,6 +1526,18 @@ class SpackSolverSetup(object): ) self.gen.newline() + def define_variant_values(self): + """Validate variant values from the command line. + + Also add valid variant values from the command line to the + possible values for a variant. + + """ + # Tell the concretizer about possible values from specs we saw in + # spec_clauses() + for pkg, variant, value in sorted(self.variant_values_from_specs): + self.gen.fact(fn.variant_possible_value(pkg, variant, value)) + def setup(self, driver, specs, tests=False): """Generate an ASP program with relevant constraints for specs. @@ -1580,51 +1606,20 @@ class SpackSolverSetup(object): for dep in spec.traverse(): self.gen.h2('Spec: %s' % str(dep)) + # Inject dev_path from environment _develop_specs_from_env(dep) + if dep.virtual: for clause in self.virtual_spec_clauses(dep): self.gen.fact(clause) - else: - for clause in self.spec_clauses(dep): - self.gen.fact(clause) - # TODO: This might need to be moved somewhere else. - # TODO: It's needed to account for open-ended variants - # TODO: validated through a function. The rationale is - # TODO: that if a value is set from cli and validated - # TODO: then it's also a possible value. - if clause.name == 'variant_set': - variant_name = clause.args[1] - # 'dev_path' and 'patches are treated in a - # special way, as they are injected from cli - # or files - if variant_name == 'dev_path': - pkg_name = clause.args[0] - self.gen.fact(fn.variant( - pkg_name, variant_name - )) - self.gen.fact(fn.variant_single_value( - pkg_name, variant_name - )) - elif variant_name == 'patches': - pkg_name = clause.args[0] - self.gen.fact(fn.variant( - pkg_name, variant_name - )) - else: - variant_def = dep.package.variants[ - variant_name - ] - variant_def.validate_or_raise( - dep.variants[variant_name], - dep.package - ) - # State that this variant is a possible value - # to account for variant values that are not - # enumerated explicitly - self.gen.fact( - fn.variant_possible_value(*clause.args) - ) + continue + + for clause in self.spec_clauses(dep): + self.gen.fact(clause) + + self.gen.h1("Variant Values defined in specs") + self.define_variant_values() self.gen.h1("Virtual Constraints") self.define_virtual_constraints() diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index acfed6f599..96bf23a45c 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -222,6 +222,16 @@ variant_default_value(Package, Variant, Value) :- 2 {variant_value(Package, Variant, Value): variant_possible_value(Package, Variant, Value)}, variant_value(Package, Variant, "none"). +% patches and dev_path are special variants -- they don't have to be +% declared in the package, so we just allow them to spring into existence +% when assigned a value. +auto_variant("dev_path"). +auto_variant("patches"). +variant(Package, "dev_path") + :- variant_set(Package, Variant, _), auto_variant(Variant). +variant_single_value(Package, "dev_path") + :- variant_set(Package, "dev_path", _). + % suppress warnings about this atom being unset. It's only set if some % spec or some package sets it, and without this, clingo will give % warnings like 'info: atom does not occur in any rule head'. -- cgit v1.2.3-70-g09d2