-
-
Notifications
You must be signed in to change notification settings - Fork 633
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
test: Remove go1.24.x #8364
Conversation
aarongable
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.
LGTM with optional nits
| module github.com/letsencrypt/boulder | ||
|
|
||
| go 1.24.0 | ||
| go 1.25.0 |
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) and latest -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.
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.
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;
go mod graph | grep 'github.com/letsencrypt/boulder'
github.com/sigstore/rekor@v1.4.3 github.com/letsencrypt/boulder@v0.0.0-20240620165639-de9c06129bec
github.com/sigstore/rekor-tiles/v2@v2.0.1 github.com/letsencrypt/boulder@v0.0.0-20240620165639-de9c06129bec
github.com/sigstore/sigstore@v1.10.0 github.com/letsencrypt/boulder@v0.20251110.0
github.com/sigstore/sigstore-go@v1.1.4-0.20251124094504-b5fe07a5a7d7 github.com/letsencrypt/boulder@v0.20251110.0
github.com/sigstore/timestamp-authority/v2@v2.0.2 github.com/letsencrypt/boulder@v0.20251110.0
github.com/theupdateframework/go-tuf/v2@v2.3.0 github.com/letsencrypt/boulder@v0.0.0-20240620165639-de9c06129becWe'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.
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)
|
|
||
| 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 |
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.
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.
No description provided.