From 02513eae7efb918d2ba4160045da51514d1e3f4d Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Wed, 16 Apr 2025 18:15:14 -0700 Subject: [PATCH] wip Signed-off-by: Gregory Becker --- lib/spack/spack/solver/asp.py | 140 ++++++++++++++++++++++----------- lib/spack/spack/spec.py | 44 ++++++++--- lib/spack/spack/spec_parser.py | 9 ++- 3 files changed, 135 insertions(+), 58 deletions(-) diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 9c3b31e2357..a68d35713af 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -1877,6 +1877,73 @@ def _get_condition_id( return cond_id + def condition_clauses( + self, + required_spec: spack.spec.Spec, + imposed_spec: Optional[spack.spec.Spec] = None, + *, + required_name: Optional[str] = None, + imposed_name: Optional[str] = None, + msg: Optional[str] = None, + context: Optional[ConditionContext] = None, + ): + """Generate facts for a dependency or virtual provider condition. + + Arguments: + required_spec: the constraints that triggers this condition + imposed_spec: the constraints that are imposed when this condition is triggered + required_name: name for ``required_spec`` + (required if required_spec is anonymous, ignored if not) + imposed_name: name for ``imposed_spec`` + (required if imposed_spec is anonymous, ignored if not) + msg: description of the condition + context: if provided, indicates how to modify the clause-sets for the required/imposed + specs based on the type of constraint they are generated for (e.g. `depends_on`) + Returns: + int: id of the condition created by this function + """ + clauses = [] + required_name = required_spec.name or required_name + if not required_name: + raise ValueError(f"Must provide a name for anonymous condition: '{required_spec}'") + + if not context: + context = ConditionContext() + context.transform_imposed = remove_facts("node", "virtual_node") + + if imposed_spec: + imposed_name = imposed_spec.name or imposed_name + if not imposed_name: + raise ValueError(f"Must provide a name for imposed constraint: '{imposed_spec}'") + + with named_spec(required_spec, required_name), named_spec(imposed_spec, imposed_name): + # Check if we can emit the requirements before updating the condition ID counter. + # In this way, if a condition can't be emitted but the exception is handled in the + # caller, we won't emit partial facts. + + condition_id = next(self._id_counter) + requirement_context = context.requirement_context() + trigger_id = self._get_condition_id( + required_spec, cache=self._trigger_cache, body=True, context=requirement_context + ) + clauses.append(fn.pkg_fact(required_spec.name, fn.condition(condition_id))) + clauses.append(fn.condition_reason(condition_id, msg)) + clauses.append( + fn.pkg_fact(required_spec.name, fn.condition_trigger(condition_id, trigger_id)) + ) + if not imposed_spec: + return clauses, condition_id + + impose_context = context.impose_context() + effect_id = self._get_condition_id( + imposed_spec, cache=self._effect_cache, body=False, context=impose_context + ) + clauses.append( + fn.pkg_fact(required_spec.name, fn.condition_effect(condition_id, effect_id)) + ) + + return clauses, condition_id + def condition( self, required_spec: spack.spec.Spec, @@ -1902,46 +1969,18 @@ def condition( Returns: int: id of the condition created by this function """ - required_name = required_spec.name or required_name - if not required_name: - raise ValueError(f"Must provide a name for anonymous condition: '{required_spec}'") + clauses, condition_id = self.condition_clauses( + required_spec=required_spec, + imposed_spec=imposed_spec, + required_name=required_name, + imposed_name=imposed_name, + msg=msg, + context=context, + ) + for clause in clauses: + self.gen.fact(clause) - if not context: - context = ConditionContext() - context.transform_imposed = remove_facts("node", "virtual_node") - - if imposed_spec: - imposed_name = imposed_spec.name or imposed_name - if not imposed_name: - raise ValueError(f"Must provide a name for imposed constraint: '{imposed_spec}'") - - with named_spec(required_spec, required_name), named_spec(imposed_spec, imposed_name): - # Check if we can emit the requirements before updating the condition ID counter. - # In this way, if a condition can't be emitted but the exception is handled in the - # caller, we won't emit partial facts. - - condition_id = next(self._id_counter) - requirement_context = context.requirement_context() - trigger_id = self._get_condition_id( - required_spec, cache=self._trigger_cache, body=True, context=requirement_context - ) - self.gen.fact(fn.pkg_fact(required_spec.name, fn.condition(condition_id))) - self.gen.fact(fn.condition_reason(condition_id, msg)) - self.gen.fact( - fn.pkg_fact(required_spec.name, fn.condition_trigger(condition_id, trigger_id)) - ) - if not imposed_spec: - return condition_id - - impose_context = context.impose_context() - effect_id = self._get_condition_id( - imposed_spec, cache=self._effect_cache, body=False, context=impose_context - ) - self.gen.fact( - fn.pkg_fact(required_spec.name, fn.condition_effect(condition_id, effect_id)) - ) - - return condition_id + return condition_id def impose(self, condition_id, imposed_spec, node=True, body=False): imposed_constraints = self.spec_clauses(imposed_spec, body=body) @@ -2635,13 +2674,22 @@ def _spec_clauses( # if it's concrete, then the hashes above take care of dependency # constraints, but expand the hashes if asked for. if not spec.concrete or expand_hashes: - dependency_clauses = self._spec_clauses( - dep, - body=body, - expand_hashes=expand_hashes, - concrete_build_deps=concrete_build_deps, - context=context, - ) + if dspec.when != spack.spec.Spec(): + # Are the non-attr things issued here a problem? + dependency_clauses, _ = self.condition_clauses( + required_spec=dspec.when, + imposed_spec=dep, + required_name=dspec.when.name or spec.name, + msg=f"{spec.name} depends conditionally on {dep.name}", + ) + else: + dependency_clauses = self._spec_clauses( + dep, + body=body, + expand_hashes=expand_hashes, + concrete_build_deps=concrete_build_deps, + context=context, + ) if dspec.depflag == dt.BUILD: clauses.append(fn.attr("depends_on", spec.name, dep.name, "build")) if body is False: diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 01fb7f18d5d..ea8809c2092 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -720,7 +720,7 @@ class DependencySpec: virtuals: virtual packages provided from child to parent node. """ - __slots__ = "parent", "spec", "depflag", "virtuals", "direct" + __slots__ = "parent", "spec", "depflag", "virtuals", "direct", "when" def __init__( self, @@ -730,12 +730,14 @@ def __init__( depflag: dt.DepFlag, virtuals: Tuple[str, ...], direct: bool = False, + when: Optional["Spec"] = None, ): self.parent = parent self.spec = spec self.depflag = depflag self.virtuals = tuple(sorted(set(virtuals))) self.direct = direct + self.when = when or Spec() def update_deptypes(self, depflag: dt.DepFlag) -> bool: """Update the current dependency types""" @@ -766,6 +768,7 @@ def copy(self) -> "DependencySpec": depflag=self.depflag, virtuals=self.virtuals, direct=self.direct, + when=self.when, ) def _cmp_iter(self): @@ -777,10 +780,13 @@ def _cmp_iter(self): def __str__(self) -> str: parent = self.parent.name if self.parent else None child = self.spec.name if self.spec else None - return f"{parent} {self.depflag}[virtuals={','.join(self.virtuals)}] --> {child}" + virtuals_string = f"virtuals={','.join(self.virtuals)}" if self.virtuals else "" + when_string = f"when={self.when}" if self.when != Spec() else "" + edge_attrs = filter((virtuals_string, when_string), lambda x: bool(x)) + return f"{parent} {self.depflag}[{' '.join(edge_attrs)}] --> {child}" def flip(self) -> "DependencySpec": - """Flip the dependency, and drop virtual information""" + """Flip the dependency, and drop virtual and conditional information""" return DependencySpec( parent=self.spec, spec=self.parent, depflag=self.depflag, virtuals=() ) @@ -1752,7 +1758,13 @@ def _set_architecture(self, **kwargs): setattr(self.architecture, new_attr, new_value) def _add_dependency( - self, spec: "Spec", *, depflag: dt.DepFlag, virtuals: Tuple[str, ...], direct: bool = False + self, + spec: "Spec", + *, + depflag: dt.DepFlag, + virtuals: Tuple[str, ...], + direct: bool = False, + when: Optional["Spec"] = None, ): """Called by the parser to add another spec as a dependency. @@ -1760,20 +1772,25 @@ def _add_dependency( depflag: dependency type for this edge virtuals: virtuals on this edge direct: if True denotes a direct dependency (associated with the % sigil) + when: if non-None, condition under which dependency holds """ if spec.name not in self._dependencies or not spec.name: - self.add_dependency_edge(spec, depflag=depflag, virtuals=virtuals, direct=direct) + self.add_dependency_edge( + spec, depflag=depflag, virtuals=virtuals, direct=direct, when=when + ) return # Keep the intersection of constraints when a dependency is added multiple times with # the same deptype. Add a new dependency if it is added with a compatible deptype - # (for example, a build-only dependency is compatible with a link-only dependenyc). + # (for example, a build-only dependency is compatible with a link-only dependency). # The only restrictions, currently, are that we cannot add edges with overlapping # dependency types and we cannot add multiple edges that have link/run dependency types. # See ``spack.deptypes.compatible``. orig = self._dependencies[spec.name] try: - dspec = next(dspec for dspec in orig if depflag == dspec.depflag) + dspec = next( + dspec for dspec in orig if depflag == dspec.depflag and when == dspec.when + ) except StopIteration: # Error if we have overlapping or incompatible deptypes if any(not dt.compatible(dspec.depflag, depflag) for dspec in orig): @@ -1785,7 +1802,9 @@ def _add_dependency( f"\t'{str(self)}' cannot depend on '{required_dep_str}'" ) - self.add_dependency_edge(spec, depflag=depflag, virtuals=virtuals, direct=direct) + self.add_dependency_edge( + spec, depflag=depflag, virtuals=virtuals, direct=direct, when=when + ) return try: @@ -1803,6 +1822,7 @@ def add_dependency_edge( depflag: dt.DepFlag, virtuals: Tuple[str, ...], direct: bool = False, + when: Optional["Spec"] = None, ): """Add a dependency edge to this spec. @@ -1811,6 +1831,7 @@ def add_dependency_edge( deptypes: dependency types for this edge virtuals: virtuals provided by this edge direct: if True denotes a direct dependency + when: if non-None, condition under which dependency holds """ # Check if we need to update edges that are already present selected = self._dependencies.select(child=dependency_spec.name) @@ -1818,6 +1839,9 @@ def add_dependency_edge( has_errors, details = False, [] msg = f"cannot update the edge from {edge.parent.name} to {edge.spec.name}" + if edge.when != when: + continue + # If the dependency is to an existing spec, we can update dependency # types. If it is to a new object, check deptype compatibility. if id(edge.spec) != id(dependency_spec) and not dt.compatible(edge.depflag, depflag): @@ -1841,7 +1865,7 @@ def add_dependency_edge( raise spack.error.SpecError(msg, "\n".join(details)) for edge in selected: - if id(dependency_spec) == id(edge.spec): + if id(dependency_spec) == id(edge.spec) and edge.when == when: # If we are here, it means the edge object was previously added to # both the parent and the child. When we update this object they'll # both see the deptype modification. @@ -1850,7 +1874,7 @@ def add_dependency_edge( return edge = DependencySpec( - self, dependency_spec, depflag=depflag, virtuals=virtuals, direct=direct + self, dependency_spec, depflag=depflag, virtuals=virtuals, direct=direct, when=when ) self._dependencies.add(edge) dependency_spec._dependents.add(edge) diff --git a/lib/spack/spack/spec_parser.py b/lib/spack/spack/spec_parser.py index aab6dc292b7..eee941c59d9 100644 --- a/lib/spack/spack/spec_parser.py +++ b/lib/spack/spack/spec_parser.py @@ -511,10 +511,10 @@ def parse(self): name = name[:-1] value = value.strip("'\" ").split(",") attributes[name] = value - if name not in ("deptypes", "virtuals"): + if name not in ("deptypes", "virtuals", "when"): msg = ( "the only edge attributes that are currently accepted " - 'are "deptypes" and "virtuals"' + 'are "deptypes", "virtuals", and "when"' ) raise SpecParsingError(msg, self.ctx.current_token, self.literal_str) # TODO: Add code to accept bool variants here as soon as use variants are implemented @@ -528,6 +528,11 @@ def parse(self): if "deptypes" in attributes: deptype_string = attributes.pop("deptypes") attributes["depflag"] = spack.deptypes.canonicalize(deptype_string) + + # Turn "when" into a spec + if "when" in attributes: + attributes["when"] = spack.spec.Spec(attributes["when"][0]) + return attributes