Allow packages to depend on my.version, variant=my.value, etc.#41700
Allow packages to depend on my.version, variant=my.value, etc.#41700
my.version, variant=my.value, etc.#41700Conversation
- [x] allow caller of `condition()` to pass lists of transforms - [x] all transform functions now take trigger *and* effect as parameters - [x] add some utility functions to simplify `condition()`
my.version, variant=my.value, etc.
5613e69 to
e3fc937
Compare
scheibelp
left a comment
There was a problem hiding this comment.
This appears incomplete (not all functionality discussed in the PR description is yet implemented) so I'm going for higher-level comments:
depends_on("geant4-data@{my.version}", when="@10:")
looks reasonable
depends_on("hdf5 mpi={my.variants['mpi'].value}")
makes it look like my is the Spec and you can execute any function you want on it. This seems susceptible to confusion and IMO users would only want to do things like this for a limited set of values (which suggests to me that a limited set of identifiers would be appropriate vs. adding a mechanism to execute arbitrary {})
| version("3.2.0", sha256="18d459400558f4ea99527bc9786c033965a3db45bf4c6a32eefdc07aa9e306a6") | ||
| version("3.1.1", sha256="18d459400558f4ea99527bc9786c033965a3db45bf4c6a32eefdc07aa9e306a6") | ||
| version("3.1.0", sha256="18d459400558f4ea99527bc9786c033965a3db45bf4c6a32eefdc07aa9e306a6") | ||
| version("3.0.0", sha256="18d459400558f4ea99527bc9786c033965a3db45bf4c6a32eefdc07aa9e306a6") |
There was a problem hiding this comment.
You might want to introduce some "mismatches" so the tests can demonstrate how that is handled (it seems like when= might be sufficient to that purpose).
| fn = AspFunctionBuilder() | ||
|
|
||
| TransformFunction = Callable[[spack.spec.Spec, List[AspFunction]], List[AspFunction]] | ||
| TransformFunction = Callable[ |
There was a problem hiding this comment.
This could use a docstring (here or in the transform method).
|
Commented this on Slack, but I think I would prefer names like:
These names match Spack/Python syntax more than |
|
I think it's worth trying to generalize this slightly -- as you've formulated it, this can only apply to dependency relationships. It would be great if we could apply it to siblings within the same unification space. Maybe something like this? attr(AffectedAttrName, AffectedNode, Value)
:- attr("node", node(X, Package)),
attr("node", AffectedNode),
unification_set(ID, node(X, Package)),
unification_set(ID, AffectedNode),
attr(PackageAttrName, node(X, Package), Value),
attr("sync", AffectedNode, AffectedAttrName, attr(PackageAttrName, Package)). |
Based on #37418 (comment), we want to try to enable something like:
You can think of
{my.version}as an expression template that expands out thedepends_on()to all possible values of some spec attribute.myhere is like a proxy for this package's spec, with a limited set of allowed attributes: basically the ones we know how to expand all values of. I think this makes it intuitive to filter by thewhen=clause (to be consistent with other directives). It also enables you to, e.g., only do version matching only on certain versions (10 or higher here).Expressions could be more complex:
And you could use this on variants as well:
The final advantage here is that we can push logic down to the solver instead of doing a
forloop at the package level. Not all versions, variant values, etc. are known at package definition time. They can come in from the CLI, config, externals, and other places, so only the solver really knows all the values at setup time. Similarly, for variants with infinite legal values (e.g,. ints, strings) we only know the values at solver setup time, and we can't explode the solve to include all possible strings/ints/whatever.@greenc-FNAL: this is a proof of concept to give you an idea of what parts of the code you'd need to touch to get something like this working. I have some ideas for how you could propagate an actual version/variant expressions from the
depends_on()directive, through the spec, down to the solver. Ultimately I think those need to be evaluated indefine_version_constraints(or a similar location for variants). That is, they should be run at the end of setup after all values from all specs in the solve are known.The amount of concretizer logic required here is surprisingly small. It's basically one new type of fact in the encoding:
That says that you should take
Versionin any derivedattr("version", Package, Version)and create a rule likeattr("node_version_satisfies", Dependency, Version)to force the dependency to use the same version. The implementation of that is just: