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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require MySQL 8.0, PostgreSQL 12, MSSQL 2012 #27337

Merged
merged 11 commits into from
Oct 3, 2023

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Sep 28, 2023

  • MySQL 5.7 support and testing is dropped
  • MySQL tests now execute against 8.1, up from 5.7 and 8.0
  • PostgreSQL 10 and 11 support ist dropped
  • PostgreSQL tests now execute against 16, up from 15
  • MSSQL 2008 support is dropped
  • MSSQL tests now run against locked 2022 version

Fixes: #25657

Ref: https://endoflife.date/mysql
Ref: https://endoflife.date/postgresql
Ref: https://endoflife.date/mssqlserver

⚠️ BREAKING ⚠️

Support for MySQL 5.7, PostgreSQL 10 and 11, and MSSQL 2008 is dropped. You are encouraged to upgrade to supported versions.

- MySQL 5.7 support and testing is dropped
- MySQL tests now only test against 8.1, previously it was 5.7 and 8.0

Fixes: go-gitea#25657
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 28, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 28, 2023
@github-actions github-actions bot added type/docs This PR mainly updates/creates documentation topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile labels Sep 28, 2023
@silverwind silverwind added pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! and removed type/docs This PR mainly updates/creates documentation topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile labels Sep 28, 2023
@silverwind silverwind changed the title Require MySQL >= 8.0, test with 8.1 Require MySQL 8.0 or greater, test with 8.1 Sep 28, 2023
@silverwind
Copy link
Member Author

silverwind commented Sep 28, 2023

Before merging this, we need to adjust the required checks config on github ui to update from test-mysql5 and test-mysql8 to just test-mysql.

@github-actions github-actions bot added type/docs This PR mainly updates/creates documentation topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile labels Sep 28, 2023
@silverwind silverwind changed the title Require MySQL 8.0 or greater, test with 8.1 Require MySQL 8.0, Postgres 12, MSSQL 2012 Sep 28, 2023
@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 Sep 28, 2023
@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 Sep 28, 2023
@delvh
Copy link
Member

delvh commented Sep 28, 2023

I mean, we could also be so nice and block this PR until the mentioned versions are actually EOL.
But I'm also fine with the current approach.

@denyskon
Copy link
Member

I don't think that people who use MySQL 5.7 also use Gitea nightly in production 😆
I think it's fine, the release will definitely happen after all of the mentioned versions are EOL.

@silverwind
Copy link
Member Author

By the time the 1.22 branch releases they will be EOL. I imagine anyone running unstable builds will likely not run unsupported databases :)

@denyskon
Copy link
Member

@silverwind ✨ same though at the same moment ✨

@delvh
Copy link
Member

delvh commented Sep 28, 2023

I don't think that people who use MySQL 5.7 also use Gitea nightly in production 😆

Good point.

@silverwind silverwind changed the title Require MySQL 8.0, Postgres 12, MSSQL 2012 Require MySQL 8.0, PostgreSQL 12, MSSQL 2012 Sep 28, 2023
Makefile Show resolved Hide resolved
@techknowlogick
Copy link
Member

Thanks for keeping this current 😀

Could you add a note that should someone need support for an outdated database that they can contact us for an "extended support contract" (or some other name?). As then we can then test for those older databases and add fixes into this codebase too. I know there are some people who are using the outdated mssql version that are paying MS for them to backport fixes, so they may need to have that support in Gitea too.

@silverwind
Copy link
Member Author

Feel free to edit the OP to your liking :)

@@ -86,7 +86,7 @@ jobs:
runs-on: ubuntu-latest
services:
mysql:
image: mysql:5.7
image: mysql:8.1
Copy link
Contributor

Choose a reason for hiding this comment

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

It should test with MySQL 8.0 but not 8.1

The same to pgsql/mssql

Copy link
Member Author

@silverwind silverwind Sep 29, 2023

Choose a reason for hiding this comment

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

Why not test with latest? New deployments are likely to use latest. I see no point testing non-latest if we only test one version.

Copy link
Member Author

@silverwind silverwind Sep 29, 2023

Choose a reason for hiding this comment

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

This one step I guess we could move to 8.0 because 8.1 is tested below separately, but I don't see much of a point either.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if you used a MySQL 8.1 only syntax?

Copy link
Member Author

@silverwind silverwind Sep 29, 2023

Choose a reason for hiding this comment

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

By your notion, we would have to test every single supported db version, which given our CI resources is likely not feasible. Also, MariaDB is completely untested.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, MariaDB should have a separate test.

For others, databases have pretty good backward compatibility. If MySQL 8.0 passes, then MySQL 8.1 are almost sure to pass.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should stick to MySQL LTS versions? According to this 8.0, 8.4, 9.7 will be LTS:

No, at the moment, only the minimal supported version is enough.

Copy link
Member Author

@silverwind silverwind Sep 29, 2023

Choose a reason for hiding this comment

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

I still don't understand the reasons. We can:

  • Test latest, be most likely correct for new deployment, but old ones may break with incompatibly SQL syntax etc.
  • Test oldest, be correct for ancient deployments, but unknown for new ones.
  • Test something in between, but by what rules?

I think it's easier to maintain if we just test latest, so that the maintainer updating this doesn't have to thing about obscure rules about which version to test.

Ideally we'd have a matrix build to easily add/remove versions. Currently it's a pain because of all the Makefile changes and such. If we had a matrix, I guess testing oldest and newest of each version would be ideal.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • Test latest, be most likely correct for new deployment, but old ones may break with incompatibly SQL syntax etc.

No, it would break the old database easily.

  • Test oldest, be correct for ancient deployments, but unknown for new ones.

This is the right approach. It won't be "unknown": see my comment: databases have pretty good backward compatibility. If MySQL 8.0 passes, then MySQL 8.1 are almost sure to pass.

  • Test something in between, but by what rules?

No, it's not worth to be considered.

Copy link
Member

Choose a reason for hiding this comment

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

This PR doesn't change the behaviour of what we are doing with tests (w/ psql we're using latest, and w/ mqsl we were using 8.x).

I agree we should test both min/max db versions, but that can be done in a different PR.

@lunny
Copy link
Member

lunny commented Sep 30, 2023

Yes, if we say it required mysql 8.0, we should test the minial version 8.0 but not 8.1 like we did with Golang.

@techknowlogick techknowlogick merged commit e94f9fc into go-gitea:main Oct 3, 2023
25 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Oct 3, 2023
@silverwind silverwind deleted the mysql8 branch October 3, 2023 20:51
@wxiaoguang
Copy link
Contributor

@techknowlogick

This PR doesn't change the behaviour of what we are doing with tests (w/ psql we're using latest, and w/ mqsl we were using 8.x).
I agree we should test both min/max db versions, but that can be done in a different PR.

Testing the minimal version is good enough and the right approach, like before.

But I guess nobody would really follow it if this gets merged, until some bugs like #22809 occurs (pgsql 10 works but pgsql 9 doesn't -- newer version works, older version fails)

This is my last comment for this problem, because I have explained enough and I do not want to bother people.

@lunny
Copy link
Member

lunny commented Nov 1, 2023

ref: #27429

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 2, 2024
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! size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile type/docs This PR mainly updates/creates documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop MySQL 5.7 support
7 participants