-
Notifications
You must be signed in to change notification settings - Fork 9
Workspaces III - Building Platforms #1276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
This currently cannot return data in many cases but will evolve as other areas of the workspace setup do, too. Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
This commit augments the templating system for spk recipes,
enabling the definition of multiple package versions from a single
recipe file.
Key changes include:
- **`spk-schema`**:
- Updated `Template` trait and `SpecTemplate` struct to represent
recipe templates.
- A `v1::Platform` spec for defining platforms in a more structured
way.
- The `SpecRecipe` enum now includes `V1Platform`.
- **`spk-workspace`**:
- The `Workspace` and `WorkspaceBuilder` are updated to work with
the new `SpecTemplate` and templating system. Specifially,
workspace items can no longer specify versions - this is done
directly in the spec file now.
- **`spk-cli`**:
- The `make-recipe`, `workspace info`, and `view` commands are
updated to support the new templating system.
- **`spk-storage`**:
- The `WorkspaceRepository` is updated to work with the new
templating system, presenting versions based on the template.
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Adds a serde-peekable crate with a simple wrapper that allows peeking map keys so that custom deserialize methods can appropriately predict enum variants Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
This allows us to enalbe source builds for requests that define an explicit repo, provided that it has a source package for the package and version. Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Because templates may support many versions of a package, the existing method of turning ranges into LowestSpecified can make it so that there's no way for users to disambiguate the actual version that they want. By supporting version ranges, the = and == syntax can be used. Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
The presence of this field enables build-from-source for these packages within a workspace, and allows us to expand on the configuration for these builds in the future. Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
|
|
||
| pub fn from_yaml<S: Into<String>>(yaml: S) -> Result<SpecFileData> { | ||
| /// Parse the provided string as a yaml-encoded [`SpecFileData`]. | ||
| pub fn from_yaml<S: Into<String>>(yaml: S) -> Result<Self> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've being meaning to ask if the decision to use Into<String> instead of AsRef<str> (or Borrow?) was out of necessity. From what I've seen the argument doesn't need to be an owned value, and there are potentially callers that could pass a reference in here and avoid allocating a copy.
| let mut peekable = serde_peekable::PeekableMapAccess::from(map); | ||
| let first_key = peekable.peek_key::<Stringified>()?; | ||
| let Some(first_key) = first_key else { | ||
| return Err(serde::de::Error::missing_field("pkg or var")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error message seems wrong here.
| return Err(serde::de::Error::missing_field("pkg or var")); | ||
| }; | ||
| match first_key.as_ref() { | ||
| "gitTags" => Ok(DiscoverStrategy::GitTags(Deserialize::deserialize( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain the peekable map stuff here? It looks like this is just manually implementing what you get from a tagged enum. But the struct is marked as untagged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scratch what I said, I see that it is not like the available tagged enum flavors. However I do still have a concern that this requires that the first key is "gitTags" when it would otherwise be perfectly legal to have the content in the yaml file in some other order. For example, maybe a yaml prettier decides it wants to reorder the mapping and "gitTags" isn't first anymore.
You'd need to peek the whole mapping and check if it contains that key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The internally tagged flavor isn't terrible or so different from what we're trying to do and then we could just derive the code.
Opening this early just so that I can link to some of the changes and docs...