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

Store the solution's 'requested by' and 'repo name' data in an spfs annotation for spk to use #837

Merged
merged 5 commits into from
May 16, 2024

Conversation

dcookspi
Copy link
Collaborator

@dcookspi dcookspi commented Aug 11, 2023

This stores the Solution's 'requested by' data in the spfs runtime creates by spk commands, e.g. spk env.... The data is stored as an spfs annotation. spk commands run inside that runtime restore this data into the current environment's Solution object. This allows commands like spk info to access the correct 'requested by' information, instead of making do with the placeholder "required by current environment" text.

Originally, I looked at storing/restoring the full Solution data in the runtime, but the Solution objects contain a lot data. Some of the data isn't easily serialisable, most of it is already reconstructed from the current runtime and repos in current_env(), some of it applies for the initial solver run but doesn't inside a runtime, and the 'requested by' data that's part of PkgRequest objects that we need is marked to skip serialisation (which is correct because we don't want this included in other places where PkgRequest are written out). So this change only stores/restores the requested by data from the resolved packages in the original Solution in an annotation in spfs.

Example, before this change:

> spk env python -- spk info -v
Installed Packages:
  runtime/stdfs:{build,run}/=1.0.0/3I42H3S6  highest (required by current environment) 
  runtime/python:{build,run}/=3.9.7/XO645NLV highest (required by current environment) 
 Number of Packages: 2

And, after this change:

Installed Packages:
  runtime/stdfs:{build,run}/=1.0.0/3I42H3S6  highest (required by python/3.9.7/XO645NLV) 
  runtime/python:{build,run}/=3.9.7/XO645NLV highest (required by command line) 
 Number of Packages: 2

This requires the changes in #815 (spfs runtime extra data support)

Todo:

  • Look at storing the original package sources so this information is available in the running environment
  • Turn stored BTreeMap into a versionable struct to make extending it easier, consider using protocol buffer
  • Update based on changes from spfs annotation data support #844

@dcookspi dcookspi self-assigned this Aug 11, 2023
@dcookspi dcookspi requested review from jrray and rydrman August 11, 2023 23:27
@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Attention: Patch coverage is 25.66138% with 281 lines in your changes are missing coverage. Please review.

Project coverage is 53.87%. Comparing base (e8cf549) to head (40ca8fc).
Report is 421 commits behind head on main.

❗ Current head 40ca8fc differs from pull request most recent head dcf2146. Consider uploading reports for the commit dcf2146 to get more accurate results

Files Patch % Lines
crates/spfs-cli/main/src/cmd_run.rs 22.44% 76 Missing ⚠️
crates/spfs/src/runtime/storage.rs 56.75% 48 Missing ⚠️
crates/spfs/src/env.rs 0.00% 45 Missing ⚠️
crates/spk-cli/common/src/env.rs 0.00% 28 Missing ⚠️
crates/spfs/src/bootstrap.rs 0.00% 13 Missing ⚠️
crates/spk-solve/crates/solution/src/solution.rs 0.00% 13 Missing ⚠️
crates/spfs-cli/cmd-clean/src/cmd_clean.rs 0.00% 11 Missing ⚠️
crates/spfs-cli/cmd-monitor/src/cmd_monitor.rs 0.00% 9 Missing ⚠️
crates/spfs-cli/main/src/cmd_runtime_remove.rs 0.00% 5 Missing ⚠️
crates/spfs-cli/main/src/cmd_shell.rs 0.00% 5 Missing ⚠️
... and 9 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #837      +/-   ##
==========================================
+ Coverage   53.55%   53.87%   +0.32%     
==========================================
  Files         258      253       -5     
  Lines       20466    20218     -248     
==========================================
- Hits        10960    10893      -67     
+ Misses       9506     9325     -181     

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

@dcookspi
Copy link
Collaborator Author

Added some todo items, following a discussion today.

@dcookspi dcookspi mentioned this pull request Aug 17, 2023
3 tasks
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from af2d9dd to 72f4164 Compare August 22, 2023 00:22
@dcookspi dcookspi changed the title Store and restore the solution's 'requested by' data in the runtimes created by spk Store and restore the solution's 'requested by' and 'repo name' data in the runtimes created by spk Aug 22, 2023
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from 8d07f98 to 1744610 Compare August 22, 2023 00:34
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from 72f4164 to d1041a9 Compare August 22, 2023 17:58
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from 1744610 to 40ca8fc Compare August 22, 2023 18:36
@dcookspi dcookspi added the enhancement New feature or request label Sep 6, 2023
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from d1041a9 to ccb39db Compare September 21, 2023 23:55
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from 40ca8fc to e7cf66c Compare September 22, 2023 18:22
@dcookspi dcookspi added the SPI AOI Area of interest for SPI label Sep 22, 2023
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from ccb39db to 31f51c0 Compare September 29, 2023 22:28
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from e7cf66c to d1a71b5 Compare September 29, 2023 22:44
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from 31f51c0 to 1c75710 Compare November 1, 2023 00:04
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from d1a71b5 to 40c28dd Compare November 1, 2023 00:37
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from 1c75710 to e7f0912 Compare November 6, 2023 23:12
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from 40c28dd to 6591a3c Compare November 6, 2023 23:31
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from e7f0912 to fcf11c5 Compare November 10, 2023 00:25
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from 6591a3c to f7e8aed Compare November 10, 2023 00:49
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from fcf11c5 to f20b03b Compare November 10, 2023 02:00
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from f7e8aed to fee728d Compare November 10, 2023 02:06
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch 2 times, most recently from 0c40ba3 to beca46f Compare November 16, 2023 20:30
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from fee728d to 8e82514 Compare November 16, 2023 20:54
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from beca46f to b6d0727 Compare December 13, 2023 21:05
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from ed1e155 to 88d3920 Compare February 10, 2024 02:07
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch 2 times, most recently from 06a8dd0 to fe012c7 Compare February 14, 2024 01:33
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from aa1fb04 to e6901d9 Compare February 24, 2024 02:07
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from fe012c7 to f446fa3 Compare February 28, 2024 17:40
@dcookspi
Copy link
Collaborator Author

Updated this for the external data to annotation name change.

@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch 2 times, most recently from 3ba643b to 1af5c65 Compare March 6, 2024 21:00
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from f446fa3 to 5eda518 Compare March 7, 2024 01:19
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from d3d2f33 to b213173 Compare March 8, 2024 00:18
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from 5eda518 to 03af1f8 Compare March 8, 2024 00:47
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from b213173 to d5285d6 Compare March 12, 2024 00:56
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from 03af1f8 to c459ddb Compare March 12, 2024 16:26
@dcookspi dcookspi force-pushed the spfs-add-arbitrary-data-to-runtime branch from d5285d6 to 5bec48e Compare March 14, 2024 00:58
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from c459ddb to 07c7a97 Compare March 14, 2024 01:11
Base automatically changed from spfs-add-arbitrary-data-to-runtime to main March 14, 2024 16:18
@dcookspi dcookspi removed the pr-chain This PR doesn't target the main branch, don't merge! label Mar 14, 2024
… runtime

Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
@dcookspi dcookspi force-pushed the spk-add-requested-by-to-runtime-data branch from 07c7a97 to 9cbbe80 Compare April 17, 2024 16:54
@dcookspi dcookspi removed the SPI-0.39 label Apr 17, 2024
@dcookspi dcookspi changed the title Store and restore the solution's 'requested by' and 'repo name' data in the runtimes created by spk Store the solution's 'requested by' and 'repo name' data in an spfs annotation for spk to use May 2, 2024
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.

Overall seems good - couple of questions/thoughts for you

Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
@dcookspi dcookspi merged commit 15bbc95 into main May 16, 2024
7 checks passed
@dcookspi dcookspi deleted the spk-add-requested-by-to-runtime-data branch May 16, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SPI AOI Area of interest for SPI SPI-0.41
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants