caddyserver / caddy

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS
https://caddyserver.com
Apache License 2.0
57.43k stars 4k forks source link

chore: downgrade minimum Go version in go.mod #6318

Closed mohammed90 closed 4 months ago

mohammed90 commented 4 months ago

Just like caddyserver/certmagic#289, the dependency of caddyserver/zerossl forced Caddy to require go1.22. None of them have a hard dependency on the language semantics of 1.22, as far as I know. It's causing pains when building custom Caddy using the builder image variant. It's best to push out the new builder variant with the newer version of Go before upgrading the minimum required version here.

This PR is only ready once caddyserver/certmagic#289 is merged. Then we can only validate our language semantics need and (most likely) merge this PR.

Depends on caddyserver/certmagic#289

mholt commented 4 months ago

This is weird, the tests on IBM Z are failing consistently with this change.

I have no idea why automerge happened because that test is failing. (Oh, maybe it's not required to pass before merging?)

francislavoie commented 4 months ago

We have go version go1.22.3 linux/s390x on that machine. That shouldn't be a problem :thinking:

francislavoie commented 4 months ago

I re-ran the job another time, it passed this time https://github.com/caddyserver/caddy/actions/runs/9101519695/job/25020869066

mholt commented 4 months ago

GitHub needs a :shrug: emoji reaction

brotbert commented 4 months ago

@mholt

(For whatever it's worth: I only just learned that as of Go 1.21, the go directive in go.mod is a minimum required Go version, and that is a new breaking change in the Go toolchain.)

But you did it again (now in caddy-webdav): https://github.com/mholt/caddy-webdav/commit/bfcd3903f21e5cdaac29ed47777356a8a18deaf5 😬

francislavoie commented 4 months ago

You're right @brotbert, opened https://github.com/mholt/caddy-webdav/pull/43 to solve it.

mholt commented 4 months ago

Oh, wait... but I didn't change that. I mean, clearly I did, but I did not edit those lines.

I wonder if this was done by my editor on my laptop where I have Go 1.22 installed and I used the "upgrade transitive dependencies" feature.

Sorry I didn't see that. How surprising and unfortunate :confused:

mohammed90 commented 4 months ago

Oh, wait... but I didn't change that. I mean, clearly I did, but I did not edit those lines.

I wonder if this was done by my editor on my laptop where I have Go 1.22 installed and I used the "upgrade transitive dependencies" feature.

Sorry I didn't see that. How surprising and unfortunate :confused:

You upgraded a dependency whose new version requires go1.22, which forced the edit of that line in go.mod. it's not because you have go1.22 installed.

mholt commented 4 months ago

This still seems like a problem though. If we can't upgrade dependencies then how do we upgrade dependencies?

francislavoie commented 4 months ago

You just upgrade the one dependency you need to upgrade by hand (edit go.mod), then run go mod tidy. That's all. Don't run go get -u or w/e.

But only upgrade deps if necessary. Don't bump the Caddy version unless necessary (i.e. using a new API, or using an API that changed), and don't use new APIs before the stable release of Caddy otherwise anyone on the current stable won't be able to build anymore (unless they target the previous tag/commit, but majority of users won't, they'll try to use latest).

mohammed90 commented 4 months ago

Think of it this way: When you contract Company A for some work who sub-contracts some of its work to Company B, you wouldn't go to Company B directly to change their terms with Company A behind Company A's back.

mholt commented 4 months ago

It's just weird that the editor does that automatically when it knows there's not a func main() in it.

We will just have to accept plugins raising all the alarm bells of old versions from naive vulnerability scanners.

EdenSpire commented 4 months ago

I'm happy everything is sorted. I spent hours to find the culprit.

Thanks for fast response guys!!