-
-
Notifications
You must be signed in to change notification settings - Fork 635
test: Remove go1.24.x #8364
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
Merged
Merged
test: Remove go1.24.x #8364
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,6 @@ jobs: | |
| fail-fast: false | ||
| matrix: | ||
| GO_VERSION: | ||
| - "1.24.6" | ||
| - "1.25.0" | ||
| runs-on: ubuntu-24.04 | ||
| permissions: | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,6 @@ jobs: | |
| fail-fast: false | ||
| matrix: | ||
| GO_VERSION: | ||
| - "1.24.6" | ||
| - "1.25.0" | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| module github.com/letsencrypt/boulder | ||
|
|
||
| go 1.24.0 | ||
| go 1.25.0 | ||
|
|
||
| require ( | ||
| github.com/aws/aws-sdk-go-v2 v1.36.5 | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,7 @@ docker buildx build \ | |
| . | ||
|
|
||
| docker run boulder tar -C /opt/boulder -cpz . > "./boulder-${VERSION}-${COMMIT_ID}.${ARCH}.tar.gz" . | ||
| # Produces e.g. boulder-1.24.5.1754519595-591c0545.x86_64.deb | ||
| # Produces e.g. boulder-1.25.0.1754519595-591c0545.x86_64.deb | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really want to establish a pattern of updating this comment every time we update versions; it's good and fine for it to be out of date. |
||
| docker run -v .:/boulderrepo \ | ||
| -e "COMMIT_ID=$(git rev-parse --short=8 HEAD)" \ | ||
| -e "VERSION=${VERSION}" \ | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
nit: technically this should remain 1.24.0 until you land the PR which adds a dependency on the new csrf lib. But updating it now is fine in the grand scheme of things.
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.
Hm; yeah, this looks to be forcing our hands to drop support for go1.24 (which is still a supported release);
Given that this module is used as a library module, it's generally best to stick to MVS, and keep versions as low as possible.
Would it be possible for this project to test against
latest(stable) andlatest -1(oldstable) to not force consumers into dropping support for Go versions?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 can't generally guarantee compatibility with older releases. As alluded to by the comment above, this PR was immediately followed by #8365, which added a dependency on http.CrosOriginProtection, which is new in go1.25. So this is actually the minimum viable version.
Nothing in this repo is designed to be used as a library outside of Boulder -- all library packages here exist to support the binaries under //cmd. We're aware that some projects have taken dependencies on Boulder's library packages, and we try to support that as best we can, but our guiding principle is that we develop this repo for our own consumption.
We're discussing internally whether we can take on the maintenance burden of splitting goodkey out into its own repository or its own module within this repository, but I can't make any guarantees at this time.
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; that wasn't clear to me, and I can definitely relate to that. Apologies if my comment came across the wrong way; we've been hit many times by modules that are designed to be a module that over-aggressively update dependencies to "yolo-latest", which sometimes can cause an unwanted ripple-effect.
Go makes it really convenient to share code, but it's a bit of a double-edged-sword; when the language was still gaining early adopters, it was a nice way to share your work for others who may find it useful, but especially with Go modules now enforcing SemVer compatibility, that's become more of an issue.
We were in the process to try and reduce indirect dependencies through various upstreams which exploded our binary sizes and it looks indeed that in our case we inherited the dependency indirectly through other modules;
I feel the pain; we've just gone through the process of migrating github.com/moby/moby to use modules, and it was painful (splitting off modules where suitable, and - sometimes aggressively - moving things to
internal/packages); and we're not done yet!Either way; I appreciate you taking the time to reply, and outline the situation. Happy to think along if needed if you choose to split library modules (always feel free to
@me)