Fix wrong xorm get usage on migration #27111

lunny · 2023-09-18T03:47:48Z

Fix the bug on try.gitea.io

2023/09/18 01:48:41 ...ations/migrations.go:635:Migrate() [I] Migration[276]: Add RemoteAddress to mirrors
2023/09/18 01:48:41 routers/common/db.go:34:InitDBEngine() [E] ORM engine initialization attempt #7/10 failed. Error: migrate: migration[276]: Add RemoteAddress to mirrors failed: exit status 128 - fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
 - fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).

Caused by #26952

models/migrations/v1_21/v276.go

delvh

So, the main change is that we use an INNER join instead of an implicit LEFT join?
And that fixes problems?

delvh · 2023-09-18T07:05:41Z

models/migrations/v1_21/v276.go

@@ -118,7 +123,9 @@ func migratePushMirrors(x *xorm.Engine) error {

 	for {
 		var mirrors []PushMirror
-		if err := sess.Limit(limit, start).Find(&mirrors); err != nil {
+		if err := sess.Select("push_mirror.id, push_mirror.repo_id, push_mirror.remote_name, push_mirror.remote_address, repository.owner_name as repo_owner, repository.name as repo_name").
+			Join("INNER", "repository", "repository.id = push_mirror.repo_id").


If we are already at it, can we also use

Suggested change

Join("INNER", "repository", "repository.id = push_mirror.repo_id").

Join("INNER", "repository", "repository.id = push_mirror.repo_id").

Where("push_mirror.remote_address IS NULL")

?
That way, the migration at least becomes symmetric.

Since remote_address is not an index column and the second update will not affect anything, I don't think it's necessary for this change.

delvh · 2023-09-18T07:06:26Z

models/migrations/v1_21/v276.go

@@ -59,7 +60,9 @@ func migratePullMirrors(x *xorm.Engine) error {

 	for {
 		var mirrors []Mirror
-		if err := sess.Limit(limit, start).Find(&mirrors); err != nil {
+		if err := sess.Select("mirror.id, mirror.repo_id, mirror.remote_address, repository.owner_name as repo_owner, repository.name as repo_name").
+			Join("INNER", "repository", "repository.id = mirror.repo_id").


Suggested change

Join("INNER", "repository", "repository.id = mirror.repo_id").

Join("INNER", "repository", "repository.id = mirror.repo_id").

Where("mirror.remote_address IS NULL")

Co-authored-by: Jason Song <i@wolfogre.com>

KN4CK3R · 2023-09-18T09:00:23Z

Isn't the problem that there are mirrors in the database which do not exist in the file system? I don't think we should work around that in the migration.

puni9869

Not blocking this PR. I agree with @delvh 's feedback. I had encountered same problem with Archived Label migration.

KN4CK3R · 2023-09-18T12:25:45Z

I don't see how the change should be related to the error message.

lunny · 2023-09-18T15:04:53Z

I don't see how the change should be related to the error message.

This PR will ignore the empty repository and repositories which status is not ready. So reading git repository should not be failed.

KN4CK3R · 2023-09-18T17:48:03Z

If there is a mirror the repo path must exists:
pull mirror

gitea/services/repository/create.go

Line 241 in 323135b

if err := db.WithTx(ctx, func(ctx context.Context) error {

push mirror

gitea/services/mirror/mirror_push.go

Line 29 in 323135b

    
           func AddPushMirrorRemote(ctx context.Context, m *repo_model.PushMirror, addr string) error {

and following lines

KN4CK3R · 2023-09-18T20:55:47Z

That PR should not have merged at that point...

See also https://discord.com/channels/322538954119184384/322910365237248000/1153441247940321391 and the following messages.

lunny · 2023-09-19T08:28:58Z

That PR should not have merged at that point...

See also https://discord.com/channels/322538954119184384/322910365237248000/1153441247940321391 and the following messages.

Please review #27132

* giteaofficial/main: Improve actions docs related to `pull_request` event (go-gitea#27126) Remove outdated paragraphs when comparing Gitea Actions to GitHub Actions (go-gitea#27119) Fix: treat tab "overview" as "repositories" in user profiles without readme (go-gitea#27124) Fix incorrect test code for error handling (go-gitea#27139) Increase auth provider icon size on login page (go-gitea#27122) fix pagination for followers and following (go-gitea#27127) services/wiki: Close() after error handling (go-gitea#27129) Use fetch helpers instead of fetch (go-gitea#27026) Change green buttons to primary color (go-gitea#27099) Fix wrong xorm get usage on migration (go-gitea#27111) Fix the incorrect route path in the user edit page. (go-gitea#27007) Refactor lfs requests (go-gitea#26783) Display archived labels specially when listing labels (go-gitea#26820) Remove a `gt-float-right` and some unnecessary helpers (go-gitea#27110) [skip ci] Updated licenses and gitignores Fix token endpoints ignore specified account (go-gitea#27080) Make SSPI auth mockable (go-gitea#27036)

* origin/main: Fix dropdown icon position (go-gitea#27175) Fix repo sub menu (go-gitea#27169) Fix review request number and add more tests (go-gitea#27104) Fix the variable regexp pattern on web page (go-gitea#27161) Fix organization field being null in POST /orgs/{orgid}/teams (go-gitea#27150) Add index to `issue_user.issue_id` (go-gitea#27154) [skip ci] Updated translations via Crowdin Start development on Gitea 1.22 (go-gitea#27155) Fix successful return value for `SyncAndGetUserSpecificDiff` (go-gitea#27152) Improve actions docs related to `pull_request` event (go-gitea#27126) Remove outdated paragraphs when comparing Gitea Actions to GitHub Actions (go-gitea#27119) Fix: treat tab "overview" as "repositories" in user profiles without readme (go-gitea#27124) Fix incorrect test code for error handling (go-gitea#27139) Increase auth provider icon size on login page (go-gitea#27122) fix pagination for followers and following (go-gitea#27127) services/wiki: Close() after error handling (go-gitea#27129) Use fetch helpers instead of fetch (go-gitea#27026) Change green buttons to primary color (go-gitea#27099) Fix wrong xorm get usage on migration (go-gitea#27111)

Fix wrong xomr get usage on migration

6239288

lunny added type/bug skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Sep 18, 2023

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 18, 2023

pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 18, 2023

lunny changed the title ~~Fix wrong xomr get usage on migration~~ Fix wrong xorm get usage on migration Sep 18, 2023

wolfogre reviewed Sep 18, 2023

View reviewed changes

models/migrations/v1_21/v276.go Outdated Show resolved Hide resolved

wolfogre approved these changes Sep 18, 2023

View reviewed changes

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 18, 2023

delvh reviewed Sep 18, 2023

View reviewed changes

lunny and others added 3 commits September 18, 2023 16:15

Update models/migrations/v1_21/v276.go

adbd89a

Co-authored-by: Jason Song <i@wolfogre.com>

Fix error

5ee5025

Add error track

2ac38f6

puni9869 approved these changes Sep 18, 2023

View reviewed changes

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 18, 2023

Merge branch 'main' into lunny/fix_migration_get

5c63864

lafriks merged commit e644cc9 into go-gitea:main Sep 18, 2023

GiteaBot added this to the 1.22.0 milestone Sep 18, 2023

lunny deleted the lunny/fix_migration_get branch September 19, 2023 08:24

delvh modified the milestones: 1.22.0, 1.21.0 Sep 20, 2023

go-gitea locked as resolved and limited conversation to collaborators Dec 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix wrong xorm get usage on migration #27111

Fix wrong xorm get usage on migration #27111

lunny commented Sep 18, 2023 •

edited

Loading

delvh left a comment

delvh Sep 18, 2023

lunny Sep 18, 2023

delvh Sep 18, 2023

KN4CK3R commented Sep 18, 2023

puni9869 left a comment

KN4CK3R commented Sep 18, 2023

lunny commented Sep 18, 2023

KN4CK3R commented Sep 18, 2023 •

edited

Loading

KN4CK3R commented Sep 18, 2023 •

edited

Loading

lunny commented Sep 19, 2023

	Join("INNER", "repository", "repository.id = push_mirror.repo_id").
	Join("INNER", "repository", "repository.id = push_mirror.repo_id").
	Where("push_mirror.remote_address IS NULL")

	Join("INNER", "repository", "repository.id = mirror.repo_id").
	Join("INNER", "repository", "repository.id = mirror.repo_id").
	Where("mirror.remote_address IS NULL")

Fix wrong xorm get usage on migration #27111

Fix wrong xorm get usage on migration #27111

Conversation

lunny commented Sep 18, 2023 • edited Loading

delvh left a comment

Choose a reason for hiding this comment

delvh Sep 18, 2023

Choose a reason for hiding this comment

lunny Sep 18, 2023

Choose a reason for hiding this comment

delvh Sep 18, 2023

Choose a reason for hiding this comment

KN4CK3R commented Sep 18, 2023

puni9869 left a comment

Choose a reason for hiding this comment

KN4CK3R commented Sep 18, 2023

lunny commented Sep 18, 2023

KN4CK3R commented Sep 18, 2023 • edited Loading

KN4CK3R commented Sep 18, 2023 • edited Loading

lunny commented Sep 19, 2023

lunny commented Sep 18, 2023 •

edited

Loading

KN4CK3R commented Sep 18, 2023 •

edited

Loading

KN4CK3R commented Sep 18, 2023 •

edited

Loading