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

[Analyze] Provide mixin information #573

Closed
natalieparellano opened this issue Apr 12, 2021 · 8 comments
Closed

[Analyze] Provide mixin information #573

natalieparellano opened this issue Apr 12, 2021 · 8 comments
Assignees
Labels
epic/stackpacks/phase-1 Reverse analyze & detect; analyze enhancements epic/stackpacks good first issue A good first issue to get started with. status/blocked type/enhancement New feature or request

Comments

@natalieparellano
Copy link
Member

As outlined in RFC: https://github.com/buildpacks/rfcs/blob/main/text/0075-move-analyze-phase.md and spec PR: https://github.com/buildpacks/spec/pull/197/files

Analyzer will be provided new -run-image and -stack flags. Analyzer should resolve the run-image from the -run-image if provided, read it from the -stack, or fail if a valid run-image cannot be determined. Analyzer should read the build-image from the -stack, or fail if a valid build-image cannot be determined.

For both the build- and run-images, analyzer should read mixins and write this information in analyzed.toml.

(from RFC) Q: How should analyzed.toml be changed to include run-image information (mixins)

@matejvasek
Copy link
Contributor

I’d like to pick this up.

@jabrown85
Copy link
Contributor

The question around analyzed.toml was resolved in the spec here

@natalieparellano natalieparellano added this to the lifecycle-0.12.0 milestone May 10, 2021
@natalieparellano
Copy link
Member Author

@matejvasek have you had a chance to pick this up?

@matejvasek
Copy link
Contributor

@natalieparellano sorry for late reply, unfortunately I haven had much time as I had to work on something else.

Now I can work on this. But I will probably need some guidance.

Analyzer will be provided new -run-image and -stack flags.

I see this has already been done be @jabrown85 right?

https://github.com/buildpacks/spec/blob/platform/0.7/platform.md#analyzedtoml-toml

I see that the structure of the file has change between 0.6 and 0.7.
I guess my first step would be to change platform.AnalyzedMetadata to include:
previous-image, run-image and build-image and the old members probably put to some v06 sub-struct?
Am I right?

@natalieparellano
Copy link
Member Author

@matejvasek thanks for getting back on this! You are right, the struct for analyzed.toml will need to change for the newer platform API.

This is actually hitting on an issue that we have been grappling with for some time - how to manage behaviors and data structures across api versions - that we haven't found a good way of managing as of yet (more context here: #525). It is probably a bit overly optimistic to call this issue a "good first issue", given that we haven't solved the larger problem yet :)

I am trying to tackle some of this complexity in a PR for #578 (currently very messy WIP: main...validate-mixins). I think we'll probably want to align on the direction of this refactor before #573 will be sane to work on.

Would it be alright if we were to circle back once that discussion has happened? Please feel free to add your thoughts as well!

@matejvasek
Copy link
Contributor

This is actually hitting on an issue that we have been grappling with for some time - how to manage behaviors and data structures across api versions

@natalieparellano is there progress on this?

@natalieparellano
Copy link
Member Author

@matejvasek this PR #639 is my proposal for how to solve this problem (related Slack conversation). We're still gathering input and talking it over (please feel free to weigh in if you have thoughts here!). I suspect this will be a topic of this week's implementation sub team sync which you are more than welcome to join if that interests you.

With that said I must say that this RFC buildpacks/rfcs#167 to remove the stack and mixin concepts has given us pause as to whether we should forge ahead with this feature as part of platform 0.7 / lifecycle 0.12.0. I am planning to bring this up at this week's working group. Your input here is again very welcome.

In any case I'll update this issue with more information once those discussions have been had.

@natalieparellano natalieparellano removed this from the lifecycle-0.12.0 milestone Jun 24, 2021
@natalieparellano
Copy link
Member Author

Hi @matejvasek after discussion in both forums we've decided to pause this issue until there is more clarity around buildpacks/rfcs#167. I'll keep updating this issue as we learn more... in the meantime, please feel free to weigh in on buildpacks/rfcs#167 as you're interested in stackpacks - we'd love to know if the alternative presented there would be useful for your needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic/stackpacks/phase-1 Reverse analyze & detect; analyze enhancements epic/stackpacks good first issue A good first issue to get started with. status/blocked type/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants