diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index d6dbce009cf..d4dd97cb0b4 100644 --- a/routers/private/hook_post_receive.go +++ b/routers/private/hook_post_receive.go @@ -71,7 +71,7 @@ func hookPostReceiveSyncDatabaseBranches(ctx *gitea_context.PrivateContext, opts return } - if err = repo_service.SyncBranchesToDB(ctx, repo.ID, opts.UserID, gitRepo, branchNames); err != nil { + if err = repo_service.SyncBranchesToDB(ctx, repo, opts.UserID, gitRepo, branchNames); err != nil { log.Error("failed to sync branch to DB: %v", err) } } @@ -87,16 +87,20 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { // In the post-receive hook, the git repo has received all changes, // So even if any error happens, nothing can be stopped or undone, so the errors should be skipped repo := ctx.Repo.Repository - updates := hookPostReceiveCollectPushUpdates(opts, repo) // first, collect updates and sync branches + wasEmpty := repo.IsEmpty + updates := hookPostReceiveCollectPushUpdates(opts, repo) + hookPostReceiveUpdateRepoByOptions(ctx, opts, repo) // push options are only honored while the repo is still empty hookPostReceiveSyncDatabaseBranches(ctx, opts, repo, updates) - hookPostReceiveSyncRepoDefaultBranch(ctx, opts, repo) - hookPostReceiveUpdateRepoByOptions(ctx, opts, repo) // handle async updates (e.g.: notification, cache management, repo update time, etc.) if err := repo_service.PushUpdates(updates...); err != nil { log.Error("failed to push updates: %v", err) } + if wasEmpty && !repo.IsFork { + ctx.JSON(http.StatusOK, private.HookPostReceiveResult{}) + return + } hookPostReceiveRespondWithTrailer(ctx, opts, repo) } @@ -201,43 +205,3 @@ func hookPostReceiveRespondWithTrailer(ctx *gitea_context.PrivateContext, opts * } ctx.JSON(http.StatusOK, private.HookPostReceiveResult{Results: results}) } - -func hookPostReceiveSyncRepoDefaultBranch(ctx *gitea_context.PrivateContext, opts *private.HookOptions, repo *repo_model.Repository) { - hasBranch := false - for _, refFullName := range opts.RefFullNames { - if hasBranch = refFullName.IsBranch(); hasBranch { - break - } - } - if !hasBranch { - return - } - gitRepo, err := git.RepositoryFromRequestContextOrOpen(ctx, repo) - if err != nil { - log.Error("failed to open git repo: %v", err) - return - } - - // if default branch doesn't exist, try to guess one from existing git repo - _, err = gitRepo.GetBranchCommitID(ctx, repo.DefaultBranch) - if errors.Is(err, util.ErrNotExist) { - for _, guessBranchName := range []string{"main", "master"} { - if _, err = gitRepo.GetBranchCommitID(ctx, guessBranchName); err == nil { - repo.DefaultBranch = guessBranchName - err = repo_model.UpdateDefaultBranch(ctx, repo) - if err != nil { - log.Error("failed to update default branch: %v", err) - return - } - break - } - } - } - - // if default branch was pushed, always keep the HEAD ref in sync - for _, refFullName := range opts.RefFullNames { - if refFullName.IsBranch() && refFullName.BranchName() == repo.DefaultBranch { - _ = git.SetDefaultBranch(ctx, repo, repo.DefaultBranch) - } - } -} diff --git a/services/repository/branch.go b/services/repository/branch.go index 320acdf8f12..2f575b45916 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "slices" "strings" "gitea.dev/models/db" @@ -18,6 +19,7 @@ import ( user_model "gitea.dev/models/user" "gitea.dev/modules/cache" "gitea.dev/modules/git" + "gitea.dev/modules/git/gitcmd" "gitea.dev/modules/globallock" "gitea.dev/modules/graceful" "gitea.dev/modules/json" @@ -305,7 +307,8 @@ func checkBranchName(ctx context.Context, gitRepo *git.Repository, name string) // It will check whether the branches of the repository have never been synced before. // If so, it will sync all branches of the repository. // Otherwise, it will sync the branches that need to be updated. -func SyncBranchesToDB(ctx context.Context, repoID, pusherID int64, gitRepo *git.Repository, branchNames []string) error { +// It also initializes an empty repository and repairs a default branch missing in git. +func SyncBranchesToDB(ctx context.Context, repo *repo_model.Repository, pusherID int64, gitRepo *git.Repository, branchNames []string) error { // Some designs that make the code look strange but are made for performance optimization purposes: // 1. Sync branches in a batch to reduce the number of DB queries. // 2. Lazy load commit information since it may be not necessary. @@ -319,14 +322,18 @@ func SyncBranchesToDB(ctx context.Context, repoID, pusherID int64, gitRepo *git. // In case there are concurrent pushes, we need to lock the sync operation for the same repository // and always use the latest branch information in the database. - releaser, err := globallock.Lock(ctx, fmt.Sprintf("repo_branch_sync_%d", repoID)) + releaser, err := globallock.Lock(ctx, fmt.Sprintf("repo_branch_sync_%d", repo.ID)) if err != nil { return err } defer releaser() - return db.WithTx(ctx, func(ctx context.Context) error { - dbBranches, err := git_model.GetBranches(ctx, repoID, branchNames, true) + wasEmpty, oldDefaultBranch := repo.IsEmpty, repo.DefaultBranch + err = db.WithTx(ctx, func(ctx context.Context) error { + if err := syncDefaultBranch(ctx, repo, gitRepo); err != nil { + return err + } + dbBranches, err := git_model.GetBranches(ctx, repo.ID, branchNames, true) if err != nil { return fmt.Errorf("git_model.GetBranches: %v", err) } @@ -335,15 +342,15 @@ func SyncBranchesToDB(ctx context.Context, repoID, pusherID int64, gitRepo *git. // if user haven't visited UI but directly push to a branch after upgrading from 1.20 -> 1.21, // we cannot simply insert the branch but need to check we have branches or not hasAnyActiveBranch, err := db.Exist[git_model.Branch](ctx, git_model.FindBranchOptions{ - RepoID: repoID, + RepoID: repo.ID, IsDeletedBranch: optional.Some(false), }.ToConds()) if err != nil { return err } if !hasAnyActiveBranch { - if _, err = repo_module.SyncRepoBranches(ctx, repoID, pusherID); err != nil { - return fmt.Errorf("repo_module.SyncRepoBranches %d failed: %v", repoID, err) + if _, _, err = repo_module.SyncRepoBranchesWithRepo(ctx, repo, gitRepo, pusherID); err != nil { + return fmt.Errorf("repo_module.SyncRepoBranchesWithRepo %d failed: %v", repo.ID, err) } return nil } @@ -361,8 +368,8 @@ func SyncBranchesToDB(ctx context.Context, repoID, pusherID int64, gitRepo *git. commit, err := gitRepo.GetCommit(ctx, git.RefNameFromBranch(branchName).String()) if git.IsErrNotExist(err) { if existInDb && !branch.IsDeleted { - if err := git_model.MarkBranchAsDeleted(ctx, repoID, branchName, pusherID); err != nil { - log.Error("SyncBranchesToDB: mark branch %d:%s deleted: %v", repoID, branchName, err) + if err := git_model.MarkBranchAsDeleted(ctx, repo.ID, branchName, pusherID); err != nil { + log.Error("SyncBranchesToDB: mark branch %d:%s deleted: %v", repo.ID, branchName, err) } } continue @@ -375,15 +382,15 @@ func SyncBranchesToDB(ctx context.Context, repoID, pusherID int64, gitRepo *git. } if existInDb { - if _, err := git_model.UpdateBranch(ctx, repoID, pusherID, branchName, commit); err != nil { - return fmt.Errorf("git_model.UpdateBranch %d:%s failed: %v", repoID, branchName, err) + if _, err := git_model.UpdateBranch(ctx, repo.ID, pusherID, branchName, commit); err != nil { + return fmt.Errorf("git_model.UpdateBranch %d:%s failed: %v", repo.ID, branchName, err) } continue } // if database have branches but not this branch, it means this is a new branch newBranches = append(newBranches, &git_model.Branch{ - RepoID: repoID, + RepoID: repo.ID, Name: branchName, CommitID: commit.ID.String(), CommitMessage: commit.MessageTitle(), @@ -397,6 +404,46 @@ func SyncBranchesToDB(ctx context.Context, repoID, pusherID int64, gitRepo *git. } return nil }) + if err != nil { + repo.IsEmpty, repo.DefaultBranch = wasEmpty, oldDefaultBranch + return err + } + if !wasEmpty && repo.DefaultBranch == oldDefaultBranch && slices.Contains(branchNames, repo.DefaultBranch) { + if err := git.SetDefaultBranch(ctx, repo, repo.DefaultBranch); err != nil { + log.Error("SyncBranchesToDB: set HEAD of %s: %v", repo.FullName(), err) + } + } + return nil +} + +func syncDefaultBranch(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository) error { + freshRepo, err := repo_model.GetRepositoryByID(ctx, repo.ID) // read under the lock, another push may have changed it meanwhile + if err != nil { + return err + } + repo.IsEmpty, repo.DefaultBranch = freshRepo.IsEmpty, freshRepo.DefaultBranch + defaultBranch, err := findDefaultBranch(ctx, repo, gitRepo) + if err != nil || defaultBranch == "" || (!repo.IsEmpty && defaultBranch == repo.DefaultBranch) { + return err + } + if err = git.SetDefaultBranch(ctx, repo, defaultBranch); err != nil { + return err + } + repo.IsEmpty, repo.DefaultBranch = false, defaultBranch + return repo_model.UpdateRepositoryColsWithAutoTime(ctx, repo, "is_empty", "default_branch") +} + +// findDefaultBranch matches GitLab's Gitaly GetDefaultBranch +func findDefaultBranch(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository) (string, error) { + for _, branch := range []string{repo.DefaultBranch, "main", "master"} { + if _, err := gitRepo.GetBranchCommitID(ctx, branch); err == nil { + return branch, nil + } else if !errors.Is(err, util.ErrNotExist) { + return "", err + } + } + stdout, _, err := gitcmd.NewCommand("for-each-ref", "--count=1", "--format=%(refname:strip=2)", git.BranchPrefix).WithRepo(repo).RunStdString(ctx) + return strings.TrimSuffix(stdout, "\n"), err } // CreateNewBranchFromCommit creates a new repository branch diff --git a/services/repository/branch_test.go b/services/repository/branch_test.go index d695553efef..3b777c98d26 100644 --- a/services/repository/branch_test.go +++ b/services/repository/branch_test.go @@ -4,6 +4,7 @@ package repository import ( + "context" "testing" "gitea.dev/models/db" @@ -59,7 +60,7 @@ func TestSyncBranchesToDBUsesGitState(t *testing.T) { require.NoError(t, err) require.NoError(t, git_model.MarkBranchAsDeleted(ctx, repo.ID, "develop", 1)) - require.NoError(t, SyncBranchesToDB(ctx, repo.ID, 2, gitRepo, []string{"master", "foo", "develop", "pr-to-update"})) + require.NoError(t, SyncBranchesToDB(ctx, repo, 2, gitRepo, []string{"master", "foo", "develop", "pr-to-update"})) loadBranch := func(name string) *git_model.Branch { return unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo.ID, Name: name}) @@ -69,3 +70,51 @@ func TestSyncBranchesToDBUsesGitState(t *testing.T) { assert.False(t, loadBranch("develop").IsDeleted) assert.Equal(t, "62fb502a7172d4453f0322a2cc85bddffa57f07a", loadBranch("pr-to-update").CommitID) } + +func TestSyncBranchesToDBDefaultBranch(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + for _, tc := range []struct { + name string + dbEmpty bool + dbDefault string + branches []string + expected string + }{ + {name: "KeepDefaultBranch", dbEmpty: true, dbDefault: "main", branches: []string{"dev", "main"}, expected: "main"}, + {name: "GuessMaster", dbEmpty: true, dbDefault: "custom", branches: []string{"branch-0", "master"}, expected: "master"}, + {name: "RepairWithFirstBranchByName", dbDefault: "gone", branches: []string{"zeta", "alpha"}, expected: "alpha"}, + {name: "KeepDefaultOfConcurrentInit", dbDefault: "dev", branches: []string{"dev", "main"}, expected: "dev"}, + {name: "StayEmptyWithoutBranches", dbEmpty: true, dbDefault: "main", expected: "main"}, + } { + t.Run(tc.name, func(t *testing.T) { + repo, err := CreateRepositoryDirectly(t.Context(), user2, user2, CreateRepoOptions{Name: "default-branch-test", DefaultBranch: "main"}, true) + require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, DeleteRepositoryDirectly(context.WithoutCancel(t.Context()), repo.ID)) + }) + require.NoError(t, repo_model.UpdateRepositoryColsNoAutoTime(t.Context(), &repo_model.Repository{ID: repo.ID, IsEmpty: tc.dbEmpty, DefaultBranch: tc.dbDefault}, "is_empty", "default_branch")) + require.NoError(t, git.SetDefaultBranch(t.Context(), repo, tc.dbDefault)) + commits := make([]git.FastImportCommit, len(tc.branches)) + for i, branch := range tc.branches { + commits[i].Ref = git.BranchPrefix + branch + } + require.NoError(t, git.ForceFastImport(t.Context(), repo, commits)) + gitRepo, err := git.OpenRepository(t.Context(), repo) + require.NoError(t, err) + defer gitRepo.Close() + + require.NoError(t, SyncBranchesToDB(t.Context(), repo, user2.ID, gitRepo, append(tc.branches, "deleted"))) + + dbRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, len(tc.branches) == 0, dbRepo.IsEmpty) + assert.Equal(t, tc.expected, dbRepo.DefaultBranch) + assert.Equal(t, dbRepo.IsEmpty, repo.IsEmpty) + assert.Equal(t, tc.expected, repo.DefaultBranch) + headBranch, err := git.GetDefaultBranch(t.Context(), repo) + require.NoError(t, err) + assert.Equal(t, tc.expected, headBranch) + }) + } +} diff --git a/services/repository/files/update.go b/services/repository/files/update.go index d746de7262b..8b2e80b6bb1 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -26,6 +26,7 @@ import ( "gitea.dev/routers/api/v1/utils" asymkey_service "gitea.dev/services/asymkey" pull_service "gitea.dev/services/pull" + repo_service "gitea.dev/services/repository" ) // IdentityOptions for a person's identity like an author or committer @@ -287,20 +288,15 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use return nil, err } - // FIXME: this call seems not right, why it needs to read the file content again - // FIXME: why it uses the NewBranch as "ref", it should use the commit ID because the response is only for this commit - filesResponse, err := GetFilesResponseFromCommit(ctx, repo, gitRepo, utils.NewRefCommit(git.RefNameFromBranch(opts.NewBranch), commit), treePaths) - if err != nil { - return nil, err - } - if repo.IsEmpty { - if isEmpty, err := gitRepo.IsEmpty(ctx); err == nil && !isEmpty { - _ = repo_model.UpdateRepositoryColsWithAutoTime(ctx, &repo_model.Repository{ID: repo.ID, IsEmpty: false, DefaultBranch: opts.NewBranch}, "is_empty", "default_branch") + if err := repo_service.SyncBranchesToDB(ctx, repo, doer.ID, gitRepo, []string{opts.NewBranch}); err != nil { + log.Error("SyncBranchesToDB: %v", err) } } - return filesResponse, nil + // FIXME: this call seems not right, why it needs to read the file content again + // FIXME: why it uses the NewBranch as "ref", it should use the commit ID because the response is only for this commit + return GetFilesResponseFromCommit(ctx, repo, gitRepo, utils.NewRefCommit(git.RefNameFromBranch(opts.NewBranch), commit), treePaths) } // ErrRepoFileAlreadyExists represents a "RepoFileAlreadyExist" kind of error. diff --git a/services/repository/push.go b/services/repository/push.go index 6a3799006be..29ad4ae8929 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -254,17 +254,9 @@ func getCompareURL(ctx context.Context, repo *repo_model.Repository, gitRepo *gi } func pushNewBranch(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, pusher *user_model.User, opts *repo_module.PushUpdateOptions, newCommit *git.Commit) ([]*git.Commit, error) { - if repo.IsEmpty { // Change default branch and empty status only if pushed ref is non-empty branch. - repo.DefaultBranch = opts.RefName() - repo.IsEmpty = false - if repo.DefaultBranch != setting.Repository.DefaultBranch { - if err := git.SetDefaultBranch(ctx, repo, repo.DefaultBranch); err != nil { - return nil, err - } - } - // Update the is empty and default_branch columns - if err := repo_model.UpdateRepositoryColsWithAutoTime(ctx, repo, "default_branch", "is_empty"); err != nil { - return nil, fmt.Errorf("UpdateRepositoryCols: %w", err) + if repo.IsEmpty { + if err := SyncBranchesToDB(ctx, repo, pusher.ID, gitRepo, []string{opts.RefName()}); err != nil { + return nil, fmt.Errorf("pushNewBranch: %w", err) } } diff --git a/tests/integration/git_push_test.go b/tests/integration/git_push_test.go index d38d59b5c40..039096225db 100644 --- a/tests/integration/git_push_test.go +++ b/tests/integration/git_push_test.go @@ -103,7 +103,13 @@ func testGitPush(t *testing.T, u *url.URL) { doGitCreateBranch(gitPath, branchName)(t) } pushed = append(pushed, "master") - doGitPushTestRepository(gitPath, "origin", "--all")(t) + _, stderr, err := gitcmd.NewCommand("push", "origin", "--all").WithDir(gitPath).RunStdString(t.Context()) + require.NoError(t, err) + assert.NotContains(t, stderr, "Create a new pull request") + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 2, LowerName: "repo-to-push"}) + assert.False(t, repo.IsEmpty) + assert.Equal(t, "master", repo.DefaultBranch) return pushed, deleted }) }) @@ -139,7 +145,9 @@ func testGitPush(t *testing.T, u *url.URL) { pushed = append(pushed, "master") // push all, so that master is not changed - doGitPushTestRepository(gitPath, "origin", "--all")(t) + _, stderr, err := gitcmd.NewCommand("push", "origin", "--all").WithDir(gitPath).RunStdString(t.Context()) + require.NoError(t, err) + assert.Contains(t, stderr, "Create a new pull request for 'branch-2'") return pushed, deleted })