Skip to content

Use a migration test instead of a wrong test which populated the meta test repositories and fix a migration bug #36160

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

Open
lunny wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Use a migration test instead of a wrong test which populated the meta test repositories and fix a migration bug #36160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb1952c
Use a migration test instead of a wrong test which populated the meta…
lunny Dec 14, 2025
c69e54d
Fix a bug when migrating pull requests' head branch
lunny Dec 14, 2025
e598838
Merge branch 'main' into lunny/remove_wrong_test
lunny Dec 15, 2025
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
1 change: 1 addition & 0 deletions modules/migration/uploader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type Uploader interface {
CreateMilestones(ctx context.Context, milestones ...*Milestone) error
CreateReleases(ctx context.Context, releases ...*Release) error
SyncTags(ctx context.Context) error
SyncBranches(ctx context.Context) error
CreateLabels(ctx context.Context, labels ...*Label) error
CreateIssues(ctx context.Context, issues ...*Issue) error
CreateComments(ctx context.Context, comments ...*Comment) error
Expand Down
5 changes: 5 additions & 0 deletions services/migrations/dump.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,11 @@ func (g *RepositoryDumper) SyncTags(ctx context.Context) error {
return nil
}

// SyncBranches syncs branches in the database
func (g *RepositoryDumper) SyncBranches(ctx context.Context) error {
return nil
}

// CreateIssues creates issues
func (g *RepositoryDumper) CreateIssues(_ context.Context, issues ...*base.Issue) error {
var err error
Expand Down
5 changes: 5 additions & 0 deletions services/migrations/gitea_uploader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,11 @@ func (g *GiteaLocalUploader) SyncTags(ctx context.Context) error {
return repo_module.SyncReleasesWithTags(ctx, g.repo, g.gitRepo)
}

func (g *GiteaLocalUploader) SyncBranches(ctx context.Context) error {
_, err := repo_module.SyncRepoBranchesWithRepo(ctx, g.repo, g.gitRepo, g.doer.ID)
return err
}

// CreateIssues creates issues
func (g *GiteaLocalUploader) CreateIssues(ctx context.Context, issues ...*base.Issue) error {
iss := make([]*issues_model.Issue, 0, len(issues))
Expand Down
302 changes: 0 additions & 302 deletions services/migrations/gitea_uploader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
package migrations

import (
"fmt"
"os"
"path/filepath"
"strconv"
"testing"
"time"
Expand All @@ -17,15 +14,10 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/git/gitcmd"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/graceful"
"code.gitea.io/gitea/modules/log"
base "code.gitea.io/gitea/modules/migration"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test"
repo_service "code.gitea.io/gitea/services/repository"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -228,297 +220,3 @@ func TestGiteaUploadRemapExternalUser(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, linkedUser.ID, target.GetUserID())
}

func TestGiteaUploadUpdateGitForPullRequest(t *testing.T) {
unittest.PrepareTestEnv(t)

//
// fromRepo master
//
fromRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
baseRef := "master"
// this is very different from the real situation. It should be a bare repository for all the Gitea managed repositories
assert.NoError(t, git.InitRepository(t.Context(), fromRepo.RepoPath(), false, fromRepo.ObjectFormatName))
err := gitrepo.RunCmd(t.Context(), fromRepo, gitcmd.NewCommand("symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+baseRef))
assert.NoError(t, err)
assert.NoError(t, os.WriteFile(filepath.Join(fromRepo.RepoPath(), "README.md"), []byte("# Testing Repository\n\nOriginally created in: "+fromRepo.RepoPath()), 0o644))
assert.NoError(t, git.AddChanges(t.Context(), fromRepo.RepoPath(), true))
signature := git.Signature{
Email: "[email protected]",
Name: "test",
When: time.Now(),
}
assert.NoError(t, git.CommitChanges(t.Context(), fromRepo.RepoPath(), git.CommitChangesOptions{
Committer: &signature,
Author: &signature,
Message: "Initial Commit",
}))
fromGitRepo, err := gitrepo.OpenRepository(t.Context(), fromRepo)
assert.NoError(t, err)
defer fromGitRepo.Close()
baseSHA, err := fromGitRepo.GetBranchCommitID(baseRef)
assert.NoError(t, err)

//
// fromRepo branch1
//
headRef := "branch1"
_, err = gitrepo.RunCmdString(t.Context(), fromRepo, gitcmd.NewCommand("checkout", "-b").AddDynamicArguments(headRef))
assert.NoError(t, err)
assert.NoError(t, os.WriteFile(filepath.Join(fromRepo.RepoPath(), "README.md"), []byte("SOMETHING"), 0o644))
assert.NoError(t, git.AddChanges(t.Context(), fromRepo.RepoPath(), true))
signature.When = time.Now()
assert.NoError(t, git.CommitChanges(t.Context(), fromRepo.RepoPath(), git.CommitChangesOptions{
Committer: &signature,
Author: &signature,
Message: "Pull request",
}))
assert.NoError(t, err)
headSHA, err := fromGitRepo.GetBranchCommitID(headRef)
assert.NoError(t, err)

fromRepoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: fromRepo.OwnerID})

//
// forkRepo branch2
//
forkHeadRef := "branch2"
forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 8})
assert.NoError(t, git.Clone(t.Context(), fromRepo.RepoPath(), forkRepo.RepoPath(), git.CloneRepoOptions{
Branch: headRef,
}))
_, err = gitrepo.RunCmdString(t.Context(), forkRepo, gitcmd.NewCommand("checkout", "-b").AddDynamicArguments(forkHeadRef))
assert.NoError(t, err)
assert.NoError(t, os.WriteFile(filepath.Join(forkRepo.RepoPath(), "README.md"), []byte("# branch2 "+forkRepo.RepoPath()), 0o644))
assert.NoError(t, git.AddChanges(t.Context(), forkRepo.RepoPath(), true))
assert.NoError(t, git.CommitChanges(t.Context(), forkRepo.RepoPath(), git.CommitChangesOptions{
Committer: &signature,
Author: &signature,
Message: "branch2 commit",
}))
forkGitRepo, err := gitrepo.OpenRepository(t.Context(), forkRepo)
assert.NoError(t, err)
defer forkGitRepo.Close()
forkHeadSHA, err := forkGitRepo.GetBranchCommitID(forkHeadRef)
assert.NoError(t, err)

toRepoName := "migrated"
ctx := t.Context()
uploader := NewGiteaLocalUploader(ctx, fromRepoOwner, fromRepoOwner.Name, toRepoName)
uploader.gitServiceType = structs.GiteaService

assert.NoError(t, repo_service.Init(t.Context()))
assert.NoError(t, uploader.CreateRepo(ctx, &base.Repository{
Description: "description",
OriginalURL: fromRepo.RepoPath(),
CloneURL: fromRepo.RepoPath(),
IsPrivate: false,
IsMirror: true,
}, base.MigrateOptions{
GitServiceType: structs.GiteaService,
Private: false,
Mirror: true,
}))

for _, testCase := range []struct {
name string
head string
logFilter []string
logFiltered []bool
pr base.PullRequest
}{
{
name: "fork, good Head.SHA",
head: fmt.Sprintf("%s/%s", forkRepo.OwnerName, forkHeadRef),
pr: base.PullRequest{
PatchURL: "",
Number: 1,
State: "open",
Base: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: baseRef,
SHA: baseSHA,
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
Head: base.PullRequestBranch{
CloneURL: forkRepo.RepoPath(),
Ref: forkHeadRef,
SHA: forkHeadSHA,
RepoName: forkRepo.Name,
OwnerName: forkRepo.OwnerName,
},
},
},
{
name: "fork, invalid Head.Ref",
head: "unknown repository",
pr: base.PullRequest{
PatchURL: "",
Number: 1,
State: "open",
Base: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: baseRef,
SHA: baseSHA,
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
Head: base.PullRequestBranch{
CloneURL: forkRepo.RepoPath(),
Ref: "INVALID",
SHA: forkHeadSHA,
RepoName: forkRepo.Name,
OwnerName: forkRepo.OwnerName,
},
},
logFilter: []string{"Fetch branch from"},
logFiltered: []bool{true},
},
{
name: "invalid fork CloneURL",
head: "unknown repository",
pr: base.PullRequest{
PatchURL: "",
Number: 1,
State: "open",
Base: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: baseRef,
SHA: baseSHA,
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
Head: base.PullRequestBranch{
CloneURL: "UNLIKELY",
Ref: forkHeadRef,
SHA: forkHeadSHA,
RepoName: forkRepo.Name,
OwnerName: "WRONG",
},
},
logFilter: []string{"AddRemote"},
logFiltered: []bool{true},
},
{
name: "no fork, good Head.SHA",
head: headRef,
pr: base.PullRequest{
PatchURL: "",
Number: 1,
State: "open",
Base: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: baseRef,
SHA: baseSHA,
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
Head: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: headRef,
SHA: headSHA,
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
},
},
{
name: "no fork, empty Head.SHA",
head: headRef,
pr: base.PullRequest{
PatchURL: "",
Number: 1,
State: "open",
Base: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: baseRef,
SHA: baseSHA,
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
Head: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: headRef,
SHA: "",
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
},
logFilter: []string{"Empty reference", "Cannot remove local head"},
logFiltered: []bool{true, false},
},
{
name: "no fork, invalid Head.SHA",
head: headRef,
pr: base.PullRequest{
PatchURL: "",
Number: 1,
State: "open",
Base: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: baseRef,
SHA: baseSHA,
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
Head: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: headRef,
SHA: "brokenSHA",
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
},
logFilter: []string{"Deprecated local head"},
logFiltered: []bool{true},
},
{
name: "no fork, not found Head.SHA",
head: headRef,
pr: base.PullRequest{
PatchURL: "",
Number: 1,
State: "open",
Base: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: baseRef,
SHA: baseSHA,
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
Head: base.PullRequestBranch{
CloneURL: fromRepo.RepoPath(),
Ref: headRef,
SHA: "2697b352310fcd01cbd1f3dbd43b894080027f68",
RepoName: fromRepo.Name,
OwnerName: fromRepo.OwnerName,
},
},
logFilter: []string{"Deprecated local head", "Cannot remove local head"},
logFiltered: []bool{true, false},
},
} {
t.Run(testCase.name, func(t *testing.T) {
stopMark := fmt.Sprintf(">>>>>>>>>>>>>STOP: %s<<<<<<<<<<<<<<<", testCase.name)

logChecker, cleanup := test.NewLogChecker(log.DEFAULT)
logChecker.Filter(testCase.logFilter...).StopMark(stopMark)
defer cleanup()

testCase.pr.EnsuredSafe = true

head, err := uploader.updateGitForPullRequest(ctx, &testCase.pr)
assert.NoError(t, err)
assert.Equal(t, testCase.head, head)

log.Info(stopMark)

logFiltered, logStopped := logChecker.Check(5 * time.Second)
assert.True(t, logStopped)
if len(testCase.logFilter) > 0 {
assert.Equal(t, testCase.logFiltered, logFiltered, "for log message filters: %v", testCase.logFilter)
}
})
}
}
9 changes: 9 additions & 0 deletions services/migrations/migrate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,15 @@ func migrateRepository(ctx context.Context, doer *user_model.User, downloader ba
break
}
}
if len(mapInsertedPRIndexes) > 0 {
// The pull requests migrating process may created head branches in the base repository
// because head repository maybe a fork one which will not be migrated. So that we need
// to sync branches again.
log.Trace("syncing branches after migrating pull requests")
if err = uploader.SyncBranches(ctx); err != nil {
return err
}
}
}

if opts.Comments && supportAllComments {
Expand Down
Loading