Check go and nodejs version by go.mod and package.json, update Go official site URL #19197

gesangtome · 2022-03-24T11:55:39Z

Looks like upstream has removed support built
with GO 1.16, the string should be updated here.

Looks like upstream has removed support built with GO 1.16, the string should be updated here.

wxiaoguang · 2022-03-24T12:00:32Z

Maybe we can remove the whole go-check and MIN_GO_VERSION, because the go.mod can take the work.

go mod tidy: go.mod file indicates go 1.17, but maximum supported version is 1.16

gitea/go.mod

Line 3 in def5456

go 1.17

silverwind

Change is good in any case, it was missed in fe9626a

silverwind · 2022-03-24T12:36:39Z

Maybe we can remove the whole go-check and MIN_GO_VERSION, because the go.mod can take the work.

I think go.mod is meant to declare the current (e.g. maximum) version, not the minimum one.

wxiaoguang · 2022-03-24T12:38:32Z

I think go.mod is meant to declare the current (e.g. maximum) version, not the minimum one.

Why? Then why can I compile Gitea with my Go1.18 ..........

silverwind · 2022-03-24T12:38:56Z

Actually, I'm wrong, it specifies minimum as per https://go.dev/doc/modules/gomod-ref#go, so it might be an option to just parse it out of go.mod, but I think it's material for another PR.

wxiaoguang · 2022-03-24T12:39:41Z

Why another? Go will report the correct required version automatically by go.mod now, what's the necessary of the old age go-check?

silverwind · 2022-03-24T12:41:32Z

Will go fail to build if the version is too low? If yes, than we can actually remove the check.

gesangtome · 2022-03-24T12:49:16Z

Doesn't gitea have a minimum go version requirement?
@silverwind @wxiaoguang

wxiaoguang · 2022-03-24T13:01:24Z

Doesn't gitea have a minimum go version requirement?

Yes, only the official supported Golang releases (the latest 2 minor versions) are supported.

Will go fail to build if the version is too low? If yes, than we can actually remove the check.

Starting from 1.16, the go build could report correct messages. However, Go 1.15 or older can not report correct messages.

So I think it's fine to remove the go-check, because Go1.15 is quite old now.

And it's also fine to keep the go-check, and maybe the version can be read from the go.mod, then reduce a magic number in Makefile.

$ /usr/local/go-1.16/bin/go build
# code.gitea.io/gitea/modules/hostmatcher
modules/hostmatcher/hostmatcher.go:105:34: ip.IsPrivate undefined (type net.IP has no field or method IsPrivate)
modules/hostmatcher/hostmatcher.go:109:9: ip.IsPrivate undefined (type net.IP has no field or method IsPrivate)
note: module requires Go 1.17

$ /usr/local/go-1.15/bin/go build
../../go/pkg/mod/github.com/caddyserver/certmagic@v0.15.4/filestorage.go:23:2: package io/fs is not in GOROOT (/usr/local/go-1.15/src/io/fs)

silverwind · 2022-03-24T13:17:48Z

And it's also fine to keep the go-check, and maybe the version can be read from the go.mod, then reduce a magic number in Makefile.

Yes, I support this. Same can be done for node-check as well which defines its minimum version in package.json, but I guess parsing it without additional dependencies will be a bit ugly.

wxiaoguang · 2022-03-24T13:44:30Z

I updated the go version parsing, and tested locally.

gesangtome · 2022-03-24T14:00:55Z

And it's also fine to keep the go-check, and maybe the version can be read from the go.mod, then reduce a magic number in Makefile.

Yes, I support this. Same can be done for node-check as well which defines its minimum version in package.json, but I guess parsing it without additional dependencies will be a bit ugly.

Make go-check emit version hints when GO < 1.16?
If yes, we can do this:

diff --git a/Makefile b/Makefile
index 613c08326..5c066fd3b 100644
--- a/Makefile
+++ b/Makefile
@@ -25,7 +25,7 @@ HAS_GO = $(shell hash $(GO) > /dev/null 2>&1 && echo "GO" || echo "NOGO" )
COMMA := ,

XGO_VERSION := go-1.18.x
-MIN_GO_VERSION := 001017000
+MIN_GO_VERSION := 001016000
MIN_NODE_VERSION := 012017000

AIR_PACKAGE ?= github.com/cosmtrek/air@v1.29.0
@@ -205,7 +205,7 @@ help:
go-check:
$(eval GO_VERSION := $(shell printf "%03d%03d%03d" $(shell $(GO) version | grep -Eo '[0-9]+.[0-9.]+' | tr '.' ' ');))
@if [ "$(GO_VERSION)" -lt "$(MIN_GO_VERSION)" ]; then \

          echo "Gitea requires Go 1.16 or greater to build. You can get it at https://golang.org/dl/"; \

          echo "Sorry, your GO version is too low, Gitea requests an updated GO to build. You can get it at https://golang.org/dl/"; \
          exit 1; \
  fi

wxiaoguang · 2022-03-24T14:20:01Z

@silverwind I also make this PR use the nodejs version from package.json

Makefile

silverwind · 2022-03-24T16:15:40Z

Makefile

+MIN_GO_VERSION_STR := $(shell grep -Eo '^go\s+[0-9]+\.[0-9.]+' go.mod | cut -d' ' -f2)
+MIN_GO_VERSION := $(shell printf "%03d%03d%03d" $(shell echo '$(MIN_GO_VERSION_STR)' | tr '.' ' '))
+
+MIN_NODE_VERSION_STR := $(shell grep -Eo '"node":.*[0-9.]+"' package.json | sed -n 's/.*[^0-9.]\([0-9.]*\)"/\1/p')


May as well extract the third version segment as well because node version is always 3-segments as opposed to go version which is 2.

It already has the 3-segments. The regex is [0-9.]*

silverwind · 2022-03-24T16:17:07Z

Makefile

-MIN_NODE_VERSION := 012017000
+
+MIN_GO_VERSION_STR := $(shell grep -Eo '^go\s+[0-9]+\.[0-9.]+' go.mod | cut -d' ' -f2)
+MIN_GO_VERSION := $(shell printf "%03d%03d%03d" $(shell echo '$(MIN_GO_VERSION_STR)' | tr '.' ' '))


The line above extracts 2 segments, but this seems to output 3. Is it correct?

It already has the 3-segments. The regex is [0-9.]*

silverwind · 2022-03-24T16:21:57Z

Also, I strongly suggest not doing such shell operation on top-level make scope because they will execute whenever the Makefile is invoked which is both wasteful and may produce errors in case when node is absent. It should only be done in eval statements which execute only when the specific target runs.

wxiaoguang · 2022-03-24T16:46:33Z

Also, I strongly suggest not doing such shell operation on top-level make scope because they will execute whenever the Makefile is invoked which is both wasteful and may produce errors in case when node is absent. It should only be done in eval statements which execute only when the specific target runs.

Moved these lines into go-check and node-check

gesangtome · 2022-03-24T17:11:47Z

@wxiaoguang @silverwind

Things are going in a good direction, great, looking forward to it being fixed.

codecov-commenter · 2022-03-26T09:51:42Z

Codecov Report

Merging #19197 (948516d) into main (49c5fc5) will increase coverage by 0.02%.
The diff coverage is 39.03%.

@@            Coverage Diff             @@
##             main   #19197      +/-   ##
==========================================
+ Coverage   46.55%   46.58%   +0.02%     
==========================================
  Files         856      858       +2     
  Lines      123018   123128     +110     
==========================================
+ Hits        57277    57365      +88     
- Misses      58814    58833      +19     
- Partials     6927     6930       +3

Impacted Files	Coverage Δ
cmd/admin.go	`0.00% <0.00%> (ø)`
cmd/hook.go	`7.11% <0.00%> (ø)`
cmd/serv.go	`2.39% <0.00%> (ø)`
cmd/web_acme.go	`0.00% <0.00%> (ø)`
models/asymkey/ssh_key_deploy.go	`55.55% <ø> (+1.13%)`	⬆️
models/helper_environment.go	`100.00% <ø> (ø)`
modules/context/permission.go	`25.39% <0.00%> (ø)`
modules/doctor/checkOldArchives.go	`22.85% <0.00%> (ø)`
modules/doctor/fix16961.go	`35.06% <0.00%> (ø)`
modules/doctor/mergebase.go	`10.25% <0.00%> (ø)`
... and 198 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59b867d...948516d. Read the comment docs.

* giteaofficial/main: Check go and nodejs version by go.mod and package.json (go-gitea#19197) Add `ContextUser` to http request context (go-gitea#18798) Set OpenGraph title to DisplayName in profile pages (go-gitea#19206)

* Check go and nodejs version by go.mod and package.json * Update Go official site URL Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

* Check go and nodejs version by go.mod and package.json * Update Go official site URL Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: gesangtome <gesangtome@foxmail.com>

Update string for go-check

8ba8f67

Looks like upstream has removed support built with GO 1.16, the string should be updated here.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 24, 2022

silverwind approved these changes Mar 24, 2022

View reviewed changes

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 Mar 24, 2022

wxiaoguang added 2 commits March 24, 2022 21:41

Use go version in go.mod instead of magic number

65e3d54

use new golang site url

402d1c7

use grep directly instead of cat

fe7b886

Revmoe magic number for nodejs version check

1d8fff7

wxiaoguang changed the title ~~Update string for go-check~~ Check go and nodejs version by go.mod and package.json, update Go official site URL Mar 24, 2022

wxiaoguang 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. labels Mar 24, 2022

wxiaoguang approved these changes Mar 24, 2022

View reviewed changes

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 Mar 24, 2022

remove unnecessary MIN_NODE_VER_FMT

d1d2df3

silverwind self-requested a review March 24, 2022 15:30

fix message string

8e4ae0a

silverwind reviewed Mar 24, 2022

View reviewed changes

Makefile Outdated Show resolved Hide resolved

silverwind reviewed Mar 24, 2022

View reviewed changes

Update Makefile

124ff3d

wxiaoguang requested a review from silverwind March 24, 2022 16:52

Merge branch 'main' into main

220b3cc

silverwind approved these changes Mar 25, 2022

View reviewed changes

wxiaoguang added 3 commits March 25, 2022 16:50

Merge branch 'main' into main

6cc8fe2

Merge branch 'main' into main

af61490

Merge branch 'main' into main

948516d

wxiaoguang merged commit c119828 into go-gitea:main Mar 26, 2022

wxiaoguang mentioned this pull request Mar 29, 2022

Check go and nodejs version by go.mod and package.json (#19197) #19254

Merged

wxiaoguang linked an issue Mar 29, 2022 that may be closed by this pull request

Error message when compiling with a too-old version of Go requests the wrong version #19253

Closed

wxiaoguang removed a link to an issue Mar 29, 2022

Error message when compiling with a too-old version of Go requests the wrong version #19253

Closed

go-gitea locked and limited conversation to collaborators Apr 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check go and nodejs version by go.mod and package.json, update Go official site URL #19197

Check go and nodejs version by go.mod and package.json, update Go official site URL #19197

gesangtome commented Mar 24, 2022

wxiaoguang commented Mar 24, 2022 •

edited

Loading

silverwind left a comment

silverwind commented Mar 24, 2022 •

edited

Loading

wxiaoguang commented Mar 24, 2022

silverwind commented Mar 24, 2022 •

edited

Loading

wxiaoguang commented Mar 24, 2022 •

edited

Loading

silverwind commented Mar 24, 2022 •

edited

Loading

gesangtome commented Mar 24, 2022 •

edited

Loading

wxiaoguang commented Mar 24, 2022

silverwind commented Mar 24, 2022

wxiaoguang commented Mar 24, 2022

gesangtome commented Mar 24, 2022

wxiaoguang commented Mar 24, 2022

silverwind Mar 24, 2022 •

edited

Loading

wxiaoguang Mar 24, 2022

silverwind Mar 24, 2022

wxiaoguang Mar 24, 2022 •

edited

Loading

silverwind commented Mar 24, 2022

wxiaoguang commented Mar 24, 2022

gesangtome commented Mar 24, 2022

codecov-commenter commented Mar 26, 2022

Check go and nodejs version by go.mod and package.json, update Go official site URL #19197

Check go and nodejs version by go.mod and package.json, update Go official site URL #19197

Conversation

gesangtome commented Mar 24, 2022

wxiaoguang commented Mar 24, 2022 • edited Loading

silverwind left a comment

Choose a reason for hiding this comment

silverwind commented Mar 24, 2022 • edited Loading

wxiaoguang commented Mar 24, 2022

silverwind commented Mar 24, 2022 • edited Loading

wxiaoguang commented Mar 24, 2022 • edited Loading

silverwind commented Mar 24, 2022 • edited Loading

gesangtome commented Mar 24, 2022 • edited Loading

wxiaoguang commented Mar 24, 2022

silverwind commented Mar 24, 2022

wxiaoguang commented Mar 24, 2022

gesangtome commented Mar 24, 2022

wxiaoguang commented Mar 24, 2022

silverwind Mar 24, 2022 • edited Loading

Choose a reason for hiding this comment

wxiaoguang Mar 24, 2022

Choose a reason for hiding this comment

silverwind Mar 24, 2022

Choose a reason for hiding this comment

wxiaoguang Mar 24, 2022 • edited Loading

Choose a reason for hiding this comment

silverwind commented Mar 24, 2022

wxiaoguang commented Mar 24, 2022

gesangtome commented Mar 24, 2022

codecov-commenter commented Mar 26, 2022

Codecov Report

wxiaoguang commented Mar 24, 2022 •

edited

Loading

silverwind commented Mar 24, 2022 •

edited

Loading

silverwind commented Mar 24, 2022 •

edited

Loading

wxiaoguang commented Mar 24, 2022 •

edited

Loading

silverwind commented Mar 24, 2022 •

edited

Loading

gesangtome commented Mar 24, 2022 •

edited

Loading

silverwind Mar 24, 2022 •

edited

Loading

wxiaoguang Mar 24, 2022 •

edited

Loading