fix(repo): mark repository as non-empty before git push returns (#40102)

The post-receive hook syncs pushed branches to the database, but
`is_empty` and the default branch of an empty repository were only
updated later by the `push_update` queue. Until then the API treated the
repository as empty (branch list `null`, `GET /branches/{branch}` 500),
and the queue overwrote the default branch chosen by the hook or set via
the API.

`SyncBranchesToDB` now initializes an empty repository and repairs a
default branch missing in git in its locked transaction, so the state is
final when `git push` returns. The push queue and web file edits retry
it while the repository is still empty.

- The default branch is the configured one if it exists, else `main`,
else `master`, else the first branch by name, like GitLab. HEAD follows
it.
- A first push of several branches no longer depends on push order. The
push that initializes a repository still prints no pull request hints,
like GitHub.

---------

Co-authored-by: silverwind <me@silverwind.io>

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)
-		}
-	}
-}

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

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)
+		})
+	}
+}

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.

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)
 		}
 	}
 

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
 		})