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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Golang 1.18 for Gitea 1.17 release #19918

Merged
merged 7 commits into from
Jun 10, 2022
Merged

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Jun 8, 2022

Use Golang 1.18 (as minimal requirement) for Gitea 1.17 release, make sure the Golang version is still actively supported during Gitea 1.17 lifecycle.

And Gitea can start using Golang generics from then on.
update: As discussed in #19917 (comment) , only use it when it's carefully designed.


(sorry for that I opened a similar issue and opened this PR, I was going to ask to delete the issue, but soon it contains more important information 馃槀)

@wxiaoguang wxiaoguang added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Jun 8, 2022
@wxiaoguang wxiaoguang added this to the 1.17.0 milestone Jun 8, 2022
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

Agreed with using 1.18 - Disagree with adding generics directly to gitea1.17.

We need to get 1.17 ready for release - adding generics refactors is going to delay that.

Get RC1 out then we can think about adding in Generics. (But please be aware of the caveats with their current implementation in the golang runtime - They are not free and may come with serious costs where interface{} is used.)

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jun 8, 2022
@wxiaoguang wxiaoguang linked an issue Jun 8, 2022 that may be closed by this pull request
@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 Jun 8, 2022
@delvh
Copy link
Member

delvh commented Jun 8, 2022

@zeripath What about releasing 1.17 as soon as possible and then backporting refactoring PRs once they appear?

@zeripath
Copy link
Contributor

zeripath commented Jun 8, 2022

@zeripath What about releasing 1.17 as soon as possible and then backporting refactoring PRs once they appear?

Generally we should avoid backporting refactoring PRs as they often come with bugs and or change of behaviour. However, I think if there were something that would need backporting that included generics then having 1.17 run go1.18 would allow us to consider backporting those generics to avoid having to completely restructure that backport if necessary. I expect most of the time though, that making a non-generic backport would be easy so we shouldn't need to backport generics.

If it were getting too difficult to backport things we should just consider releasing 1.18 quickly as really our releases are too long and too big and we're making our lives difficult because of this.

@delvh
Copy link
Member

delvh commented Jun 8, 2022

If it were getting too difficult to backport things we should just consider releasing 1.18 quickly as really our releases are too long and too big and we're making our lives difficult because of this.

Strongly agree, but that's a completely different topic that we should tackle soon.

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@lafriks
Copy link
Member

lafriks commented Jun 8, 2022

shouldn't go mod tidy also changed without compat 1.17 flag than?

@wxiaoguang
Copy link
Contributor Author

shouldn't go mod tidy also changed without compat 1.17 flag than?

Do you mean these code?

.PHONY: tidy
tidy:
	$(eval MIN_GO_VERSION := $(shell grep -Eo '^go\s+[0-9]+\.[0-9.]+' go.mod | cut -d' ' -f2))
	$(GO) mod tidy -compat=$(MIN_GO_VERSION)

It's already automated to parse go.mod.

@6543
Copy link
Member

6543 commented Jun 10, 2022

I hope all package maintainer do have enough time to prepare for that ...
... but it's a good move to use new libs who do relay on generics :)

@wxiaoguang
Copy link
Contributor Author

I hope all package maintainer do have enough time to prepare for that ... ... but it's a good move to use new libs who do relay on generics :)

Recently I wrote another CopyMap in #17624, then I remembered the generic again 馃槀

@6543 6543 merged commit 5f61824 into go-gitea:main Jun 10, 2022
@wxiaoguang wxiaoguang deleted the use-go1.18 branch June 10, 2022 03:35
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 11, 2022
* giteaoffical/main:
  Fix data-race problems in git module (quick patch) (go-gitea#19934)
  [skip ci] Updated translations via Crowdin
  Fix copy/paste of empty lines (go-gitea#19798)
  Normalize line endings in fomantic build files (go-gitea#19932)
  Make user profile image show full image on mobile (go-gitea#19840)
  Custom regexp external issues (go-gitea#17624)
  Use Golang 1.18 for Gitea 1.17 release (go-gitea#19918)
  Refactor git module, make Gitea use internal git config (go-gitea#19732)
  [skip ci] Updated translations via Crowdin
@6543 6543 added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Jun 18, 2022
@6543 6543 mentioned this pull request Jun 18, 2022
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
Use Golang 1.18 (as minimal requirement) for Gitea 1.17 release, make sure the Golang version is still actively supported during Gitea 1.17 lifecycle.

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: 6543 <6543@obermui.de>
zeripath pushed a commit that referenced this pull request Oct 15, 2022
We should make sure we're using the same version across the codebase.
* We upgraded in #19918 but forgot about the following line
https://github.com/go-gitea/gitea/blob/6bb6a108e0c03b323402b452fc05c6845f7d00df/build/code-batch-process.go#L273

Signed-off-by: Yarden Shoham <hrsi88@gmail.com>
Co-authored-by: 6543 <6543@obermui.de>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
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. pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! 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.

[Proposal] Use Golang 1.18 for Gitea 1.17 release
8 participants