Skip to content

Conversation

@rydrman
Copy link
Collaborator

@rydrman rydrman commented Nov 24, 2022

This allows the trait to replace redundant arguments where solutions were already being passed which have access to the options from the solver as desired. Also adds the BuildEnvMember trait, expanding it to report which components were used rather than just the packages that were resolved. Finally, adds a target to each build env that identifies the package being built.

There's no practical use for these extensions in this merge, but all are required in order to manage the conditional logic that is laid out for the v1 spec format.

@rydrman rydrman added the Rust API Pertaining to the public rust API of the library label Nov 24, 2022
@rydrman rydrman added this to the V1 Spec milestone Nov 24, 2022
@rydrman rydrman requested review from dcookspi and jrray November 24, 2022 02:47
@rydrman rydrman self-assigned this Nov 24, 2022
@codecov
Copy link

codecov bot commented Nov 24, 2022

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (36c5262) 58.00% compared to head (072505b) 57.97%.
Report is 841 commits behind head on main.

Files Patch % Lines
crates/spk-schema/src/build_env.rs 18.18% 18 Missing ⚠️
crates/spk-solve/crates/solution/src/solution.rs 76.47% 4 Missing ⚠️
crates/spk-schema/src/recipe.rs 0.00% 2 Missing ⚠️
crates/spk-build/src/build/binary.rs 85.71% 1 Missing ⚠️
crates/spk-cli/common/src/exec.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #567      +/-   ##
==========================================
- Coverage   58.00%   57.97%   -0.03%     
==========================================
  Files         216      217       +1     
  Lines       15945    15996      +51     
==========================================
+ Hits         9249     9274      +25     
- Misses       6696     6722      +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rydrman rydrman mentioned this pull request Dec 23, 2022
11 tasks
@rydrman rydrman changed the base branch from master to 254-generic-ident December 24, 2022 02:46
@rydrman rydrman changed the base branch from 254-generic-ident to 296-solution-simplify December 24, 2022 02:47
@rydrman rydrman changed the title Expand BuildEnv trait to include options Expand BuildEnv trait to include options, target and used components Dec 24, 2022
@rydrman rydrman force-pushed the 296-solution-simplify branch from 01623ae to c1a5718 Compare January 18, 2023 06:38
Copy link
Collaborator

@jrray jrray left a comment

Choose a reason for hiding this comment

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

This one looks like it needs to be rebased past the ident reorg stuff too.

Base automatically changed from 296-solution-simplify to master February 1, 2023 01:00
@rydrman rydrman requested a review from jrray February 1, 2023 01:38
rydrman and others added 3 commits February 22, 2023 20:13
This allows the trait to replace a number of redundant arguments where
solutions were already being passed which have access to the options
from the solver as desired.

Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Build environments are not just package, but must also include
information about what components of each package are being used,
otherwise there's not enought information to reason about the complete
environment state.

Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
The target identifies the package and version being built that the build
environment was resolved for. This is needed for some conditionals to be
able to check if the condition is for the current package or one that
actually should have been resolved in the build environment itself.

Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
@rydrman rydrman added the unresolved Closed but not completed label Feb 14, 2024
@rydrman rydrman closed this Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Rust API Pertaining to the public rust API of the library unresolved Closed but not completed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants