From f371f84fa3456c2a71470632b6458d81e4892a54 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 20 Mar 2024 09:45:27 +0800 Subject: [PATCH] Restore deleted branches when syncing (#29898) Regression of #29493. If a branch has been deleted, repushing it won't restore it. Lunny may have noticed that, but I didn't delve into the comment then overlooked it: https://github.com/go-gitea/gitea/pull/29493#discussion_r1509046867 The additional comments added are to explain the issue I found during testing, which are unrelated to the fixes. --- routers/private/hook_post_receive.go | 8 +++++--- services/repository/branch.go | 4 ++-- tests/integration/git_push_test.go | 17 +++++++++++++++++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index 3dad39f7b..a09956f73 100644 --- a/routers/private/hook_post_receive.go +++ b/routers/private/hook_post_receive.go @@ -75,6 +75,10 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { updates = append(updates, option) if repo.IsEmpty && (refFullName.BranchName() == "master" || refFullName.BranchName() == "main") { // put the master/main branch first + // FIXME: It doesn't always work, since the master/main branch may not be the first batch of updates. + // If the user pushes many branches at once, the Git hook will call the internal API in batches, rather than all at once. + // See https://github.com/go-gitea/gitea/blob/cb52b17f92e2d2293f7c003649743464492bca48/cmd/hook.go#L27 + // If the user executes `git push origin --all` and pushes more than 30 branches, the master/main may not be the default branch. copy(updates[1:], updates) updates[0] = option } @@ -129,9 +133,7 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { commitIDs = append(commitIDs, update.NewCommitID) } - if err := repo_service.SyncBranchesToDB(ctx, repo.ID, opts.UserID, branchNames, commitIDs, func(commitID string) (*git.Commit, error) { - return gitRepo.GetCommit(commitID) - }); err != nil { + if err := repo_service.SyncBranchesToDB(ctx, repo.ID, opts.UserID, branchNames, commitIDs, gitRepo.GetCommit); err != nil { ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ Err: fmt.Sprintf("Failed to sync branch to DB in repository: %s/%s Error: %v", ownerName, repoName, err), }) diff --git a/services/repository/branch.go b/services/repository/branch.go index 8d8cfa2d1..db7acdb50 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -318,11 +318,11 @@ func SyncBranchesToDB(ctx context.Context, repoID, pusherID int64, branchNames, for i, branchName := range branchNames { commitID := commitIDs[i] branch, exist := branchMap[branchName] - if exist && branch.CommitID == commitID { + if exist && branch.CommitID == commitID && !branch.IsDeleted { continue } - commit, err := getCommit(branchName) + commit, err := getCommit(commitID) if err != nil { return fmt.Errorf("get commit of %s failed: %v", branchName, err) } diff --git a/tests/integration/git_push_test.go b/tests/integration/git_push_test.go index cb2910b17..0a3572480 100644 --- a/tests/integration/git_push_test.go +++ b/tests/integration/git_push_test.go @@ -69,6 +69,23 @@ func testGitPush(t *testing.T, u *url.URL) { return pushed, deleted }) }) + + t.Run("Push to deleted branch", func(t *testing.T) { + runTestGitPush(t, u, func(t *testing.T, gitPath string) (pushed, deleted []string) { + doGitPushTestRepository(gitPath, "origin", "master")(t) // make sure master is the default branch instead of a branch we are going to delete + pushed = append(pushed, "master") + + doGitCreateBranch(gitPath, "branch-1")(t) + doGitPushTestRepository(gitPath, "origin", "branch-1")(t) + pushed = append(pushed, "branch-1") + + // delete and restore + doGitPushTestRepository(gitPath, "origin", "--delete", "branch-1")(t) + doGitPushTestRepository(gitPath, "origin", "branch-1")(t) + + return pushed, deleted + }) + }) } func runTestGitPush(t *testing.T, u *url.URL, gitOperation func(t *testing.T, gitPath string) (pushed, deleted []string)) {