Skip to content
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

Preserve install environment #1129

Merged
merged 3 commits into from
Sep 19, 2024
Merged

Preserve install environment #1129

merged 3 commits into from
Sep 19, 2024

Conversation

jrray
Copy link
Collaborator

@jrray jrray commented Sep 18, 2024

Refactor various is_default() methods into IsDefault trait

Fixes #1081. The bug causing #1081 was in InstallSpec::is_default as the
logic was not updated when the environment field was added to the struct
way back at

pub environment: Vec<EnvOp>,

As a guard against this kind of bug in the future, an IsDefault trait was
created and a derive macro created to derive the is_default method
semi-automatically. All existing is_default methods have been changed to
use the trait instead.

And...

Refactor Package::runtime_environment

Both recipes and packages have the install.environment property, so
refactor runtime_environment() out of the Package trait into its own
trait, then require both Recipe and Package implement it.

This change was prompted in order to make the code in the new test possible.

Both recipes and packages have the `install.environment` property, so
refactor `runtime_environment()` out of the `Package` trait into its own
trait, then require both `Recipe` and `Package` implement it.

Signed-off-by: J Robert Ray <jrray@jrray.org>
In order to implement foreign traits for this type.

Signed-off-by: J Robert Ray <jrray@jrray.org>
Fixes #1081. The bug causing #1081 was in `InstallSpec::is_default` as the
logic was not updated when the `environment` field was added to the struct
way back at https://github.com/spkenv/spk/blob/f4899ce1e4ac0634ee95608e322a86a358096d4d/src/api/install_spec.rs#L31

As a guard against this kind of bug in the future, an `IsDefault` trait was
created and a derive macro created to derive the `is_default` method
semi-automatically. All existing `is_default` methods have been changed to
use the trait instead.

Signed-off-by: J Robert Ray <jrray@jrray.org>
@jrray jrray added the bug Something isn't working label Sep 18, 2024
@jrray jrray self-assigned this Sep 18, 2024
@jrray jrray requested a review from rydrman September 18, 2024 01:02
Copy link
Collaborator

@rydrman rydrman left a comment

Choose a reason for hiding this comment

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

Nice, love the addition of the proc macro as well

@jrray jrray merged commit 662e4fa into main Sep 19, 2024
5 checks passed
@jrray jrray deleted the preserve-install-environment branch September 19, 2024 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

install.environment config is lost when recipe is published
2 participants