Skip to content

Split Request to PinnableRequest and PinnedRequest#1297

Merged
jrray merged 3 commits intomainfrom
pinned-request
Dec 17, 2025
Merged

Split Request to PinnableRequest and PinnedRequest#1297
jrray merged 3 commits intomainfrom
pinned-request

Conversation

@jrray
Copy link
Collaborator

@jrray jrray commented 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:

# 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":

# 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. A related bug was recently discovered and fixed in #1302.

PinnedRequest did not get a different type for its Pkg variant
(yet?), but it also has a similar pinned/unpinned concept.

@jrray jrray self-assigned this Nov 30, 2025
@jrray jrray added maintenance Cleanup, upgrades, etc pr-chain This PR doesn't target the main branch, don't merge! labels Nov 30, 2025
assert_eq!(req.to_string(), "test");

install_spec
let install_spec = recipe_install_spec
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good example of the new flow from recipe to package, since they are now different types rendering pins is not an in-place operation.

Comment on lines -701 to -702
spk_schema::ident::PinnableValue::FromBuildEnv => todo!(),
spk_schema::ident::PinnableValue::FromBuildEnvIfPresent => todo!(),
Copy link
Collaborator Author

@jrray jrray Nov 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found these todo!s still hanging out, but in practice they are unreachable. Now the code path isn't possible.

@jrray jrray force-pushed the recipe-and-package-specs branch from 8292908 to cf80ac3 Compare December 1, 2025 09:07
@jrray jrray force-pushed the recipe-and-package-specs branch 2 times, most recently from 80f5787 to 3de2d63 Compare December 10, 2025 20:20
@jrray jrray force-pushed the pinned-request branch 2 times, most recently from 7cc4873 to 4954423 Compare December 10, 2025 20:47
@jrray jrray force-pushed the recipe-and-package-specs branch from 3de2d63 to 11ce6cb Compare December 10, 2025 20:47
Base automatically changed from recipe-and-package-specs to main December 10, 2025 21:20
@jrray jrray removed the pr-chain This PR doesn't target the main branch, don't merge! label Dec 10, 2025
@jrray jrray force-pushed the pinned-request branch 3 times, most recently from ecfe0e7 to 0f0d853 Compare December 10, 2025 21:33
@jrray jrray requested a review from rydrman December 10, 2025 21:45
@jrray jrray force-pushed the pinned-request branch 3 times, most recently from e2c9d96 to 9236ac7 Compare December 11, 2025 19:26
@jrray jrray requested a review from dcookspi December 12, 2025 20:14
Signed-off-by: J Robert Ray <jrray@jrray.org>
As this method always cloned self, per convention it should allow the
caller to decide if it needs to clone or give up ownership.

Admittedly there wasn't any existing code that could stop cloning.

Signed-off-by: J Robert Ray <jrray@jrray.org>
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 jrray merged commit 2c3b666 into main Dec 17, 2025
9 checks passed
@jrray jrray deleted the pinned-request branch December 17, 2025 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Cleanup, upgrades, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants