Replace Spec with RecipeSpec and PackageSpec#1295
Merged
Conversation
jrray
force-pushed
the
recipe-and-package-specs
branch
from
8f67468 to
77f8db0
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
jrray
force-pushed
the
recipe-and-package-specs
branch
from
77f8db0 to
d1dc29f
Compare
Collaborator
Author
2 tasks
jrray
force-pushed
the
recipe-and-package-specs
branch
from
5de59ee to
6e44792
Compare
jrray
added a commit
that referenced
this pull request
Nov 30, 2025
This takes advantage of the work in #1295 in order to guarantee via types that var requirements have values in the context of a package requirements. The `PinnableRequest` type is what was previously called `Request`. This models what can appear in the install.requirements section of a recipe, as in: ```yaml # in a recipe install: requirements: - var: foo fromBuildEnv: true ``` When the package is built then a value must have been provided via options somewhere, and so the var becomes "pinned": ```yaml # in a package install: requirements: - var: foo/bar ``` While previously all code that operated on a `Request` would have to contend with the type allowing for the value to be missing (or not yet pinned), now a `PinnedRequest` is used that guarantees the value is present. This change also enforces via the type system that when a recipe is turned into a package that all the requirements are pinned. `PinnedRequest` did not get a different type for its `Pkg` variant (yet?), but it also has a similar pinned/unpinned concept. Signed-off-by: J Robert Ray <jrray@jrray.org>
jrray
force-pushed
the
recipe-and-package-specs
branch
from
6e44792 to
8292908
Compare
jrray
added a commit
that referenced
this pull request
Nov 30, 2025
This takes advantage of the work in #1295 in order to guarantee via types that var requirements have values in the context of a package requirements. The `PinnableRequest` type is what was previously called `Request`. This models what can appear in the install.requirements section of a recipe, as in: ```yaml # in a recipe install: requirements: - var: foo fromBuildEnv: true ``` When the package is built then a value must have been provided via options somewhere, and so the var becomes "pinned": ```yaml # in a package install: requirements: - var: foo/bar ``` While previously all code that operated on a `Request` would have to contend with the type allowing for the value to be missing (or not yet pinned), now a `PinnedRequest` is used that guarantees the value is present. This change also enforces via the type system that when a recipe is turned into a package that all the requirements are pinned. `PinnedRequest` did not get a different type for its `Pkg` variant (yet?), but it also has a similar pinned/unpinned concept. Signed-off-by: J Robert Ray <jrray@jrray.org>
jrray
force-pushed
the
recipe-and-package-specs
branch
from
8292908 to
cf80ac3
Compare
rydrman
approved these changes
Dec 5, 2025
Collaborator
rydrman
left a comment
rydrman
left a comment
There was a problem hiding this comment.
I'll mention that I didn't review much in the way of logic here, based on the copy-pasting notes that you made above. But if you were particularly nervous about anything let me know. Otherwise, I looked mostly at the structs, fields and new limitation in the embedded specs.
Comment on lines
+70
to
+72
|
|
||
| #[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd, Serialize)] | ||
| pub struct PackageSpec { |
Collaborator
There was a problem hiding this comment.
just noting that some of these "top-level" structs don't have any descriptions, might be a useful addition if you are feeling inclined
Collaborator
Author
There was a problem hiding this comment.
Ah yes, the "source material" for the new content didn't have doc strings either, but no doubt it would be good to have.
Thanks for the approval. I'm not nervous yet about this effort because I believe all of these changes are backwards compatible. My priority is getting #1296 merged so I can keep my promise of delivering that feature by the end of the year.
jrray
force-pushed
the
recipe-and-package-specs
branch
2 times, most recently
from
80f5787 to
3de2d63
Compare
This is the first step towards having separate types for the different
variations of specs that are valid in specific contexts. These changes
will be done in a series of PRs to limit the diff size of each change.
In this PR the focus is on embedded packages. In a recipe, an embedded
package spec in a recipe spec is expected to be given a `VersionIdent`,
as in:
```yaml
install:
embedded:
- pkg: example/1.0.0
```
It is now illegal to specify a build id for an embedded package in a
recipe.
When the package is built, the embedded package in a package spec is a
`BuildIdent` to reflect the idea that the embedded package is "built",
such as:
```yaml
install:
embedded:
- pkg: example/1.0.0/embedded
```
Test content has been updated appropriately since this was not enforced
previously.
What is allowed in the `build:` and `install:` sections of an embedded
package is different from a regular package, so `EmbeddedRecipeSpec`
and `EmbeddedPackageSpec` model those differences. These contain
`EmbeddedBuildSpec` and `EmbeddedInstallSpec`.
An `EmbeddedInstallSpec` does not contain an `embedded` field, making it
impossible to nest embedded packages.
`EmbeddedPackagesList` gained a generic parameter for the type of spec
it contains so it can be reused for the two flavors of embedded specs.
`Package::components()` was moved to its own trait to make is possible
to not have `Package` implemented for embedded packages. Note that when
embedded packages are turned into stubs, the stubs are normal packages
that implement `Package`.
All the new types were created in the `v0` module since they are part of
the `v0/package` API. The types like `BuildSpec` and `InstallSpec`
should arguably have been defined in `v0` as well.
These new types were created by copying and modifying the existing types
(and tests) and I haven't (at this point) done the work to remove
duplicate or unused code.
Signed-off-by: J Robert Ray <jrray@jrray.org>
Eliminate some duplicate code. Signed-off-by: J Robert Ray <jrray@jrray.org>
Coverage testing shows that this is unused. Signed-off-by: J Robert Ray <jrray@jrray.org>
As seen from coverage tests. Signed-off-by: J Robert Ray <jrray@jrray.org>
jrray
force-pushed
the
recipe-and-package-specs
branch
from
3de2d63 to
11ce6cb
Compare
jrray
added a commit
that referenced
this pull request
Dec 10, 2025
This takes advantage of the work in #1295 in order to guarantee via types that var requirements have values in the context of a package requirements. The `PinnableRequest` type is what was previously called `Request`. This models what can appear in the install.requirements section of a recipe, as in: ```yaml # in a recipe install: requirements: - var: foo fromBuildEnv: true ``` When the package is built then a value must have been provided via options somewhere, and so the var becomes "pinned": ```yaml # in a package install: requirements: - var: foo/bar ``` While previously all code that operated on a `Request` would have to contend with the type allowing for the value to be missing (or not yet pinned), now a `PinnedRequest` is used that guarantees the value is present. This change also enforces via the type system that when a recipe is turned into a package that all the requirements are pinned. `PinnedRequest` did not get a different type for its `Pkg` variant (yet?), but it also has a similar pinned/unpinned concept. Signed-off-by: J Robert Ray <jrray@jrray.org>
jrray
added a commit
that referenced
this pull request
Dec 10, 2025
This takes advantage of the work in #1295 in order to guarantee via types that var requirements have values in the context of a package requirements. The `PinnableRequest` type is what was previously called `Request`. This models what can appear in the install.requirements section of a recipe, as in: ```yaml # in a recipe install: requirements: - var: foo fromBuildEnv: true ``` When the package is built then a value must have been provided via options somewhere, and so the var becomes "pinned": ```yaml # in a package install: requirements: - var: foo/bar ``` While previously all code that operated on a `Request` would have to contend with the type allowing for the value to be missing (or not yet pinned), now a `PinnedRequest` is used that guarantees the value is present. This change also enforces via the type system that when a recipe is turned into a package that all the requirements are pinned. `PinnedRequest` did not get a different type for its `Pkg` variant (yet?), but it also has a similar pinned/unpinned concept. Signed-off-by: J Robert Ray <jrray@jrray.org>
jrray
added a commit
that referenced
this pull request
Dec 10, 2025
This takes advantage of the work in #1295 in order to guarantee via types that var requirements have values in the context of a package requirements. The `PinnableRequest` type is what was previously called `Request`. This models what can appear in the install.requirements section of a recipe, as in: ```yaml # in a recipe install: requirements: - var: foo fromBuildEnv: true ``` When the package is built then a value must have been provided via options somewhere, and so the var becomes "pinned": ```yaml # in a package install: requirements: - var: foo/bar ``` While previously all code that operated on a `Request` would have to contend with the type allowing for the value to be missing (or not yet pinned), now a `PinnedRequest` is used that guarantees the value is present. This change also enforces via the type system that when a recipe is turned into a package that all the requirements are pinned. `PinnedRequest` did not get a different type for its `Pkg` variant (yet?), but it also has a similar pinned/unpinned concept. Signed-off-by: J Robert Ray <jrray@jrray.org>
jrray
added a commit
that referenced
this pull request
Dec 11, 2025
This takes advantage of the work in #1295 in order to guarantee via types that var requirements have values in the context of a package requirements. The `PinnableRequest` type is what was previously called `Request`. This models what can appear in the install.requirements section of a recipe, as in: ```yaml # in a recipe install: requirements: - var: foo fromBuildEnv: true ``` When the package is built then a value must have been provided via options somewhere, and so the var becomes "pinned": ```yaml # in a package install: requirements: - var: foo/bar ``` While previously all code that operated on a `Request` would have to contend with the type allowing for the value to be missing (or not yet pinned), now a `PinnedRequest` is used that guarantees the value is present. This change also enforces via the type system that when a recipe is turned into a package that all the requirements are pinned. `PinnedRequest` did not get a different type for its `Pkg` variant (yet?), but it also has a similar pinned/unpinned concept. Signed-off-by: J Robert Ray <jrray@jrray.org>
jrray
added a commit
that referenced
this pull request
Dec 17, 2025
This takes advantage of the work in #1295 in order to guarantee via types that var requirements have values in the context of a package requirements. The `PinnableRequest` type is what was previously called `Request`. This models what can appear in the install.requirements section of a recipe, as in: ```yaml # in a recipe install: requirements: - var: foo fromBuildEnv: true ``` When the package is built then a value must have been provided via options somewhere, and so the var becomes "pinned": ```yaml # in a package install: requirements: - var: foo/bar ``` While previously all code that operated on a `Request` would have to contend with the type allowing for the value to be missing (or not yet pinned), now a `PinnedRequest` is used that guarantees the value is present. This change also enforces via the type system that when a recipe is turned into a package that all the requirements are pinned. `PinnedRequest` did not get a different type for its `Pkg` variant (yet?), but it also has a similar pinned/unpinned concept. Signed-off-by: J Robert Ray <jrray@jrray.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the first step towards having separate types for the different variations of specs that are valid in specific contexts. These changes will be done in a series of PRs to limit the diff size of each change.
In this PR the focus is on embedded packages. In a recipe, an embedded package spec in a recipe spec is expected to be given a
VersionIdent, as in:It is now illegal to specify a build id for an embedded package in a recipe.
When the package is built, the embedded package in a package spec is a
BuildIdentto reflect the idea that the embedded package is "built", such as:Test content has been updated appropriately since this was not enforced previously.
What is allowed in the
build:andinstall:sections of an embedded package is different from a regular package, soEmbeddedRecipeSpecandEmbeddedPackageSpecmodel those differences. These containEmbeddedBuildSpecandEmbeddedInstallSpec.An
EmbeddedInstallSpecdoes not contain anembeddedfield, making it impossible to nest embedded packages.EmbeddedPackagesListgained a generic parameter for the type of spec it contains so it can be reused for the two flavors of embedded specs.Package::components()was moved to its own trait to make is possible to not havePackageimplemented for embedded packages. Note that when embedded packages are turned into stubs, the stubs are normal packages that implementPackage.All the new types were created in the
v0module since they are part of thev0/packageAPI. The types likeBuildSpecandInstallSpecshould arguably have been defined inv0as well.These new types were created by copying and modifying the existing types (and tests) and I haven't (at this point) done the work to remove duplicate or unused code.