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

fix: use Go 1.20 for 2.6 builders #298

Merged
merged 1 commit into from May 23, 2023

Conversation

dunglas
Copy link
Contributor

@dunglas dunglas commented May 22, 2023

Recent versions of Caddy 2.6 require Go 1.20 to compile successfully. This patch upgrades the Go version for the builders.

Fixes dunglas/mercure#770.

@francislavoie
Copy link
Member

francislavoie commented May 22, 2023

That doesn't really make sense... Caddy v2.6.4 builds on Go 1.19 and Go 1.20 (and CI tests against both those versions). Plugins should at least follow Caddy's minimum supported Go version.

v2.7.0 will bump it to 1.20 anyways. We already released the 2.7.0-beta.1 tag on Docker which can be used for now.

@dunglas
Copy link
Contributor Author

dunglas commented May 22, 2023

The problem seems to be a dependency of Caddy 2.6: dunglas/mercure#770 (comment)

I didn't dig deep yet but it doesn't look related to plugins.

@dunglas
Copy link
Contributor Author

dunglas commented May 22, 2023

Actually, it's probably our go.mod that requires a newer version of < some dependency > than the go.mod shipped by Caddy.

Anyway, the error message is weird, it should tell that the package X isn't compatible with Go 1.19 instead of complaining about a package part of the stdlib that has been introduced in Go 1.20.

@francislavoie
Copy link
Member

I agree, Go's module/build tooling has been a headache for us lately because of the plugin setup. If you look at it the wrong way, it breaks in crazy ways. There's not much we can do in xcaddy because it's just a thin wrapper over running go get, go mod tidy and go build in that order.

@dunglas
Copy link
Contributor Author

dunglas commented May 23, 2023

Regardless of this issue, is there a good reason to not build the latest stable version of Caddy with the latest stable version of Go?

@NorthBlue333
Copy link

NorthBlue333 commented May 23, 2023

We currently have a project in production using Symfony and Caddy.
Our pipelines are currently down due to this issue.

v2.7.0 will bump it to 1.20 anyways. We already released the 2.7.0-beta.1 tag on Docker which can be used for now.

I wanted to use the 2.7 but I see it is still in beta with a warning. I do not want to use a beta version in production.

@dunglas I see you have fixed the issue in symfony-docker https://github.com/dunglas/symfony-docker/pull/407/files but the last changed line (FROM caddy:2-alpine AS app_caddy) uses the 2.7 am I right?

Also, regarding your last question @dunglas, if I understand correctly you would prefer not to statify the language version? Seems to be mandatory to statify the versions to me (at least major/minor).

@dunglas
Copy link
Contributor Author

dunglas commented May 23, 2023

@NorthBlue333 my patch does use the stable version of Caddy (the version is passed explicitly to xcaddy). It's just a trick to use the 2.7 version of the builder (not of Caddy itself), which is shipped with Go 1.20 (while version 2.6 of the builder image is using Go 1.19).

Regarding my last question, Caddy 2.6 officially supports both Go 1.19 and Go 1.20, so it's safe to use Go 1.20 even for production builds. Compiling with Go 1.20 may also improve the performance a bit. The only problem I see with compiling with Go 1.20 is if some plugins or extra libs you use aren't compatible yet with Go 1.20 (which is very unlikely, and as we can see here, some libs already have dropped support for Go 1.19).

FROM caddy:2-alpine AS app_caddy is just an unrelated simplification. Beta versions of this Docker image aren't tagged with the 2 tag (2 is currently an alias of 2.6, and will be an alias of 2.7 when this version will be marked stable).

@mholt
Copy link
Member

mholt commented May 23, 2023

@dunglas

Regardless of this issue, is there a good reason to not build the latest stable version of Caddy with the latest stable version of Go?

Nope, not that I can think of -- I think once upon a time we had some upgrade pains but I think that was just related to modules. Oh, and one time they made a breaking change in the name of security that we had to accommodate for, but I think those are the only major issues we've had upgrading Go. Generally Go upgrades are free wins for Caddy servers 😃

Due to certain Linux distros lagging behind, we try to support the previous Go release as well as the current. So right now, we build on Go 1.19 and Go 1.20.

@dunglas
Copy link
Contributor Author

dunglas commented May 23, 2023

So IIUC nothing prevents merging this PR 😄
This will have the side effect of also fixing all builds that are currently failing!

@mholt
Copy link
Member

mholt commented May 23, 2023

This will have the side effect of also fixing all builds that are currently failing!

I still don't understand why, but let me verify with @hairyhenderson that this is OK first -- I see nothing wrong with it with my limited understanding though.

Copy link
Contributor

@hairyhenderson hairyhenderson left a comment

Choose a reason for hiding this comment

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

Seems fine to me. I'll handle the followup work after this merges.

@hairyhenderson hairyhenderson merged commit c2fe0d6 into caddyserver:master May 23, 2023
2 checks passed
@dunglas
Copy link
Contributor Author

dunglas commented May 23, 2023

@mholt Because compiling with Go 1.20 fixes the issue of the lib that needs a feature of the stdlib that has been introduced in 1.20.

@mholt
Copy link
Member

mholt commented May 23, 2023

Huh, ok. Makes sense: caddy does not require go 1.20 (yet) but a plugin does. Got it.

Thanks for all you contribute!

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.

v0.14.8 breaking build
5 participants