search

buildpacks / libcnb

A non-opinionated language binding for the Cloud Native Buildpack Buildpack and Extension specifications
Apache License 2.0
31 stars 13 forks source link

Support Buildpack API 0.10 #291

Closed c0d1ngm0nk3y closed 5 months ago

c0d1ngm0nk3y commented 6 months ago

see task list

Fixes https://github.com/buildpacks/libcnb/issues/260

loewenstein-sap commented 5 months ago

@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 commented 5 months ago

@samj1912 & @dmikusa ^^

dmikusa commented 5 months ago

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 commented 5 months ago

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

dmikusa commented 5 months ago

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

dmikusa commented 5 months ago

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

pbusko commented 5 months ago

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

dmikusa commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

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

loewenstein-sap commented 5 months ago

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 commented 4 months ago

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.