Skip to content

Conversation

@crazy-max
Copy link
Member

@crazy-max crazy-max added this to the v0.19.0 milestone Nov 7, 2024
@crazy-max crazy-max changed the title update go to 1.23 update to go 1.23 Nov 7, 2024
@crazy-max crazy-max mentioned this pull request Nov 7, 2024
@crazy-max crazy-max marked this pull request as ready for review November 7, 2024 13:07
@thaJeztah
Copy link
Member

Can you update the order of the commits?

  • The fix golangci-lint issues should probably be first
  • then the "update golangci", and then the other ones.
  • oh, and the G115 # integer overflow conversion (TODO: verify these) should be in the second one (as it's related to the golangci-lint update)

@thompson-shaun
Copy link
Collaborator

I think everything requested was addressed @thaJeztah - are we OK to merge this one?

ARG GOPLS_VERSION=v0.20.0
ARG GO_VERSION=1.23
ARG XX_VERSION=1.5.0
ARG GOLANGCI_LINT_VERSION=1.61.0
Copy link
Member

Choose a reason for hiding this comment

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

Why 61 in 62 is the latest?

Copy link
Member Author

Choose a reason for hiding this comment

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

Was released after my PR got opened 😅

Copy link
Member

Choose a reason for hiding this comment

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

As you're updating; might as well update this one as well (if it helps; I didn't need to make changes in moby when updating)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@thaJeztah
Copy link
Member

Looks like the first commit doesn't work; I'm guessing the older golangci-lint is not compatible with go1.23, so to be "correct", the go bump should probably go after the golangci-lint bump

@crazy-max
Copy link
Member Author

Looks like the first commit doesn't work; I'm guessing the older golangci-lint is not compatible with go1.23, so to be "correct", the go bump should probably go after the golangci-lint bump

Done

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

@crazy-max
Copy link
Member Author

crazy-max commented Nov 20, 2024

Ah fixes needed with latest golangci-lint 1.62 actually: https://github.com/docker/buildx/actions/runs/11930900417/job/33252637019?pr=2782#step:5:368

#19 [lint 1/1] RUN --mount=target=/go/src/github.com/docker/buildx     --mount=target=/root/.cache,type=cache,id=lint-cache-darwin/arm64     xx-go --wrap &&     golangci-lint run
#19 92.91 driver/remote/driver.go:51:8: lostcancel: the cancel function returned by context.WithTimeoutCause should be called, not discarded, to avoid a context leak (govet)
#19 92.91 		ctx, _ := context.WithTimeoutCause(cancelCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded))
#19 92.91 		     ^
#19 92.91 builder/builder.go:526:14: lostcancel: the cancel function returned by context.WithTimeoutCause should be called, not discarded, to avoid a context leak (govet)
#19 92.91 	timeoutCtx, _ := context.WithTimeoutCause(cancelCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded))
#19 92.91 	            ^
#19 92.91 commands/inspect.go:39:14: lostcancel: the cancel function returned by context.WithTimeoutCause should be called, not discarded, to avoid a context leak (govet)
#19 92.91 	timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded))
#19 92.91 	            ^
#19 92.91 commands/ls.go:62:14: lostcancel: the cancel function returned by context.WithTimeoutCause should be called, not discarded, to avoid a context leak (govet)
#19 92.91 	timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded))
#19 92.91 	            ^
#19 92.91 commands/rm.go:154:14: lostcancel: the cancel function returned by context.WithTimeoutCause should be called, not discarded, to avoid a context leak (govet)
#19 92.91 	timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 20*time.Second, errors.WithStack(context.DeadlineExceeded))
#19 92.91 	            ^

Seems a false positive though as we are handling cancel in parent already. Related to #2798

@crazy-max
Copy link
Member Author

Getting this one in

@crazy-max crazy-max merged commit 96689e5 into docker:master Nov 20, 2024
112 checks passed
@crazy-max crazy-max deleted the go-1.23 branch November 20, 2024 10:40
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.

4 participants