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

Update to Go1.21 #2377

Merged
merged 9 commits into from
Apr 23, 2024
Merged

Update to Go1.21 #2377

merged 9 commits into from
Apr 23, 2024

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Apr 15, 2024

@mtrmac mtrmac changed the title Go1.21 Update to Go1.21 Apr 15, 2024
@mtrmac
Copy link
Collaborator Author

mtrmac commented Apr 15, 2024

Note to self: Afterwards we should github.com/letsencrypt/boulder to avoid a dependency on no-longer-maintained gopkg.in/go-jose/go-jose.v2. (We don’t call it though that path AFAIK, still, we include it.)

@@ -1,6 +1,6 @@
module github.com/containers/image/v5

go 1.20
go 1.21
Copy link
Member

Choose a reason for hiding this comment

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

My understanding is that along with this, we also need toolchain 1.21 as well? Please double-check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My reading of “Go toolchain selection” in https://go.dev/doc/toolchain is that it is not compulsory, and effectively defaults to the go directive.

I don’t mind adding 1.21.0 here, if you think it would be more explicit. (Or 1.21.$latest ? That would force users to update.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added toolchain 1.21.0, per discussion in containers/skopeo#2297 (comment) .

@mtrmac
Copy link
Collaborator Author

mtrmac commented Apr 15, 2024

Cross-linking for visibility: containers/skopeo#2297 (comment) says we need an update to the Renovate config (affecting also repos using c/image).

@mtrmac
Copy link
Collaborator Author

mtrmac commented Apr 15, 2024

(Marking as draft to make sure we have a decision the Renovate configuration first; I understand Renovate would immediately start failing otherwise.)

@mtrmac mtrmac marked this pull request as draft April 15, 2024 19:46
@cevich
Copy link
Member

cevich commented Apr 15, 2024

(cross-post / copy)

I don't believe it will fail, it will simply (and mostly silently) refuse to open any module update PRs which also require a bump of go 1.21.

@cevich
Copy link
Member

cevich commented Apr 17, 2024

Ref: containers/automation#189

@mtrmac mtrmac marked this pull request as ready for review April 17, 2024 19:44
Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
... except where we need maps.Keys().

Should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
Should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
... now that Go 1.21 is acceptable.

Signed-off-by: Miloslav Trmač <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Apr 23, 2024

LGTM

@rhatdan rhatdan merged commit fb2056a into containers:main Apr 23, 2024
10 checks passed
@mtrmac mtrmac deleted the go1.21 branch April 24, 2024 17:13
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.

3 participants