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

bump compose-go to v2.4.9 #3064

Merged
merged 2 commits into from
Mar 21, 2025
Merged

Conversation

glours
Copy link
Contributor

@glours glours commented Mar 14, 2025

No description provided.

go.mod Outdated
Comment on lines 3 to 5
go 1.23

toolchain go1.23.7
Copy link
Member

Choose a reason for hiding this comment

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

Remove toolchain from here:

Suggested change
go 1.23
toolchain go1.23.7
go 1.23

See #2577 (review)

go.mod Outdated
@@ -1,12 +1,14 @@
module github.com/docker/buildx

go 1.22.0
go 1.23
Copy link
Member

Choose a reason for hiding this comment

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

Hum lint seems to fail because gopls rules apply to version from go.mod: https://github.com/docker/buildx/actions/runs/13858143332/job/38779661844?pr=3064#step:4:519

 > [gopls-analyze 3/3] RUN --mount=target=.   --mount=target=/root/.cache,type=cache,id=lint-cache-${TARGETNAME}-darwin/amd64   --mount=target=/gopls-analyzers,from=gopls,source=/out <<EOF (set -ex...):
124.1 controller/pb/export.go:30:4: Replace m[k]=v loop with maps.Copy
125.7 # github.com/docker/buildx/store
125.7 # [github.com/docker/buildx/store]
125.7 store/nodegroup.go:97:5: Replace m[k]=v loop with maps.Copy
125.7 store/nodegroup.go:151:3: Replace m[k]=v loop with maps.Clone
126.0 # github.com/docker/buildx/util/imagetools
126.0 # [github.com/docker/buildx/util/imagetools]
126.0 util/imagetools/create.go:111:5: Replace m[k]=v loop with maps.Copy
126.0 util/imagetools/loader.go:131:4: Replace m[k]=v loop with maps.Clone
126.0 util/imagetools/loader.go:134:4: Replace m[k]=v loop with maps.Copy

Whereas for golangci-lint we enforce it to avoid this issue:

buildx/.golangci.yml

Lines 4 to 6 in 00fdcd3

# default uses Go version from the go.mod file, fallback on the env var
# `GOVERSION`, fallback on 1.17: https://golangci-lint.run/usage/configuration/#run-configuration
go: "1.23"

@tonistiigi Might just be a matter of setting GOVERSION env in

go vet -vettool=/gopls-analyzers/$analyzer ./...

@tonistiigi tonistiigi changed the title bump compose verstion to v2.34.0 bump compose version to v2.34.0 Mar 17, 2025
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Typo in commit title


require (
github.com/Masterminds/semver/v3 v3.2.1
github.com/Microsoft/go-winio v0.6.2
github.com/aws/aws-sdk-go-v2/config v1.27.27
github.com/compose-spec/compose-go/v2 v2.4.8
github.com/compose-spec/compose-go/v2 v2.4.9
Copy link
Member

Choose a reason for hiding this comment

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

I think PR title is wrong and should be bump compose-go to v2.4.9 😅

Signed-off-by: Guillaume Lours <[email protected]>
@crazy-max crazy-max force-pushed the bump-compose-v2.34.0 branch from 3d628de to 3e037bb Compare March 18, 2025 09:03
@crazy-max crazy-max changed the title bump compose version to v2.34.0 bump compose-go to v2.4.9 Mar 18, 2025
@crazy-max crazy-max force-pushed the bump-compose-v2.34.0 branch from 3e037bb to f2e5bec Compare March 18, 2025 09:08
@crazy-max
Copy link
Member

pushed extra commit to fix go.mod and lint issues

@@ -214,7 +212,7 @@ func validateComposeFile(dt []byte, fn string) (bool, error) {
}

func validateCompose(dt []byte, envs map[string]string) error {
_, err := loader.Load(composetypes.ConfigDetails{
_, err := loader.LoadWithContext(context.Background(), composetypes.ConfigDetails{
Copy link
Member

Choose a reason for hiding this comment

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

Don't we have a better context for this that we could pass in?

for k, v := range n.DriverOpts {
driverOpts[k] = v
}
driverOpts := maps.Clone(n.DriverOpts)
Copy link
Member

Choose a reason for hiding this comment

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

This is not 100% the same as it could leave driverOpts nil.

Note that modernize --fix does these updates automatically.

for k, v := range mfst.manifest.Annotations {
annotations[k] = v
}
annotations := maps.Clone(mfst.desc.Annotations)
Copy link
Member

Choose a reason for hiding this comment

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

Same issue with nil

Copy link
Member

Choose a reason for hiding this comment

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

Interesting that suggested fix was maps.Clone: https://github.com/docker/buildx/actions/runs/13858143332/job/38779661844#step:4:528

126.0 util/imagetools/loader.go:131:4: Replace m[k]=v loop with maps.Clone

But modernize --fix does a maps.Copy.

@crazy-max crazy-max force-pushed the bump-compose-v2.34.0 branch from f2e5bec to 212d598 Compare March 19, 2025 10:54
@tonistiigi tonistiigi merged commit 646df6d into docker:master Mar 21, 2025
138 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants