Skip to content

fix go version#34299

Merged
techknowlogick merged 2 commits into
go-gitea:mainfrom
trdthg:trdthg-patch-1
Apr 29, 2025
Merged

fix go version#34299
techknowlogick merged 2 commits into
go-gitea:mainfrom
trdthg:trdthg-patch-1

Conversation

@trdthg

@trdthg trdthg commented Apr 28, 2025

Copy link
Copy Markdown
Contributor

go cmd will download and cache a copy of the Go toolchain, go1.24 is not a valid version since golang/go#57631.


I am using a fresh installed riscv machine, trying to compile gitea locally. When I try to run the go command, it prompts that the toolchain of version 1.24 not avaliable. After updating go.mod, the download was successful.

$ go
go: downloading go1.24 (linux/riscv64)
go: download fo1.24 for linux/riscv64: toolchain not avaliable

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 28, 2025
@lunny

lunny commented Apr 28, 2025

Copy link
Copy Markdown
Member

@mengzhuo Since the riscv64 patch was contributed by you, maybe you can review this change. I haven't encounter the problem when building in other platform.

@mengzhuo

Copy link
Copy Markdown
Contributor

FYI:
golang/go#57631 means go.mod version should be explicitly with minor version go 1.24.0 instead of adding a toolchain go1.24.2

Comment thread go.mod Outdated
@@ -2,6 +2,8 @@ module code.gitea.io/gitea

go 1.24

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

go 1.24.0

Comment thread go.mod Outdated

go 1.24

toolchain go1.24.2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drop these 2 lines.

@trdthg trdthg force-pushed the trdthg-patch-1 branch 4 times, most recently from 57a1b77 to 02c64bc Compare April 28, 2025 06:24
go cmd will download and cache a copy of the Go toolchain, but go1.24 is not a valid version since golang/go#57631
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 28, 2025
@wxiaoguang

Copy link
Copy Markdown
Contributor

How the toolchain dependency is managed?

For example: if go 1.24.1 has security fixes, then will go 1.24.0 insist to use 1.24.0, or will use the latest one like 1.24.1 or even 1.24.10?

@mengzhuo

mengzhuo commented Apr 28, 2025

Copy link
Copy Markdown
Contributor

How the toolchain dependency is managed?

For example: if go 1.24.1 has security fixes, then will go 1.24.0 insist to use 1.24.0, or will use the latest one like 1.24.1 or even 1.24.10?

The mod treat go directive as "a required minimum version" instead of a CVE mandatory version.
Please check the mod document for further information.
https://go.dev/ref/mod

@silverwind

silverwind commented Apr 28, 2025

Copy link
Copy Markdown
Member

Why should we specify minimum version >=1.24.2 when we are compatible with >=1.24. Someone using 1.24.1 or 1.24.2 would not be able to build.

If anything, I would specify 1.24.0 which should be equivalent to 1.24 as far as go is concerned (I know the go tooling has a number of bugs surrounding this .0 suffix, and I think they will only be fixed come 1.25).

@trdthg

trdthg commented Apr 28, 2025

Copy link
Copy Markdown
Contributor Author

ci check failed because of security issues in 1.24.{0,1}

@silverwind

silverwind commented Apr 29, 2025

Copy link
Copy Markdown
Member

ci check failed because of security issues in 1.24.{0,1}

It shouldn't fail with go 1.24.0 in go.mod because I think setup-go will always use the highest possible patch version, right?

@silverwind

silverwind commented Apr 29, 2025

Copy link
Copy Markdown
Member

Ah, it appears setup-go makes a difference between 1.24 and 1.24.0:

https://github.com/actions/setup-go#getting-go-version-from-the-gomod-file

If a patch version is specified, that specific patch version will be used.
If no patch version is specified, it will search for the latest available patch version in the cache, versions-manifest.json, and the official Go language website, in that order.
If both the go-version and the go-version-file inputs are provided then the go-version input is used.

This is very undesired behaviour, but I think it's unlikely they'd want to change it.

I guess the best course of action is either not specifying a patch version in go.mod or specify a patch version in go.mod and add a go-version-file with the 1.24 format to get the auto-updating behaviour (so we don't have to update go.mod on every patch release).

@hiifong

hiifong commented Apr 29, 2025

Copy link
Copy Markdown
Member

Ah, it appears setup-go makes a difference between 1.24 and 1.24.0:

https://github.com/actions/setup-go#getting-go-version-from-the-gomod-file

If a patch version is specified, that specific patch version will be used.
If no patch version is specified, it will search for the latest available patch version in the cache, versions-manifest.json, and the official Go language website, in that order.
If both the go-version and the go-version-file inputs are provided then the go-version input is used.

This is very undesired behaviour, but I think it's unlikely they'd want to change it.

I guess the best course of action is either not specifying a patch version in go.mod or specify a patch version in go.mod and add a go-version-file with the 1.24 format to get the auto-updating behaviour (so we don't have to update go.mod on every patch release).

This method seems to only support setup-go, if I want to run it locally I still need to fix the go version to 1.24.2.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 29, 2025
@techknowlogick

Copy link
Copy Markdown
Member

Thanks for your first PR to the project! I think since we will have to bump go.mod regardless, and since actions/setup-go doesn't immediately bump its known latest version we sometimes have to wait on it when go patches are released, so we can skip go-version-file

@techknowlogick techknowlogick enabled auto-merge (squash) April 29, 2025 11:59
@techknowlogick techknowlogick added topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Apr 29, 2025
@techknowlogick techknowlogick merged commit 7bd2ce7 into go-gitea:main Apr 29, 2025
26 checks passed
@GiteaBot GiteaBot added this to the 1.25.0 milestone Apr 29, 2025
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 29, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 30, 2025
* giteaofficial/main:
  Fix some dropdown problems on the issue sidebar (go-gitea#34308)
  [skip ci] Updated translations via Crowdin
  Fix button alignments (go-gitea#34307)
  fix go version (go-gitea#34299)
  Fix the ci build (go-gitea#34309)
  support the open-icon of folder (go-gitea#34168)
  Fix wrong review requests when updating the pull request (go-gitea#34286)
  Enforce two-factor auth (2FA: TOTP or WebAuthn) (go-gitea#34187)
  actions artifacts api list/download check status upload confirmed (go-gitea#34273)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants