-
Notifications
You must be signed in to change notification settings - Fork 5.7k
migrate to moby modules #13078
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
base: main
Are you sure you want to change the base?
migrate to moby modules #13078
Conversation
a5b199b to
73bb946
Compare
|
Nice to see moby API as a dedicated module, so we don't get transitive dependencies from engine inside Compose ! ❤️ |
| @@ -0,0 +1,89 @@ | |||
| // Package urlutil provides helper function to check if a given build-context | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this fork temporarily; didn't realise compose itself also uses it (not only through cli); the CLI fork is "internal", so probably needs to made public, or we keep a fork here 😞
|
Remaining uses of docker/docker; |
|
Build failure looks like it may be an incompatible change in buildx itself? |
|
OK, it's because buildx depends on an unreleased version of BuildKit, so go modules considers it to be older than the latest release and won't update; https://github.com/docker/buildx/blob/3f4bf829d8c5b92a8f670609417d9aa4c0b6be98/go.mod#L32 |
83ca48f to
13827c4
Compare
|
|
Yes, or at least the type; the utilities could be "client" - those packages are messy. Also looking at things that are in the api, but shouldn't (api/types/plugins/logdriver) |
|
Also included a (WIP) branch for docker/cli#6202 Before/After; DetailsBefore: After: |
d2146d4 to
3a7124d
Compare
3b1ec4c to
6cd56af
Compare
|
Down to two packages; both possibly candidates for CLI still needs sorting out; Details |
6cd56af to
8925b9c
Compare
d7c329f to
04ac165
Compare
ce597b5 to
8797fa0
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
8797fa0 to
b6784a8
Compare
b6784a8 to
3dbbaa6
Compare
6510727 to
fae6d96
Compare
f93c1f2 to
88078fb
Compare
go.mod
Outdated
| @@ -18,7 +18,6 @@ require ( | |||
| github.com/docker/cli v29.2.0+incompatible | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! Thought I had updated to v29.2.1; let me do so as well (no significant changes I think though)
Also update TestDefaultNetworkSettings: Test that the network with the highest priority is returned as "primary" network, and other networks as extra networks. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Format layer progress details with minimal efforts as new UI does not render individual layers Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Format layer progress details with minimal efforts as new UI does not render individual layers. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
88078fb to
47a2724
Compare
glours
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me, just 2 points we need to check
| func (s *composeService) pruneDanglingImagesOnRebuild(ctx context.Context, projectName string, imageNameToIdMap map[string]string) { | ||
| images, err := s.apiClient().ImageList(ctx, image.ListOptions{ | ||
| Filters: filters.NewArgs( | ||
| filters.Arg("dangling", "true"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We lost the projectLabel filter, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me check!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right, so it now uses projectFilter;
images, err := s.apiClient().ImageList(ctx, client.ImageListOptions{
Filters: projectFilter(projectName).Add("dangling", "true"),
})which should add it;
compose/pkg/compose/filters.go
Lines 27 to 29 in c75e494
| func projectFilter(projectName string) client.Filters { | |
| return make(client.Filters).Add("label", fmt.Sprintf("%s=%s", api.ProjectLabel, projectName)) | |
| } |
| containers, err := s.apiClient().ContainerList(ctx, container.ListOptions{ | ||
| All: true, | ||
| Filters: filters.NewArgs( | ||
| filters.Arg("label", fmt.Sprintf("%s=%s", api.ProjectLabel, project.Name)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here as well, so I think we should be good 🫶
What I did
Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did