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

[23.0] vendor: update buildkit to latest v0.10 #44959

Merged
merged 1 commit into from Feb 9, 2023

Conversation

tonistiigi
Copy link
Member

Brings in moby/buildkit#3595

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@@ -49,7 +49,7 @@ require (
github.com/klauspost/compress v1.15.12
github.com/miekg/dns v1.1.43
github.com/mistifyio/go-zfs v2.1.2-0.20190413222219-f784269be439+incompatible
github.com/moby/buildkit v0.10.7-0.20230206124303-b8fdb4b78da0
github.com/moby/buildkit v0.10.7-0.20230208155512-4f0ee09c40e2
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to have a tagged release for this fix if possible, but I'm not familiar with the process in BuildKit. Are we able to complete a release before this revendoring?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're not planning to do any actual v0.10 releases anymore, as v0.11 is already out.

Copy link
Member

Choose a reason for hiding this comment

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

I'm really not sure how I feel about that -- this is a correctness fix for a bug in BuildKit and not a simple revendor. It really feels like there should be a tagged release, instead of us floating off the unreleased tip of the 0.10.x branch indefinitely...

@thaJeztah thaJeztah added this to the 23.0.1 milestone Feb 8, 2023
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

but agreed that it would be good to have a tagged version

Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

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

Registering my -1 based on the desire for a tagged release, or compelling technical reasons to not prefer one.

@tonistiigi
Copy link
Member Author

@neersighted Moby is like any other downstream for BuildKit. If somebody using buildkit needs to vendor a historic commit, we will not make a release for them. It has own release schedule and policy on what goes into releases. This was exactly the same for the v0.8 branch for any 20.10 releases that happened after BuildKit v0.9 was already out. If downstream doesn't like this, the option is to fork and manage own patches. Until the patches are reasonable, this is not needed, as BK maintainers will happily merge them even if the release has already been superseded.

@neersighted
Copy link
Member

This sounds like a weird twilight state in between "we don't maintain anything but the latest branch" and "we accept backports for older branches." Doing code review/gatekeeping what makes it into a previous release branch, but not cutting releases off that branch, seems like a very odd distinction to me (compare this to SwarmKit, which just does not do releases).

I don't necessarily want to block an important fix over reviewing/reconsidering BuildKit release policy, but frankly I don't understand the reasoning and it seems borderline user-hostile to me.

While it's not compelling from a technical standpoint, "This is how we've always done it" does carry weight and I can stifle some of my objections on that basis. However, it was mentioned:

It has own release schedule and policy on what goes into releases.

Where can I see that policy? If it's informal/was never codified beyond convention between maintainers, that does make me want to push back and ask for it to be reconsidered more.


Overall, having tagged releases makes it easier to reason about what code is being consumed by Moby, and helps with both provenance concerns as well as diagnostics (e.g. a untagged commit could actually come off of a PR branch and still say moby/buildkit). I really feel it would be best practice here to provide patch releases, or simply force us to maintain a soft fork and manage things as we please there.

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

I think it is ok to have a new release for v0.10.x, but the current v23 branch of Moby already uses an untagged commit of BuildKit, so I guess we can just merge this PR now and discuss releasing BK v0.10.x later

@AkihiroSuda
Copy link
Member

W.r.t. BK v0.10.x, maybe just create a tag on the git repo without releasing binaries?

Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

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

LGTM given we've agreed to discuss the release lifecycle and see if we can come up with something more beneficial for both projects (which might just be the status quo -- the only commitment I'd like to see is talking about it).

@neersighted neersighted merged commit bc3805a into moby:23.0 Feb 9, 2023
@neersighted
Copy link
Member

This fixed #44943

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants