Skip to content

Conversation

jaxtew
Copy link
Contributor

@jaxtew jaxtew commented Aug 17, 2025

Addresses go version mismatch when using the devcontainer as a result of this commit (bumps Go version from 1.24.5 to 1.24.6)

The current official devcontainer Go image used in this repository (1.24-bookworm) uses 1.24.5 and sets GOTOOLCHAIN to local. This PR overrides it to auto so that build commands automatically update to the correct version.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 17, 2025
@silverwind
Copy link
Member

silverwind commented Aug 18, 2025

Is is really a problem? go should download any newer toolchain version automatically, so if you are on 1.24.5 and go.mod requires 1.24.6, go should download the 1.24.6 toolchain automatically on first invocation.

Also, in any case I find this too much of hack, there should only be one place in the repo where we specify the minimum go version, which is go.mod.

@jaxtew
Copy link
Contributor Author

jaxtew commented Aug 18, 2025

@silverwind Only a "problem" for anyone wanting the devcontainer to Just Work. Unless I was doing something wrong, which is entirely possible, the make deps command that automatically runs when the devcontainer finishes building will just exit instead of upgrading itself to use the minimum version. I haven't used Go in a while, maybe I'm missing something simple.

I agree with this being a hack.

@silverwind
Copy link
Member

Do have any logs from when it exits?

@silverwind
Copy link
Member

I suspect the bug may be in go-check:

gitea/Makefile

Lines 199 to 207 in 6619b1e

.PHONY: go-check
go-check:
$(eval MIN_GO_VERSION_STR := $(shell grep -Eo '^go\s+[0-9]+\.[0-9]+' go.mod | cut -d' ' -f2))
$(eval MIN_GO_VERSION := $(shell printf "%03d%03d" $(shell echo '$(MIN_GO_VERSION_STR)' | tr '.' ' ')))
$(eval GO_VERSION := $(shell printf "%03d%03d" $(shell $(GO) version | grep -Eo '[0-9]+\.[0-9]+' | tr '.' ' ');))
@if [ "$(GO_VERSION)" -lt "$(MIN_GO_VERSION)" ]; then \
echo "Gitea requires Go $(MIN_GO_VERSION_STR) or greater to build. You can get it at https://go.dev/dl/"; \
exit 1; \
fi

It only checks 2-part version syntax but we recently changed to 3-part.

Signed-off-by: Jackson Stewart <[email protected]>
@jaxtew
Copy link
Contributor Author

jaxtew commented Aug 18, 2025

I also found that setting GOTOOLCHAIN to auto gets it to upgrade itself. I think the packaged image sets it to local

@jaxtew
Copy link
Contributor Author

jaxtew commented Aug 18, 2025

Do have any logs from when it exits?

VS Code output:

[10843 ms] Start: Run in container: /bin/sh -c make deps
go mod download
go: go.mod requires go >= 1.24.6 (running go 1.24.5; GOTOOLCHAIN=local)
make: *** [Makefile:823: deps-backend] Error 1
[11756 ms] postCreateCommand failed with exit code 2. Skipping any further user-provided commands.
Done. Press any key to close the terminal.

@silverwind
Copy link
Member

silverwind commented Aug 18, 2025

Hmm yeah, GOTOOLCHAIN=local definitely is a problem. It worked before when we specified go 1.24 in go.mod, but with 3-digit syntax in go.mod added in 7bd2ce7, it has become strict. I wonder what the reason for GOTOOLCHAIN=local is in the devcontainer image.

@jaxtew
Copy link
Contributor Author

jaxtew commented Aug 18, 2025

Looks like the official Golang image has it set. Image references this issue as the proposal for setting it.

@techknowlogick
Copy link
Member

I think we could reasonably set it to auto in our devcontainer, as it makes sense for containers themselves, as it makes things less-deterministic (not that containers always are), but this is for development where it's a benefit for the developer to use the expected version instead of the one that is hardcoded in the image.

@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 Aug 18, 2025
@silverwind
Copy link
Member

silverwind commented Aug 18, 2025

According to devcontainers/images#921 (comment), devcontainer images only publish once a month, so they lag behind in go patch releases.

This is a stopgap solution. Ultimately, I would prefer if we set go.mod to minimum version "1.24.0" and stop updating go.mod on every patch release.

@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 Aug 19, 2025
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 19, 2025
@silverwind silverwind enabled auto-merge (squash) August 19, 2025 11:37
@silverwind silverwind merged commit 463016b into go-gitea:main Aug 19, 2025
24 checks passed
@GiteaBot GiteaBot added this to the 1.25.0 milestone Aug 19, 2025
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 19, 2025
@techknowlogick
Copy link
Member

Ultimately, I would prefer if we set go.mod to minimum version "1.24.0" and stop updating go.mod on every patch release.

That's my preference too. Sadly, we cannot, as it flags for several widely used "CVE" scanners, creating a headache as people report false positives.

@silverwind
Copy link
Member

Sounds like those scanners misinterpret the go directive. It's a minimum version, not the version being ran (which should be assumed to be latest patch release). Can you link me to said scanners that have this problem?

zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 20, 2025
* giteaofficial/main:
  exit with success when already up to date (go-gitea#35312)
  Update to [email protected] (go-gitea#35310)
  Upgrade devcontainer go version to 1.24.6 (go-gitea#35298)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/internal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants