From 198f37e33cc67f4951681391644e395f9c76c7eb Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 19 Sep 2025 08:04:18 -0700 Subject: [PATCH] Move updateref and removeref to gitrepo and remove unnecessary open repository (#35511) Extracted from #35077 `UpdateRef` and `RemoveRef` will call git commands even for gogit version. --- modules/git/repo_commit_gogit.go | 10 ---------- modules/git/repo_commit_nogogit.go | 12 ------------ modules/git/repo_compare_test.go | 10 ++++++++-- modules/gitrepo/ref.go | 21 +++++++++++++++++++++ routers/api/v1/repo/issue.go | 2 +- routers/web/repo/issue_list.go | 2 +- routers/web/repo/issue_new.go | 2 +- routers/web/repo/pull.go | 2 +- services/issue/issue.go | 10 +++++++--- services/migrations/gitea_uploader.go | 6 ++---- services/pull/pull.go | 16 ++++------------ 11 files changed, 46 insertions(+), 47 deletions(-) create mode 100644 modules/gitrepo/ref.go diff --git a/modules/git/repo_commit_gogit.go b/modules/git/repo_commit_gogit.go index c830e52b7e..fc653714ad 100644 --- a/modules/git/repo_commit_gogit.go +++ b/modules/git/repo_commit_gogit.go @@ -38,16 +38,6 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) { return ref.Hash().String(), nil } -// SetReference sets the commit ID string of given reference (e.g. branch or tag). -func (repo *Repository) SetReference(name, commitID string) error { - return repo.gogitRepo.Storer.SetReference(plumbing.NewReferenceFromStrings(name, commitID)) -} - -// RemoveReference removes the given reference (e.g. branch or tag). -func (repo *Repository) RemoveReference(name string) error { - return repo.gogitRepo.Storer.RemoveReference(plumbing.ReferenceName(name)) -} - // ConvertToHash returns a Hash object from a potential ID string func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) { objectFormat, err := repo.GetObjectFormat() diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go index f00d407ae0..d2c66a541b 100644 --- a/modules/git/repo_commit_nogogit.go +++ b/modules/git/repo_commit_nogogit.go @@ -51,18 +51,6 @@ func (repo *Repository) GetRefCommitID(name string) (string, error) { return string(shaBs), nil } -// SetReference sets the commit ID string of given reference (e.g. branch or tag). -func (repo *Repository) SetReference(name, commitID string) error { - _, _, err := gitcmd.NewCommand("update-ref").AddDynamicArguments(name, commitID).RunStdString(repo.Ctx, &gitcmd.RunOpts{Dir: repo.Path}) - return err -} - -// RemoveReference removes the given reference (e.g. branch or tag). -func (repo *Repository) RemoveReference(name string) error { - _, _, err := gitcmd.NewCommand("update-ref", "--no-deref", "-d").AddDynamicArguments(name).RunStdString(repo.Ctx, &gitcmd.RunOpts{Dir: repo.Path}) - return err -} - // IsCommitExist returns true if given commit exists in current repository. func (repo *Repository) IsCommitExist(name string) bool { if err := ensureValidGitRepository(repo.Ctx, repo.Path); err != nil { diff --git a/modules/git/repo_compare_test.go b/modules/git/repo_compare_test.go index 35636fae1a..47fd2ca102 100644 --- a/modules/git/repo_compare_test.go +++ b/modules/git/repo_compare_test.go @@ -9,6 +9,8 @@ import ( "path/filepath" "testing" + "code.gitea.io/gitea/modules/git/gitcmd" + "github.com/stretchr/testify/assert" ) @@ -99,7 +101,9 @@ func TestReadWritePullHead(t *testing.T) { // Write a fake sha1 with only 40 zeros newCommit := "feaf4ba6bc635fec442f46ddd4512416ec43c2c2" - err = repo.SetReference(PullPrefix+"1/head", newCommit) + _, _, err = gitcmd.NewCommand("update-ref"). + AddDynamicArguments(PullPrefix+"1/head", newCommit). + RunStdString(t.Context(), &gitcmd.RunOpts{Dir: repo.Path}) if err != nil { assert.NoError(t, err) return @@ -116,7 +120,9 @@ func TestReadWritePullHead(t *testing.T) { assert.Equal(t, headContents, newCommit) // Remove file after the test - err = repo.RemoveReference(PullPrefix + "1/head") + _, _, err = gitcmd.NewCommand("update-ref", "--no-deref", "-d"). + AddDynamicArguments(PullPrefix+"1/head"). + RunStdString(t.Context(), &gitcmd.RunOpts{Dir: repo.Path}) assert.NoError(t, err) } diff --git a/modules/gitrepo/ref.go b/modules/gitrepo/ref.go new file mode 100644 index 0000000000..babef8b65f --- /dev/null +++ b/modules/gitrepo/ref.go @@ -0,0 +1,21 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gitrepo + +import ( + "context" + + "code.gitea.io/gitea/modules/git/gitcmd" +) + +func UpdateRef(ctx context.Context, repo Repository, refName, newCommitID string) error { + _, _, err := gitcmd.NewCommand("update-ref").AddDynamicArguments(refName, newCommitID).RunStdString(ctx, &gitcmd.RunOpts{Dir: repoPath(repo)}) + return err +} + +func RemoveRef(ctx context.Context, repo Repository, refName string) error { + _, _, err := gitcmd.NewCommand("update-ref", "--no-deref", "-d"). + AddDynamicArguments(refName).RunStdString(ctx, &gitcmd.RunOpts{Dir: repoPath(repo)}) + return err +} diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index d4a5872fd1..b11e889eb5 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -979,7 +979,7 @@ func DeleteIssue(ctx *context.APIContext) { return } - if err = issue_service.DeleteIssue(ctx, ctx.Doer, ctx.Repo.GitRepo, issue); err != nil { + if err = issue_service.DeleteIssue(ctx, ctx.Doer, issue); err != nil { ctx.APIErrorInternal(err) return } diff --git a/routers/web/repo/issue_list.go b/routers/web/repo/issue_list.go index fd34422cfc..a11d35da1e 100644 --- a/routers/web/repo/issue_list.go +++ b/routers/web/repo/issue_list.go @@ -398,7 +398,7 @@ func BatchDeleteIssues(ctx *context.Context) { return } for _, issue := range issues { - if err := issue_service.DeleteIssue(ctx, ctx.Doer, ctx.Repo.GitRepo, issue); err != nil { + if err := issue_service.DeleteIssue(ctx, ctx.Doer, issue); err != nil { ctx.ServerError("DeleteIssue", err) return } diff --git a/routers/web/repo/issue_new.go b/routers/web/repo/issue_new.go index 887019b146..1393f62fa5 100644 --- a/routers/web/repo/issue_new.go +++ b/routers/web/repo/issue_new.go @@ -216,7 +216,7 @@ func DeleteIssue(ctx *context.Context) { return } - if err := issue_service.DeleteIssue(ctx, ctx.Doer, ctx.Repo.GitRepo, issue); err != nil { + if err := issue_service.DeleteIssue(ctx, ctx.Doer, issue); err != nil { ctx.ServerError("DeleteIssueByID", err) return } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index c9ea836899..1ce2047b13 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -224,7 +224,7 @@ func GetMergedBaseCommitID(ctx *context.Context, issue *issues_model.Issue) stri commitSHA, err = ctx.Repo.GitRepo.ReadPatchCommit(pull.Index) if err == nil { // Recreate pull head in files for next time - if err := ctx.Repo.GitRepo.SetReference(pull.GetGitHeadRefName(), commitSHA); err != nil { + if err := gitrepo.UpdateRef(ctx, ctx.Repo.Repository, pull.GetGitHeadRefName(), commitSHA); err != nil { log.Error("Could not write head file", err) } } else { diff --git a/services/issue/issue.go b/services/issue/issue.go index ae81b9f48b..62b330f8e2 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -17,6 +17,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/storage" notify_service "code.gitea.io/gitea/services/notify" @@ -180,7 +181,7 @@ func UpdateAssignees(ctx context.Context, issue *issues_model.Issue, oneAssignee } // DeleteIssue deletes an issue -func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue) error { +func DeleteIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Issue) error { // load issue before deleting it if err := issue.LoadAttributes(ctx); err != nil { return err @@ -199,8 +200,11 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi } // delete pull request related git data - if issue.IsPull && gitRepo != nil { - if err := gitRepo.RemoveReference(issue.PullRequest.GetGitHeadRefName()); err != nil { + if issue.IsPull { + if err := issue.PullRequest.LoadBaseRepo(ctx); err != nil { + return err + } + if err := gitrepo.RemoveRef(ctx, issue.PullRequest.BaseRepo, issue.PullRequest.GetGitHeadRefName()); err != nil { return err } } diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 512468a832..02c61bd0f1 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -682,8 +682,7 @@ func (g *GiteaLocalUploader) updateGitForPullRequest(ctx context.Context, pr *ba pr.Head.SHA = headSha } - _, _, err = gitcmd.NewCommand("update-ref", "--no-deref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.Head.SHA).RunStdString(ctx, &gitcmd.RunOpts{Dir: g.repo.RepoPath()}) - if err != nil { + if err = gitrepo.UpdateRef(ctx, g.repo, pr.GetGitHeadRefName(), pr.Head.SHA); err != nil { return "", err } @@ -705,8 +704,7 @@ func (g *GiteaLocalUploader) updateGitForPullRequest(ctx context.Context, pr *ba log.Warn("Deprecated local head %s for PR #%d in %s/%s, removing %s", pr.Head.SHA, pr.Number, g.repoOwner, g.repoName, pr.GetGitHeadRefName()) } else { // set head information - _, _, err = gitcmd.NewCommand("update-ref", "--no-deref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.Head.SHA).RunStdString(ctx, &gitcmd.RunOpts{Dir: g.repo.RepoPath()}) - if err != nil { + if err = gitrepo.UpdateRef(ctx, g.repo, pr.GetGitHeadRefName(), pr.Head.SHA); err != nil { log.Error("unable to set %s as the local head for PR #%d from %s in %s/%s. Error: %v", pr.Head.SHA, pr.Number, pr.Head.Ref, g.repoOwner, g.repoName, err) } } diff --git a/services/pull/pull.go b/services/pull/pull.go index c5bb60ccb1..c7b3a0d523 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -108,13 +108,6 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { assigneeCommentMap := make(map[int64]*issues_model.Comment) - // add first push codes comment - baseGitRepo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo) - if err != nil { - return err - } - defer baseGitRepo.Close() - var reviewNotifiers []*issue_service.ReviewRequestNotifier if err := db.WithTx(ctx, func(ctx context.Context) error { if err := issues_model.NewPullRequest(ctx, repo, issue, labelIDs, uuids, pr); err != nil { @@ -141,6 +134,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { return err } + // add first push codes comment if _, err := CreatePushPullComment(ctx, issue.Poster, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitHeadRefName(), false); err != nil { return err } @@ -154,12 +148,11 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { return nil }); err != nil { // cleanup: this will only remove the reference, the real commit will be clean up when next GC - if err1 := baseGitRepo.RemoveReference(pr.GetGitHeadRefName()); err1 != nil { - log.Error("RemoveReference: %v", err1) + if err1 := gitrepo.RemoveRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName()); err1 != nil { + log.Error("RemoveRef: %v", err1) } return err } - baseGitRepo.Close() // close immediately to avoid notifications will open the repository again issue_service.ReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifiers) @@ -636,8 +629,7 @@ func UpdateRef(ctx context.Context, pr *issues_model.PullRequest) (err error) { return err } - _, _, err = gitcmd.NewCommand("update-ref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.HeadCommitID).RunStdString(ctx, &gitcmd.RunOpts{Dir: pr.BaseRepo.RepoPath()}) - if err != nil { + if err := gitrepo.UpdateRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName(), pr.HeadCommitID); err != nil { log.Error("Unable to update ref in base repository for PR[%d] Error: %v", pr.ID, err) }