summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2020-12-05 21:22:20 -0800
committerTamara Dahlgren <dahlgren1@llnl.gov>2021-02-17 17:07:23 -0800
commit0e725f0ab18d1bc064d42111e3be3e8e27a5850e (patch)
tree84fd039d5c40f6dc816d8d5876b387d3eaf89ed6
parent9499dc4a7ed5037d4ee87385b621ed3910ac3969 (diff)
downloadspack-0e725f0ab18d1bc064d42111e3be3e8e27a5850e.tar.gz
spack-0e725f0ab18d1bc064d42111e3be3e8e27a5850e.tar.bz2
spack-0e725f0ab18d1bc064d42111e3be3e8e27a5850e.tar.xz
spack-0e725f0ab18d1bc064d42111e3be3e8e27a5850e.zip
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()`
-rw-r--r--lib/spack/spack/solver/asp.py95
-rw-r--r--lib/spack/spack/solver/concretize.lp10
2 files changed, 55 insertions, 50 deletions
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 890411e9e3..bf64efa3ec 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:
@@ -1513,6 +1527,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.
@@ -1581,51 +1607,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'.