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.

Sign up for GitHub

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

Jump to bottom

Use go run for tool dependencies, require go 1.17 #18874

Merged
merged 2 commits into from
Mar 16, 2022
Merged

Use go run for tool dependencies, require go 1.17 #18874

merged 2 commits into from
Mar 16, 2022

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Feb 24, 2022 •
edited
Loading

This ensures the tools only run in the versions we've tested and the method also does not polute PATH with those tools so they are truly isolated. Another benefit here is that tool dependencies are now all defined in one spot, making it easier to update them.

Fixes: #18867

@silverwind silverwind added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Feb 24, 2022
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 24, 2022
@silverwind silverwind added this to the 1.17.0 milestone Feb 24, 2022
@silverwind
Copy link
Member Author

silverwind commented Feb 24, 2022 •
edited
Loading

lint-backend-windows was giving a exec format error, presumably because having GOOS=windows set makes go run compile a windows binary, while the step still ran on linux. Not sure how to workaround, but I don't see much purpose in having this step, so I removed it.

@silverwind
Copy link
Member Author

silverwind commented Feb 24, 2022 •
edited
Loading

lint-backend-windows is now fixed as well via a somewhat acceptable workaround.

@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 Feb 25, 2022
@silverwind
Copy link
Member Author

Small tweak pushed that will speed up the lint-backend-* steps by using shared dependencies.

@silverwind
Copy link
Member Author

This syntax of go run requires 1.17, so I think we need to raise MIN_GO_VERSION.

@Gusted
Copy link
Contributor

Gusted commented Feb 28, 2022

This syntax of go run requires 1.17, so I think we need to raise MIN_GO_VERSION.

Gitea seems to follow to only support the 2 latest go versions, given 1.18 should be release soon this PR can wait until 1.18 has been dropped?

@silverwind
Copy link
Member Author

Yes, I suppose so, but it's not a hard dependency, it will still build on 1.16 too, just the tooling used on CI and by developers locally will require 1.17.

@Gusted
Copy link
Contributor

Gusted commented Mar 2, 2022

Should it be marked as breaking?

@silverwind
Use go run for tool dependencies, require go 1.17
55c5e05 
This ensures the tools only run in the versions we've tested and it also
does not polute PATH with those tools so they are truly isolated. This
syntax of `go run` requires go 1.17, so the minimum version is set
accordingly.

Fixes: go-gitea#18867
@silverwind silverwind changed the title Use go run to isolate tool dependencies Use go run for tool dependencies, require go 1.17 Mar 2, 2022
@silverwind silverwind added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Mar 2, 2022
@silverwind
Copy link
Member Author

I've bumped the minimum go version to 1.17 now. I guess if we are strict, this has to wait until go 1.18 release, which should be imminent.

@Gusted Gusted added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Mar 2, 2022
@Gusted Gusted removed the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Mar 15, 2022
@Gusted
Copy link
Contributor

Gusted commented Mar 15, 2022

I've bumped the minimum go version to 1.17 now. I guess if we are strict, this has to wait until go 1.18 release, which should be imminent.

https://groups.google.com/g/golang-announce/c/-cp44ypCT5s/m/wmegxkLiAQAJ

@techknowlogick techknowlogick mentioned this pull request Mar 15, 2022
Merged
@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 Mar 16, 2022
@techknowlogick techknowlogick merged commit fe9626a into go-gitea:main Mar 16, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 16, 2022
@zjjhot
Merge remote-tracking branch 'giteaoffical/main'
a43267b 
* giteaoffical/main:
  rm .sample hooks which aren't used (go-gitea#19101)
  use go1.18 to build gitea (go-gitea#19099)
  Use `go run` for tool dependencies, require go 1.17 (go-gitea#18874)
  Update golang.org/x/crypto (go-gitea#19097)
  Handle email address not exist. (go-gitea#19089)
techknowlogick added a commit to techknowlogick/gitea that referenced this pull request Mar 18, 2022
@techknowlogick
Use go run for tool dependencies, require go 1.17 (go-gitea#18874)
bb99f17 
This ensures the tools only run in the versions we've tested and it also
does not polute PATH with those tools so they are truly isolated. This
syntax of `go run` requires go 1.17, so the minimum version is set
accordingly.

Fixes: go-gitea#18867

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@zeripath zeripath mentioned this pull request Mar 18, 2022
Merged
6543 added a commit that referenced this pull request Mar 19, 2022
@techknowlogick @silverwind
@6543 @zeripath @MrGussio
Bump to build with go1.18 (#19120 et al) (#19127)
f9ea4ab 
Backport #19120 
Backport #19099 
Backport #18874 
Backport #18420
Backport #19128
Backport #18270 

Bump to build with go1.18

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Jelle Hulter <jellehulter@gmail.com>
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
@silverwind @techknowlogick
Use go run for tool dependencies, require go 1.17 (go-gitea#18874)
05d6fa9 
This ensures the tools only run in the versions we've tested and it also
does not polute PATH with those tools so they are truly isolated. This
syntax of `go run` requires go 1.17, so the minimum version is set
accordingly.

Fixes: go-gitea#18867

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@silverwind silverwind mentioned this pull request Apr 4, 2022
Closed
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
@6543 6543 added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Jun 18, 2022
@6543
Copy link
Member

6543 commented Jun 18, 2022

-> #19099 did surpass it

@silverwind silverwind deleted the run-tools branch June 20, 2022 07:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@techknowlogick techknowlogick techknowlogick approved these changes

@Gusted Gusted Gusted approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! 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
Milestone
1.17.0
Development

Successfully merging this pull request may close these issues.

Isolate golang tool dependencies
5 participants
@silverwind @Gusted @6543 @techknowlogick @GiteaBot