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

Expose store.toml contents in BuildContext #543

Merged
merged 1 commit into from
Jan 9, 2023
Merged

Expose store.toml contents in BuildContext #543

merged 1 commit into from
Jan 9, 2023

Conversation

Malax
Copy link
Member

@Malax Malax commented Jan 9, 2023

Up until now, it wasn't possible to access to contents of store.toml via libcnb. This PR adds a minimal implementation and tests for accessing that data. I deliberately limited this PR to just that and not address issues with store.toml handling in general:

  • Ideally, store.toml should be typed like layer metadata is. Working with raw TOML with Rust is tedious, especially when we need a specific TOML type (store.toml metadata needs to be a toml::value::Table. However, this is not just a matter of slapping a new associated type on Buildpack and use that to parse the metadata. We need a mechanism to handle incompatibilities (like we do with layer metadata). (I filed: Typed store.toml metadata #544)
  • Right now, if the BuildResult does not contain store.toml data (i.e. that field is None), the file on disk is not modified. This is inconsistent with i.e. layers where the metadata needs to be written every time to keep it around. There is currently no way to remove all metadata either since None signals "don't change" instead of "no data". (I filed: Inconsistent handling of store.toml #545)

Without the fixes for the issues above, this PR a non breaking change we can roll out immediately as a point release to provide the needed functionality to buildpacks that need it and tackle the harder breaking changes separately.

Closes GUS-W-12299144

@Malax Malax added enhancement New feature or request libcnb labels Jan 9, 2023
@Malax Malax force-pushed the malax/store-toml branch from 881585f to 298755b Compare January 9, 2023 10:06
@Malax Malax force-pushed the malax/store-toml branch from 298755b to d9da442 Compare January 9, 2023 10:14
@Malax Malax marked this pull request as ready for review January 9, 2023 10:31
@Malax Malax requested a review from a team as a code owner January 9, 2023 10:31
Copy link
Member

@edmorley edmorley left a comment

Choose a reason for hiding this comment

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

Thank you!

@Malax Malax merged commit 2e23262 into main Jan 9, 2023
@Malax Malax deleted the malax/store-toml branch January 9, 2023 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request libcnb
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants