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

Support Buildpack API 0.10 #291

Merged
merged 4 commits into from
Jun 18, 2024

Conversation

c0d1ngm0nk3y
Copy link
Contributor

see task list

Fixes #260

@c0d1ngm0nk3y c0d1ngm0nk3y force-pushed the support-bp-api-0.10 branch from 72e4c62 to 0cc9251 Compare May 15, 2024 13:10
@loewenstein-sap
Copy link

loewenstein-sap commented May 21, 2024

@samj1912 & @dmikusa could you have a look. Should be the last must have for cutting libcnb/v2 and getting rid of branch maintenance.

There is more in the 2.0 Milestone, but nothing that reads as "required for v2".

@loewenstein-sap
Copy link

@samj1912 & @dmikusa ^^

@dmikusa dmikusa added type:enhancement A general enhancement semver:minor A change requiring a minor version bump labels Jun 6, 2024
@dmikusa
Copy link
Contributor

dmikusa commented Jun 6, 2024

What's here looks good. I'm still picking through the spec changes, especially the extension images part to make sure we got everything.

One thing I don't see is support for extension config. This should have a struct so we can serialize/deserialize it. Probably also a new API in generate.go that allows one to easily add this configuration.

I also feel like we should be doing something with the dockerfile generation. The GenerateResult could have fields for this, perhaps each one could be a io.Writer. Then after the generate function runs, libcnb could take these writers and generate the files in the correct place. I feel like we should be doing something to help enforce some of the requirements of the spec in terms of what you can/can't do in the dockerfiles, however, we can probably leave that out for now. This is still experimental so our support doesn't need to be 100%.

Maybe something similar with the context folders too? https://github.com/buildpacks/spec/blob/main/image_extension.md#context-folders Although, I'm still trying to understand the purpose of those and how they might be used.

@pbusko pbusko force-pushed the support-bp-api-0.10 branch 3 times, most recently from a9890f9 to 1672b79 Compare June 7, 2024 09:39
@pbusko
Copy link
Contributor

pbusko commented Jun 7, 2024

@dmikusa support for dockerfiles and extend-config has been added

@dmikusa
Copy link
Contributor

dmikusa commented Jun 11, 2024

Thanks @pbusko! I'm going to do a little testing with this and I'll get back to you soon. In the meantime, when you have a second, can you bump the PR one more time. Thanks

nicolasbender and others added 4 commits June 11, 2024 07:52
Co-authored-by: Ralf Pannemans <[email protected]>
Signed-off-by: Nicolas Bender <[email protected]>
Co-authored-by: Ralf Pannemans <[email protected]>
Signed-off-by: Nicolas Bender <[email protected]>
Co-authored-by: Ralf Pannemans <[email protected]>
Signed-off-by: Nicolas Bender <[email protected]>
@pbusko pbusko force-pushed the support-bp-api-0.10 branch from 1672b79 to 85dc71d Compare June 11, 2024 05:52
@dmikusa dmikusa merged commit b9fc3fe into buildpacks:main Jun 18, 2024
4 checks passed
@dmikusa
Copy link
Contributor

dmikusa commented Jun 18, 2024

@pbusko - FYI, I cut https://github.com/buildpacks/libcnb/releases/tag/v2.0.0-alpha.3 which has this merged.

@pbusko pbusko deleted the support-bp-api-0.10 branch June 18, 2024 05:55
@pbusko
Copy link
Contributor

pbusko commented Jun 19, 2024

@dmikusa is there anything else blocking the v2.0.0 release?

@dmikusa
Copy link
Contributor

dmikusa commented Jun 19, 2024

@dmikusa is there anything else blocking the v2.0.0 release?

@pbusko I'm doing some cleanup on issues, and some code review to investigate that. I think we're in pretty good shape though.

Off the top of my head, the one area I would like to put a little more effort is to add some convenience around the Dockerfile/extension abstraction. The byte[] we have now is literally the least we can do. I think we can add some more convenience on top of that without too much fuss, maybe a Writer or something to facilitate higher-level interactions.

@loewenstein-sap
Copy link

Off the top of my head, the one area I would like to put a little more effort is to add some convenience around the Dockerfile/extension abstraction. The byte[] we have now is literally the least we can do. I think we can add some more convenience on top of that without too much fuss, maybe a Writer or something to facilitate higher-level interactions.

Could convenience not easily go into 2.1 instead of holding back v2?

@dmikusa
Copy link
Contributor

dmikusa commented Jun 19, 2024

Sorry, but I'm going to answer your question with a question. Why rush?

@loewenstein-sap
Copy link

Looking at how long the branch was open and the hassle a long running branch brings to the table I thought getting rid of it should ideally not be deferred unnecessarily.

@dmikusa
Copy link
Contributor

dmikusa commented Jun 19, 2024

It's OK and not that big of a hassle. The rate of PRs for this project is low.

I do want to get v2 shipped, but I want to make sure we take the time to do it right. I also know that we'll need to continue supporting v1 and v2 for a while, so in the grand scheme of things, a little more time to get it right is not a big deal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor A change requiring a minor version bump type:enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for buildpacks API 0.10
5 participants