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

integrate github.com/moby/buildkit/util/appcontext #131

Closed
wants to merge 13 commits into from

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Aug 31, 2023

dnephin and others added 12 commits April 17, 2017 17:40
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
support building on windows (no functional worker yet)
Fix for logrus rename, use fork until fixed in moby.
Removed unused tar stream.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit bc9a83144c83e9fd78007b7bfe92e8082c59d40e)
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
…ile-ref

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: coryb <cbennett@netflix.com>
…text

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@thaJeztah
Copy link
Member

Thanks! As mentioned on Slack, we probably need to look if docker/cli actually should be using this package; also not 100% sure if moby/sys is the best location for it (but if it still makes sense to have it as a separate module, we can create a repository for it).

@neersighted
Copy link
Member

Yeah, I don't think this is something we want to do to support the CLI; if the behavior we come up for with the CLI is useful, we can consider moving it here, but I'm not convinced that the logic is generic/fully re-usable yet.

Maybe this makes sense for CLI plugins, but I don't think this is the right module to re-use the code from if we do that.

@crazy-max
Copy link
Member Author

Created https://github.com/crazy-max/appcontext that we could move to moby org if that makes sense

@thaJeztah
Copy link
Member

@crazy-max the CLI no longer uses the package; where are the remaining uses? of it? (considering if it's gonna be used in multiple projects or not)

@crazy-max
Copy link
Member Author

crazy-max commented Dec 17, 2023

Ah right didn't catch docker/cli#4599 removed the dep! All good then!

PS: Does docker/cli#4599 needs a follow-up on buildx ?

@crazy-max crazy-max closed this Dec 17, 2023
@thaJeztah
Copy link
Member

PS: Does docker/cli#4599 needs a follow-up on buildx ?

Yes, it does! Or at least: the change should be backward-compatible (CLI has fallbacks), but I think @laurazard was planning to open PR's in compose and buildx to update the code to take advantage of the new handling.

I guess; feel free to start making those changes though (and I'm sure @laurazard would be willing to help review them 😄)

@laurazard
Copy link
Member

Indeed! I have one mostly ready for Compose, was going to start digging into Buildx code this week, happy to do that or review any PRs related to it :)

@crazy-max
Copy link
Member Author

Indeed! I have one mostly ready for Compose, was going to start digging into Buildx code this week, happy to do that or review any PRs related to it :)

Feel free to do so, you seem to have more context (no pun intended) than me 😅

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

Successfully merging this pull request may close these issues.

9 participants