Skip to content

Conversation

@kolaente
Copy link
Member

The migration introduced in #8742 breaks mysql installations. This pr fixes that by correctly using CONCAT.

Signed-off-by: kolaente <[email protected]>
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label May 15, 2020
@kolaente
Copy link
Member Author

My gitea instance works after running this in the mysql console, but I'm not sure if the changes it made to the db are the ones they're supposed to do. How can I verify that? cc @SijmenSchoon

@kolaente
Copy link
Member Author

I now have entries like ref: refs/heads/master in my db, seems alight.

@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 May 15, 2020
@lafriks lafriks added type/bug skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels May 15, 2020
@lafriks lafriks added this to the 1.12.0 milestone May 15, 2020
@kolaente
Copy link
Member Author

How do the tests fail on this one

@kolaente
Copy link
Member Author

Also the thing that fails is a utf 8 conversion test that has absolutly nothing to do with database... how...

@zeripath
Copy link
Contributor

make lg-tm work

@zeripath zeripath merged commit 57217ca into go-gitea:master May 15, 2020
@zeripath
Copy link
Contributor

SQLite uses || for concatenation as does Postgres

@zeripath
Copy link
Contributor

Postgres can use CONCAT too

@kolaente kolaente deleted the fix/issue-ref-migration branch May 15, 2020 14:14
@jolheiser
Copy link
Member

Support multiple databases they said. It'll be fun they said.

@mrsdizzie
Copy link
Member

(In the future) it would probably be worth it to just load the records, fix the strings in go, then update them to avoid exceptions for different database types and potential breakage. Examples:

https://github.com/go-gitea/gitea/blob/master/models/migrations/v114.go
https://github.com/go-gitea/gitea/blob/master/models/migrations/v100.go

@vijfhoek
Copy link
Contributor

vijfhoek commented May 15, 2020 via email

@lafriks
Copy link
Member

lafriks commented May 15, 2020

@mrsdizzie that would make it so much slower for large databases

ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
The migration introduced in go-gitea#8742 breaks mysql installations. This pr fixes that by correctly using CONCAT.

Signed-off-by: kolaente <[email protected]>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
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. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants