Move updateref and removeref to gitrepo and remove unnecessary open repository (#35511)

lunny · web-flow · commit 198f37e33cc6 · 2025-09-19T08:04:18.000-07:00
Extracted from #35077 `UpdateRef` and `RemoveRef` will call git commands even for gogit version.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -979,7 +979,7 @@ func DeleteIssue(ctx *context.APIContext) {`
`979`	`979`	`return`
`980`	`980`	`}`
`981`	`981`
`982`		`- if err = issue_service.DeleteIssue(ctx, ctx.Doer, ctx.Repo.GitRepo, issue); err != nil {`
	`982`	`+ if err = issue_service.DeleteIssue(ctx, ctx.Doer, issue); err != nil {`
`983`	`983`	`ctx.APIErrorInternal(err)`
`984`	`984`	`return`
`985`	`985`	`}`
Original file line number	Diff line number	Diff line change
`@@ -398,7 +398,7 @@ func BatchDeleteIssues(ctx *context.Context) {`
`398`	`398`	`return`
`399`	`399`	`}`
`400`	`400`	`for _, issue := range issues {`
`401`		`- if err := issue_service.DeleteIssue(ctx, ctx.Doer, ctx.Repo.GitRepo, issue); err != nil {`
	`401`	`+ if err := issue_service.DeleteIssue(ctx, ctx.Doer, issue); err != nil {`
`402`	`402`	`ctx.ServerError("DeleteIssue", err)`
`403`	`403`	`return`
`404`	`404`	`}`
Original file line number	Diff line number	Diff line change
`@@ -216,7 +216,7 @@ func DeleteIssue(ctx *context.Context) {`
`216`	`216`	`return`
`217`	`217`	`}`
`218`	`218`
`219`		`- if err := issue_service.DeleteIssue(ctx, ctx.Doer, ctx.Repo.GitRepo, issue); err != nil {`
	`219`	`+ if err := issue_service.DeleteIssue(ctx, ctx.Doer, issue); err != nil {`
`220`	`220`	`ctx.ServerError("DeleteIssueByID", err)`
`221`	`221`	`return`
`222`	`222`	`}`
Original file line number	Diff line number	Diff line change
`@@ -224,7 +224,7 @@ func GetMergedBaseCommitID(ctx context.Context, issue issues_model.Issue) stri`
`224`	`224`	`commitSHA, err = ctx.Repo.GitRepo.ReadPatchCommit(pull.Index)`
`225`	`225`	`if err == nil {`
`226`	`226`	`// Recreate pull head in files for next time`
`227`		`- if err := ctx.Repo.GitRepo.SetReference(pull.GetGitHeadRefName(), commitSHA); err != nil {`
	`227`	`+ if err := gitrepo.UpdateRef(ctx, ctx.Repo.Repository, pull.GetGitHeadRefName(), commitSHA); err != nil {`
`228`	`228`	`log.Error("Could not write head file", err)`
`229`	`229`	`}`
`230`	`230`	`} else {`
Original file line number	Diff line number	Diff line change
`@@ -682,8 +682,7 @@ func (g GiteaLocalUploader) updateGitForPullRequest(ctx context.Context, pr ba`
`682`	`682`	`pr.Head.SHA = headSha`
`683`	`683`	`}`
`684`	`684`
`685`		`- _, _, err = gitcmd.NewCommand("update-ref", "--no-deref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.Head.SHA).RunStdString(ctx, &gitcmd.RunOpts{Dir: g.repo.RepoPath()})`
`686`		`- if err != nil {`
	`685`	`+ if err = gitrepo.UpdateRef(ctx, g.repo, pr.GetGitHeadRefName(), pr.Head.SHA); err != nil {`
`687`	`686`	`return "", err`
`688`	`687`	`}`
`689`	`688`
`@@ -705,8 +704,7 @@ func (g GiteaLocalUploader) updateGitForPullRequest(ctx context.Context, pr ba`
`705`	`704`	`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())`
`706`	`705`	`} else {`
`707`	`706`	`// set head information`
`708`		`- _, _, err = gitcmd.NewCommand("update-ref", "--no-deref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.Head.SHA).RunStdString(ctx, &gitcmd.RunOpts{Dir: g.repo.RepoPath()})`
`709`		`- if err != nil {`
	`707`	`+ if err = gitrepo.UpdateRef(ctx, g.repo, pr.GetGitHeadRefName(), pr.Head.SHA); err != nil {`
`710`	`708`	`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)`
`711`	`709`	`}`
`712`	`710`	`}`