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

Require MySQL 8.0, PostgreSQL 12, MSSQL 2012 #27337

Merged
merged 11 commits into from
Oct 3, 2023
Merged

Require MySQL 8.0, PostgreSQL 12, MSSQL 2012 #27337

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6c33aa9
Require MySQL >= 8.0, test with 8.1
silverwind Sep 28, 2023
81aacf1
add back services from previous mysql5 step
silverwind Sep 28, 2023
520a5d5
fix indent
silverwind Sep 28, 2023
f026f4a
more reverts
silverwind Sep 28, 2023
3e18771
more reverts
silverwind Sep 28, 2023
4d2c16b
handle postgres and mysql
silverwind Sep 28, 2023
a197dd3
Try with `testgitea` name to match Makefile
silverwind Sep 28, 2023
c507648
Merge branch 'main' into mysql8
silverwind Sep 29, 2023
8a37770
Merge branch 'main' into mysql8
silverwind Sep 29, 2023
77611ee
Update docs/content/installation/database-preparation.en-us.md
techknowlogick Oct 2, 2023
2816901
Merge branch 'main' into mysql8
techknowlogick Oct 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 6 additions & 36 deletions .github/workflows/pull-db-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
runs-on: ubuntu-latest
services:
pgsql:
image: postgres:15
image: postgres:16
env:
POSTGRES_DB: test
POSTGRES_PASSWORD: postgres
Expand Down Expand Up @@ -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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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.

env:
MYSQL_ALLOW_EMPTY_PASSWORD: true
MYSQL_DATABASE: test
Expand Down Expand Up @@ -152,16 +152,16 @@ jobs:
RACE_ENABLED: true
GITHUB_READ_TOKEN: ${{ secrets.GITHUB_READ_TOKEN }}

test-mysql5:
test-mysql:
if: needs.files-changed.outputs.backend == 'true' || needs.files-changed.outputs.actions == 'true'
needs: files-changed
runs-on: ubuntu-latest
services:
mysql:
image: mysql:5.7
image: mysql:8.1
env:
MYSQL_ALLOW_EMPTY_PASSWORD: true
MYSQL_DATABASE: test
MYSQL_DATABASE: testgitea
ports:
- "3306:3306"
elasticsearch:
Expand Down Expand Up @@ -197,43 +197,13 @@ jobs:
USE_REPO_TEST_DIR: 1
TEST_INDEXER_CODE_ES_URL: "http://elastic:changeme@elasticsearch:9200"

test-mysql8:
if: needs.files-changed.outputs.backend == 'true' || needs.files-changed.outputs.actions == 'true'
needs: files-changed
runs-on: ubuntu-latest
services:
mysql8:
image: mysql:8
env:
MYSQL_ALLOW_EMPTY_PASSWORD: true
MYSQL_DATABASE: testgitea
ports:
- "3306:3306"
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v4
with:
go-version: "~1.21"
check-latest: true
- name: Add hosts to /etc/hosts
run: '[ -e "/.dockerenv" ] || [ -e "/run/.containerenv" ] || echo "127.0.0.1 mysql8" | sudo tee -a /etc/hosts'
- run: make deps-backend
- run: make backend
env:
TAGS: bindata
- run: make test-mysql8-migration test-mysql8
timeout-minutes: 50
env:
TAGS: bindata
USE_REPO_TEST_DIR: 1

test-mssql:
if: needs.files-changed.outputs.backend == 'true' || needs.files-changed.outputs.actions == 'true'
needs: files-changed
runs-on: ubuntu-latest
services:
mssql:
image: mcr.microsoft.com/mssql/server:latest
image: mcr.microsoft.com/mssql/server:2022-latest
env:
ACCEPT_EULA: Y
MSSQL_PID: Standard
Expand Down
64 changes: 7 additions & 57 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,6 @@ TEST_MYSQL_HOST ?= mysql:3306
TEST_MYSQL_DBNAME ?= testgitea
silverwind marked this conversation as resolved.
Show resolved Hide resolved
TEST_MYSQL_USERNAME ?= root
TEST_MYSQL_PASSWORD ?=
TEST_MYSQL8_HOST ?= mysql8:3306
TEST_MYSQL8_DBNAME ?= testgitea
TEST_MYSQL8_USERNAME ?= root
TEST_MYSQL8_PASSWORD ?=
TEST_PGSQL_HOST ?= pgsql:5432
TEST_PGSQL_DBNAME ?= testgitea
TEST_PGSQL_USERNAME ?= postgres
Expand Down Expand Up @@ -282,12 +278,12 @@ clean:
rm -rf $(EXECUTABLE) $(DIST) $(BINDATA_DEST) $(BINDATA_HASH) \
integrations*.test \
e2e*.test \
tests/integration/gitea-integration-pgsql/ tests/integration/gitea-integration-mysql/ tests/integration/gitea-integration-mysql8/ tests/integration/gitea-integration-sqlite/ \
tests/integration/gitea-integration-mssql/ tests/integration/indexers-mysql/ tests/integration/indexers-mysql8/ tests/integration/indexers-pgsql tests/integration/indexers-sqlite \
tests/integration/indexers-mssql tests/mysql.ini tests/mysql8.ini tests/pgsql.ini tests/mssql.ini man/ \
tests/e2e/gitea-e2e-pgsql/ tests/e2e/gitea-e2e-mysql/ tests/e2e/gitea-e2e-mysql8/ tests/e2e/gitea-e2e-sqlite/ \
tests/e2e/gitea-e2e-mssql/ tests/e2e/indexers-mysql/ tests/e2e/indexers-mysql8/ tests/e2e/indexers-pgsql/ tests/e2e/indexers-sqlite/ \
tests/e2e/indexers-mssql/ tests/e2e/reports/ tests/e2e/test-artifacts/ tests/e2e/test-snapshots/
tests/integration/gitea-integration-* \
tests/integration/indexers-* \
tests/mysql.ini tests/pgsql.ini tests/mssql.ini man/ \
tests/e2e/gitea-e2e-*/ \
tests/e2e/indexers-*/ \
tests/e2e/reports/ tests/e2e/test-artifacts/ tests/e2e/test-snapshots/

.PHONY: fmt
fmt:
Expand Down Expand Up @@ -551,27 +547,6 @@ test-mysql\#%: integrations.mysql.test generate-ini-mysql
.PHONY: test-mysql-migration
test-mysql-migration: migrations.mysql.test migrations.individual.mysql.test

generate-ini-mysql8:
sed -e 's|{{TEST_MYSQL8_HOST}}|${TEST_MYSQL8_HOST}|g' \
-e 's|{{TEST_MYSQL8_DBNAME}}|${TEST_MYSQL8_DBNAME}|g' \
-e 's|{{TEST_MYSQL8_USERNAME}}|${TEST_MYSQL8_USERNAME}|g' \
-e 's|{{TEST_MYSQL8_PASSWORD}}|${TEST_MYSQL8_PASSWORD}|g' \
-e 's|{{REPO_TEST_DIR}}|${REPO_TEST_DIR}|g' \
-e 's|{{TEST_LOGGER}}|$(or $(TEST_LOGGER),test$(COMMA)file)|g' \
-e 's|{{TEST_TYPE}}|$(or $(TEST_TYPE),integration)|g' \
tests/mysql8.ini.tmpl > tests/mysql8.ini

.PHONY: test-mysql8
test-mysql8: integrations.mysql8.test generate-ini-mysql8
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql8.ini ./integrations.mysql8.test

.PHONY: test-mysql8\#%
test-mysql8\#%: integrations.mysql8.test generate-ini-mysql8
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql8.ini ./integrations.mysql8.test -test.run $(subst .,/,$*)

.PHONY: test-mysql8-migration
test-mysql8-migration: migrations.mysql8.test migrations.individual.mysql8.test

generate-ini-pgsql:
sed -e 's|{{TEST_PGSQL_HOST}}|${TEST_PGSQL_HOST}|g' \
-e 's|{{TEST_PGSQL_DBNAME}}|${TEST_PGSQL_DBNAME}|g' \
Expand Down Expand Up @@ -644,14 +619,6 @@ test-e2e-mysql: playwright e2e.mysql.test generate-ini-mysql
test-e2e-mysql\#%: playwright e2e.mysql.test generate-ini-mysql
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini ./e2e.mysql.test -test.run TestE2e/$*

.PHONY: test-e2e-mysql8
test-e2e-mysql8: playwright e2e.mysql8.test generate-ini-mysql8
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql8.ini ./e2e.mysql8.test

.PHONY: test-e2e-mysql8\#%
test-e2e-mysql8\#%: playwright e2e.mysql8.test generate-ini-mysql8
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql8.ini ./e2e.mysql8.test -test.run TestE2e/$*

.PHONY: test-e2e-pgsql
test-e2e-pgsql: playwright e2e.pgsql.test generate-ini-pgsql
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/pgsql.ini ./e2e.pgsql.test
Expand Down Expand Up @@ -695,9 +662,6 @@ integration-test-coverage-sqlite: integrations.cover.sqlite.test generate-ini-sq
integrations.mysql.test: git-check $(GO_SOURCES)
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/tests/integration -o integrations.mysql.test

integrations.mysql8.test: git-check $(GO_SOURCES)
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/tests/integration -o integrations.mysql8.test

integrations.pgsql.test: git-check $(GO_SOURCES)
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/tests/integration -o integrations.pgsql.test

Expand All @@ -718,11 +682,6 @@ migrations.mysql.test: $(GO_SOURCES) generate-ini-mysql
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/tests/integration/migration-test -o migrations.mysql.test
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini ./migrations.mysql.test

.PHONY: migrations.mysql8.test
migrations.mysql8.test: $(GO_SOURCES) generate-ini-mysql8
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/tests/integration/migration-test -o migrations.mysql8.test
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql8.ini ./migrations.mysql8.test

.PHONY: migrations.pgsql.test
migrations.pgsql.test: $(GO_SOURCES) generate-ini-pgsql
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/tests/integration/migration-test -o migrations.pgsql.test
Expand All @@ -744,13 +703,7 @@ migrations.individual.mysql.test: $(GO_SOURCES)
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg; \
done

.PHONY: migrations.individual.mysql8.test
migrations.individual.mysql8.test: $(GO_SOURCES)
for pkg in $(shell $(GO) list code.gitea.io/gitea/models/migrations/...); do \
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql8.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg; \
done

.PHONY: migrations.individual.mysql8.test\#%
.PHONY: migrations.individual.sqlite.test\#%
migrations.individual.sqlite.test\#%: $(GO_SOURCES) generate-ini-sqlite
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/sqlite.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' code.gitea.io/gitea/models/migrations/$*

Expand Down Expand Up @@ -788,9 +741,6 @@ migrations.individual.sqlite.test\#%: $(GO_SOURCES) generate-ini-sqlite
e2e.mysql.test: $(GO_SOURCES)
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/tests/e2e -o e2e.mysql.test

e2e.mysql8.test: $(GO_SOURCES)
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/tests/e2e -o e2e.mysql8.test

e2e.pgsql.test: $(GO_SOURCES)
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/tests/e2e -o e2e.pgsql.test

Expand Down
2 changes: 1 addition & 1 deletion docs/content/installation/database-preparation.en-us.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ menu:

# Database Preparation

You need a database to use Gitea. Gitea supports PostgreSQL (>=10), MySQL (>=5.7), MariaDB, SQLite, and MSSQL (>=2008R2 SP3). This page will guide into preparing database. Only PostgreSQL and MySQL will be covered here since those database engines are widely-used in production. If you plan to use SQLite, you can ignore this chapter.
You need a database to use Gitea. Gitea supports PostgreSQL (>= 12), MySQL (>= 8.0), MariaDB, SQLite, and MSSQL (>= 2012 SP4). This page will guide into preparing database. Only PostgreSQL and MySQL will be covered here since those database engines are widely-used in production. If you plan to use SQLite, you can ignore this chapter.

techknowlogick marked this conversation as resolved.
Show resolved Hide resolved
Database instance can be on same machine as Gitea (local database setup), or on different machine (remote database).

Expand Down
2 changes: 1 addition & 1 deletion docs/content/installation/database-preparation.zh-cn.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ menu:

# 数据库准备

在使用 Gitea 前，您需要准备一个数据库。Gitea 支持 PostgreSQL（>=10）、MySQL（>=5.7）、SQLite 和 MSSQL（>=2008R2 SP3）这几种数据库。本页将指导您准备数据库。由于 PostgreSQL 和 MySQL 在生产环境中被广泛使用，因此本文档将仅涵盖这两种数据库。如果您计划使用 SQLite，则可以忽略本章内容。
在使用 Gitea 前，您需要准备一个数据库。Gitea 支持 PostgreSQL（>= 12）、MySQL（>= 8.0）、SQLite 和 MSSQL（>= 2012 SP4）这几种数据库。本页将指导您准备数据库。由于 PostgreSQL 和 MySQL 在生产环境中被广泛使用，因此本文档将仅涵盖这两种数据库。如果您计划使用 SQLite，则可以忽略本章内容。

数据库实例可以与 Gitea 实例在相同机器上（本地数据库），也可以与 Gitea 实例在不同机器上（远程数据库）。

Expand Down
5 changes: 2 additions & 3 deletions tests/e2e/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ They can be run with make commands for the appropriate backends, namely:
make test-sqlite
make test-pgsql
make test-mysql
make test-mysql8
make test-mssql
```

Expand Down Expand Up @@ -76,7 +75,7 @@ For SQLite:
make test-e2e-sqlite#example
```

For other databases(replace `mssql` to `mysql`, `mysql8` or `pgsql`):
For other databases(replace `mssql` to `mysql` or `pgsql`):

```
TEST_MSSQL_HOST=localhost:1433 TEST_MSSQL_DBNAME=test TEST_MSSQL_USERNAME=sa TEST_MSSQL_PASSWORD=MwantsaSecurePassword1 make test-e2e-mssql#example
Expand All @@ -90,4 +89,4 @@ Although the main goal of e2e is assertion testing, we have added a framework fo

VISUAL_TEST=1 will create screenshots in tests/e2e/test-snapshots. The test will fail the first time this is enabled (until we get visual test image persistence figured out), because it will be testing against an empty screenshot folder.

ACCEPT_VISUAL=1 will overwrite the snapshot images with new images.
ACCEPT_VISUAL=1 will overwrite the snapshot images with new images.
3 changes: 1 addition & 2 deletions tests/integration/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ appropriate backends, namely:
make test-sqlite
make test-pgsql
make test-mysql
make test-mysql8
make test-mssql
```

Expand Down Expand Up @@ -84,7 +83,7 @@ For SQLite:
make test-sqlite#GPG
```

For other databases(replace `mssql` to `mysql`, `mysql8` or `pgsql`):
For other databases(replace `mssql` to `mysql`, or `pgsql`):

```
TEST_MSSQL_HOST=localhost:1433 TEST_MSSQL_DBNAME=test TEST_MSSQL_USERNAME=sa TEST_MSSQL_PASSWORD=MwantsaSecurePassword1 make test-mssql#GPG
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/README_ZH.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ sqlite 数据库:
make test-sqlite#GPG
```

其它数据库(把 MSSQL 替换为 MYSQL, MYSQL8, PGSQL):
其它数据库(把 MSSQL 替换为 MYSQL, PGSQL):

```
TEST_MSSQL_HOST=localhost:1433 TEST_MSSQL_DBNAME=test TEST_MSSQL_USERNAME=sa TEST_MSSQL_PASSWORD=MwantsaSecurePassword1 make test-mssql#GPG
Expand Down
109 changes: 0 additions & 109 deletions tests/mysql8.ini.tmpl
View file
Open in desktop

This file was deleted.