refactor: make git package handle all git operations (#38543)

before: gitrepo vs git packages
after: git package fully handle all git operations

by the way, use `WithRepo(repo)` instead of `WithDir(repo.Path)` to hide
path details.

benefits:

1. remove all unnecessary wrappers, developers no need to struggle with
"which package should be used"
2. simplify code, RepositoryFacade can (will) be used everywhere, all
"path" details are (will be) hidden

services/pull/check.go

@@ -21,7 +21,6 @@ import (
 	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/git/gitcmd"
-	"gitea.dev/modules/gitrepo"
 	"gitea.dev/modules/globallock"
 	"gitea.dev/modules/graceful"
 	"gitea.dev/modules/log"
@@ -247,7 +246,7 @@ func checkSigningRequirements(ctx context.Context, pr *issues_model.PullRequest,
 		return nil
 	}
 
-	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
+	gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
 	if err != nil {
 		return err
 	}
@@ -328,12 +327,12 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com
 	// If merge-base successfully exits then prHeadRef is an ancestor of pr.BaseBranch
 
 	// Find the head commit id
-	prHeadCommitID, err := gitrepo.GetFullCommitID(ctx, pr.BaseRepo, prHeadRef)
+	prHeadCommitID, err := git.GetFullCommitID(ctx, pr.BaseRepo, prHeadRef)
 	if err != nil {
 		return nil, fmt.Errorf("GetFullCommitID(%s) in %s: %w", prHeadRef, pr.BaseRepo.FullName(), err)
 	}
 
-	gitRepo, err := gitrepo.OpenRepository(pr.BaseRepo)
+	gitRepo, err := git.OpenRepository(pr.BaseRepo)
 	if err != nil {
 		return nil, fmt.Errorf("%-v OpenRepository: %w", pr.BaseRepo, err)
 	}

services/pull/comment.go

@@ -12,7 +12,6 @@ import (
 	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/container"
 	"gitea.dev/modules/git"
-	"gitea.dev/modules/gitrepo"
 	"gitea.dev/modules/json"
 	"gitea.dev/modules/log"
 )
@@ -23,11 +22,11 @@ func preparePushPullCommentPushActionContent(ctx context.Context, pr *issues_mod
 	if isForcePush {
 		// if it's a force push, we need to get the whole pull request commits
 		// the force-push timeline comment should always be created, so all errors are ignored and logged only.
-		mergeBase, err := gitrepo.MergeBase(ctx, pr.BaseRepo, pr.BaseBranch, newCommitID)
+		mergeBase, err := git.MergeBase(ctx, pr.BaseRepo, pr.BaseBranch, newCommitID)
 		if err != nil {
 			log.Debug("MergeBase %q..%q failed: %v", pr.BaseBranch, newCommitID, err)
 		} else {
-			data.CommitIDs, err = gitrepo.GetCommitIDsBetweenReverse(ctx, pr.BaseRepo, mergeBase, newCommitID, "", maxPushCommitsInCommentCount)
+			data.CommitIDs, err = git.GetCommitIDsBetweenReverse(ctx, pr.BaseRepo, mergeBase, newCommitID, "", maxPushCommitsInCommentCount)
 			if err != nil {
 				log.Debug("GetCommitIDsBetweenReverse %q..%q failed: %v", mergeBase, newCommitID, err)
 			}
@@ -36,7 +35,7 @@ func preparePushPullCommentPushActionContent(ctx context.Context, pr *issues_mod
 	}
 
 	// for a normal push, it maybe an empty pull request, only non-empty pull request need to create push comment
-	data.CommitIDs, err = gitrepo.GetCommitIDsBetweenReverse(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch, maxPushCommitsInCommentCount)
+	data.CommitIDs, err = git.GetCommitIDsBetweenReverse(ctx, pr.BaseRepo, oldCommitID, newCommitID, pr.BaseBranch, maxPushCommitsInCommentCount)
 	return data, len(data.CommitIDs) > 0, err
 }
 
@@ -98,7 +97,7 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *iss
 		return nil, false, nil
 	}
 
-	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
+	gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
 	if err != nil {
 		return nil, false, err
 	}

services/pull/comment_test.go

@@ -11,7 +11,6 @@ import (
 	"gitea.dev/models/unittest"
 	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/git"
-	"gitea.dev/modules/gitrepo"
 	"gitea.dev/modules/json"
 
 	"github.com/stretchr/testify/assert"
@@ -26,7 +25,7 @@ func TestCreatePushPullCommentForcePushDeletesOldComments(t *testing.T) {
 	require.NoError(t, pr.LoadIssue(ctx))
 	require.NoError(t, pr.LoadBaseRepo(ctx))
 
-	gitRepo, err := gitrepo.OpenRepository(pr.BaseRepo)
+	gitRepo, err := git.OpenRepository(pr.BaseRepo)
 	require.NoError(t, err)
 	defer gitRepo.Close()
 

services/pull/commit_status.go

@@ -17,7 +17,7 @@ import (
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/modules/commitstatus"
 	"gitea.dev/modules/container"
-	"gitea.dev/modules/gitrepo"
+	"gitea.dev/modules/git"
 	"gitea.dev/modules/glob"
 	"gitea.dev/modules/log"
 )
@@ -108,7 +108,7 @@ func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullR
 	}
 
 	// check if all required status checks are successful
-	headGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.HeadRepo)
+	headGitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.HeadRepo)
 	if err != nil {
 		return "", fmt.Errorf("OpenRepository: %w", err)
 	}
@@ -121,7 +121,7 @@ func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullR
 			return "", errors.New("Head branch does not exist, can not merge")
 		}
 	}
-	if pr.Flow == issues_model.PullRequestFlowAGit && !gitrepo.IsReferenceExist(ctx, pr.HeadRepo, pr.GetGitHeadRefName()) {
+	if pr.Flow == issues_model.PullRequestFlowAGit && !git.IsReferenceExist(ctx, pr.HeadRepo, pr.GetGitHeadRefName()) {
 		return "", errors.New("Head branch does not exist, can not merge")
 	}
 

services/pull/lfs.go

@@ -13,6 +13,7 @@ import (
 
 	git_model "gitea.dev/models/git"
 	issues_model "gitea.dev/models/issues"
+	"gitea.dev/modules/git"
 	"gitea.dev/modules/git/gitcmd"
 	"gitea.dev/modules/git/pipeline"
 	"gitea.dev/modules/lfs"
@@ -23,8 +24,8 @@ import (
 )
 
 // LFSPush pushes lfs objects referred to in new commits in the head repository from the base repository
-func LFSPush(ctx context.Context, tmpBasePath, mergeHeadSHA, mergeBaseSHA string, pr *issues_model.PullRequest) error {
-	// Now we have to implement git lfs push
+func LFSPush(ctx context.Context, tmpBasePath string, tmpRepo git.RepositoryFacade, mergeHeadSHA, mergeBaseSHA string, pr *issues_model.PullRequest) error {
+	// Now we have to implement git-lfs push
 	// git rev-list --objects --filter=blob:limit=1k HEAD --not base
 	// pass blob shas in to git cat-file --batch-check (possibly unnecessary)
 	// ensure only blobs and <=1k size then pass in to git cat-file --batch
@@ -53,7 +54,7 @@ func LFSPush(ctx context.Context, tmpBasePath, mergeHeadSHA, mergeBaseSHA string
 
 	// 5. Take the shas of the blobs and batch read them
 	wg.Go(func() error {
-		return pipeline.CatFileBatch(ctx, cmd5BatchContent, tmpBasePath)
+		return pipeline.CatFileBatch(ctx, cmd5BatchContent, tmpRepo)
 	})
 
 	// 4. From the provided objects restrict to blobs <=1k

services/pull/merge.go

@@ -362,15 +362,15 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use
 	}
 
 	// OK we should cache our current head and origin/headbranch
-	mergeHeadSHA, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, "HEAD")
+	mergeHeadSHA, err := git.GetFullCommitID(ctx, mergeCtx.tmpRepo, "HEAD")
 	if err != nil {
 		return "", fmt.Errorf("Failed to get full commit id for HEAD: %w", err)
 	}
-	mergeBaseSHA, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, "original_"+tmpRepoBaseBranch)
+	mergeBaseSHA, err := git.GetFullCommitID(ctx, mergeCtx.tmpRepo, "original_"+tmpRepoBaseBranch)
 	if err != nil {
 		return "", fmt.Errorf("Failed to get full commit id for origin/%s: %w", pr.BaseBranch, err)
 	}
-	mergeCommitID, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, tmpRepoBaseBranch)
+	mergeCommitID, err := git.GetFullCommitID(ctx, mergeCtx.tmpRepo, tmpRepoBaseBranch)
 	if err != nil {
 		return "", fmt.Errorf("Failed to get full commit id for the new merge: %w", err)
 	}
@@ -379,7 +379,7 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use
 	// I think in the interests of data safety - failures to push to the lfs should prevent
 	// the merge as you can always remerge.
 	if setting.LFS.StartServer {
-		if err := LFSPush(ctx, mergeCtx.tmpBasePath, mergeHeadSHA, mergeBaseSHA, pr); err != nil {
+		if err := LFSPush(ctx, mergeCtx.tmpBasePath, mergeCtx.tmpRepo, mergeHeadSHA, mergeBaseSHA, pr); err != nil {
 			return "", err
 		}
 	}

services/pull/merge_rebase.go

@@ -10,7 +10,6 @@ import (
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/git/gitcmd"
-	"gitea.dev/modules/gitrepo"
 	"gitea.dev/modules/log"
 )
 
@@ -38,7 +37,7 @@ func getRebaseAmendMessage(ctx *mergeContext, baseGitRepo *git.Repository) (mess
 
 // Perform rebase merge without merge commit.
 func doMergeRebaseFastForward(ctx *mergeContext) error {
-	baseHeadSHA, err := git.GetFullCommitID(ctx, ctx.tmpBasePath, "HEAD")
+	baseHeadSHA, err := git.GetFullCommitID(ctx, ctx.tmpRepo, "HEAD")
 	if err != nil {
 		return fmt.Errorf("Failed to get full commit id for HEAD: %w", err)
 	}
@@ -49,8 +48,8 @@ func doMergeRebaseFastForward(ctx *mergeContext) error {
 		return err
 	}
 
-	// Check if anything actually changed before we amend the message, fast forward can skip commits.
-	newMergeHeadSHA, err := git.GetFullCommitID(ctx, ctx.tmpBasePath, "HEAD")
+	// Check if anything actually changed before we amend the message, fast-forward can skip commits.
+	newMergeHeadSHA, err := git.GetFullCommitID(ctx, ctx.tmpRepo, "HEAD")
 	if err != nil {
 		return fmt.Errorf("Failed to get full commit id for HEAD: %w", err)
 	}
@@ -59,7 +58,7 @@ func doMergeRebaseFastForward(ctx *mergeContext) error {
 	}
 
 	// Original repo to read template from.
-	baseGitRepo, err := gitrepo.OpenRepository(ctx.pr.BaseRepo)
+	baseGitRepo, err := git.OpenRepository(ctx.pr.BaseRepo)
 	if err != nil {
 		log.Error("Unable to get Git repo for rebase: %v", err)
 		return err

services/pull/merge_tree.go

@@ -11,7 +11,6 @@ import (
 	issues_model "gitea.dev/models/issues"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/git/gitcmd"
-	"gitea.dev/modules/gitrepo"
 	"gitea.dev/modules/log"
 )
 
@@ -19,7 +18,7 @@ import (
 // return true if there are conflicts otherwise return false
 // pr.Status and pr.ConflictedFiles will be updated as necessary
 func checkConflictsMergeTree(ctx context.Context, pr *issues_model.PullRequest, baseCommitID string) (bool, error) {
-	treeHash, conflict, conflictFiles, err := gitrepo.MergeTree(ctx, pr.BaseRepo, baseCommitID, pr.HeadCommitID, pr.MergeBase)
+	treeHash, conflict, conflictFiles, err := git.MergeTree(ctx, pr.BaseRepo, baseCommitID, pr.HeadCommitID, pr.MergeBase)
 	if err != nil {
 		return false, fmt.Errorf("MergeTree: %w", err)
 	}
@@ -55,7 +54,7 @@ func checkPullRequestMergeableByMergeTree(ctx context.Context, pr *issues_model.
 	if err := pr.LoadHeadRepo(ctx); err != nil {
 		return err
 	}
-	headGitRepo, err := gitrepo.OpenRepository(pr.HeadRepo)
+	headGitRepo, err := git.OpenRepository(pr.HeadRepo)
 	if err != nil {
 		return fmt.Errorf("OpenRepository: %w", err)
 	}
@@ -66,7 +65,7 @@ func checkPullRequestMergeableByMergeTree(ctx context.Context, pr *issues_model.
 	if pr.IsSameRepo() {
 		baseGitRepo = headGitRepo
 	} else {
-		baseGitRepo, err = gitrepo.OpenRepository(pr.BaseRepo)
+		baseGitRepo, err = git.OpenRepository(pr.BaseRepo)
 		if err != nil {
 			return fmt.Errorf("OpenRepository: %w", err)
 		}
@@ -94,7 +93,7 @@ func checkPullRequestMergeableByMergeTree(ctx context.Context, pr *issues_model.
 	// it will be checked in 2 weeks by default from git if the pull request created failure.
 	if !pr.IsSameRepo() {
 		if !baseGitRepo.IsReferenceExist(ctx, pr.HeadCommitID) {
-			if err := gitrepo.FetchRemoteCommit(ctx, pr.BaseRepo, pr.HeadRepo, pr.HeadCommitID); err != nil {
+			if err := git.FetchRemoteCommit(ctx, pr.BaseRepo, pr.HeadRepo, pr.HeadCommitID); err != nil {
 				return fmt.Errorf("FetchRemoteCommit: %w", err)
 			}
 		}
@@ -106,7 +105,7 @@ func checkPullRequestMergeableByMergeTree(ctx context.Context, pr *issues_model.
 		return fmt.Errorf("GetBranchCommitID: can't find commit ID for base: %w", err)
 	}
 
-	pr.MergeBase, err = gitrepo.MergeBase(ctx, pr.BaseRepo, baseCommitID, pr.HeadCommitID)
+	pr.MergeBase, err = git.MergeBase(ctx, pr.BaseRepo, baseCommitID, pr.HeadCommitID)
 	if err != nil {
 		// if there is no merge base, then it's empty, still need to allow the pull request to be created
 		// not quite right (e.g.: why not reset the fields like below), but no interest to do more investigation at the moment

services/pull/patch.go

@@ -16,7 +16,6 @@ import (
 	issues_model "gitea.dev/models/issues"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/git/gitcmd"
-	"gitea.dev/modules/gitrepo"
 	"gitea.dev/modules/glob"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/process"
@@ -30,7 +29,7 @@ func DownloadDiffOrPatch(ctx context.Context, pr *issues_model.PullRequest, w io
 		return err
 	}
 
-	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
+	gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
 	if err != nil {
 		return fmt.Errorf("OpenRepository: %w", err)
 	}

services/pull/pull.go

@@ -27,7 +27,6 @@ import (
 	"gitea.dev/modules/container"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/git/gitcmd"
-	"gitea.dev/modules/gitrepo"
 	"gitea.dev/modules/globallock"
 	"gitea.dev/modules/graceful"
 	"gitea.dev/modules/log"
@@ -157,7 +156,7 @@ 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 := gitrepo.RemoveRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName()); err1 != nil {
+		if err1 := git.RemoveRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName()); err1 != nil {
 			log.Error("RemoveRef: %v", err1)
 		}
 		return err
@@ -365,7 +364,7 @@ func checkForInvalidation(ctx context.Context, requests issues_model.PullRequest
 	if err != nil {
 		return fmt.Errorf("GetRepositoryByIDCtx: %w", err)
 	}
-	gitRepo, err := gitrepo.OpenRepository(repo)
+	gitRepo, err := git.OpenRepository(repo)
 	if err != nil {
 		return fmt.Errorf("gitrepo.OpenRepository: %w", err)
 	}
@@ -528,7 +527,7 @@ func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest,
 	}
 	defer cancel()
 
-	mergeBase, err = gitrepo.MergeBase(ctx, pr.BaseRepo, pr.BaseBranch, pr.GetGitHeadRefName())
+	mergeBase, err = git.MergeBase(ctx, pr.BaseRepo, pr.BaseBranch, pr.GetGitHeadRefName())
 	if err != nil {
 		return false, "", fmt.Errorf("GetMergeBase: %w", err)
 	}
@@ -586,7 +585,7 @@ func pushToBaseRepoHelper(ctx context.Context, pr *issues_model.PullRequest, pre
 
 	gitRefName := pr.GetGitHeadRefName()
 
-	if err := gitrepo.PushManaged(ctx, pr.HeadRepo, pr.BaseRepo, git.PushOptions{
+	if err := git.PushManaged(ctx, pr.HeadRepo, pr.BaseRepo, git.PushOptions{
 		Branch: prefixHeadBranch + pr.HeadBranch + ":" + gitRefName,
 		Force:  true,
 		// Use InternalPushingEnvironment here because we know that pre-receive and post-receive do not run on a refs/pulls/...
@@ -643,7 +642,7 @@ func UpdateRef(ctx context.Context, pr *issues_model.PullRequest) (err error) {
 		return err
 	}
 
-	if err := gitrepo.UpdateRef(ctx, pr.BaseRepo, pr.GetGitHeadRefName(), pr.HeadCommitID); err != nil {
+	if err := git.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)
 	}
 
@@ -745,7 +744,7 @@ func AdjustPullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User
 
 // CloseRepoBranchesPulls close all pull requests which head branches are in the given repository, but only whose base repo is not in the given repository
 func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *repo_model.Repository) error {
-	branches, _, err := gitrepo.GetBranchesByPath(ctx, repo, 0, 0)
+	branches, _, err := git.GetBranchesByPath(ctx, repo, 0, 0)
 	if err != nil {
 		return err
 	}
@@ -793,7 +792,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
 		}
 	}
 
-	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.HeadRepo)
+	gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.HeadRepo)
 	if err != nil {
 		return "", err
 	}
@@ -960,7 +959,7 @@ func GetIssuesAllCommitStatus(ctx context.Context, issues issues_model.IssueList
 		}
 		gitRepo, ok := gitRepos[issue.RepoID]
 		if !ok {
-			gitRepo, err = gitrepo.OpenRepository(issue.Repo)
+			gitRepo, err = git.OpenRepository(issue.Repo)
 			if err != nil {
 				log.Error("Cannot open git repository %-v for issue #%d[%d]. Error: %v", issue.Repo, issue.Index, issue.ID, err)
 				continue
@@ -997,7 +996,7 @@ func IsHeadEqualWithBranch(ctx context.Context, pr *issues_model.PullRequest, br
 	if err = pr.LoadBaseRepo(ctx); err != nil {
 		return false, err
 	}
-	baseGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
+	baseGitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
 	if err != nil {
 		return false, err
 	}
@@ -1017,7 +1016,7 @@ func IsHeadEqualWithBranch(ctx context.Context, pr *issues_model.PullRequest, br
 	} else {
 		var closer io.Closer
 
-		headGitRepo, closer, err = gitrepo.RepositoryFromContextOrOpen(ctx, pr.HeadRepo)
+		headGitRepo, closer, err = git.RepositoryFromContextOrOpen(ctx, pr.HeadRepo)
 		if err != nil {
 			return false, err
 		}

services/pull/pull_test.go

@@ -12,7 +12,6 @@ import (
 	"gitea.dev/models/unit"
 	"gitea.dev/models/unittest"
 	"gitea.dev/modules/git"
-	"gitea.dev/modules/gitrepo"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/test"
 
@@ -45,7 +44,7 @@ func TestPullRequest_GetDefaultMergeMessage_InternalTracker(t *testing.T) {
 	pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
 
 	assert.NoError(t, pr.LoadBaseRepo(t.Context()))
-	gitRepo, err := gitrepo.OpenRepository(pr.BaseRepo)
+	gitRepo, err := git.OpenRepository(pr.BaseRepo)
 	assert.NoError(t, err)
 	defer gitRepo.Close()
 
@@ -75,7 +74,7 @@ func TestPullRequest_GetDefaultMergeMessage_ExternalTracker(t *testing.T) {
 	pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2, BaseRepo: baseRepo})
 
 	assert.NoError(t, pr.LoadBaseRepo(t.Context()))
-	gitRepo, err := gitrepo.OpenRepository(pr.BaseRepo)
+	gitRepo, err := git.OpenRepository(pr.BaseRepo)
 	assert.NoError(t, err)
 	defer gitRepo.Close()
 

services/pull/review.go

@@ -16,7 +16,6 @@ import (
 	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/git/gitcmd"
-	"gitea.dev/modules/gitrepo"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/optional"
 	"gitea.dev/modules/setting"
@@ -57,7 +56,7 @@ var ErrSubmitReviewOnClosedPR = errors.New("can't submit review for a closed or
 
 // LineBlame returns the latest commit at the given line
 func lineBlame(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, branch, file string, line uint) (*git.Commit, error) {
-	sha, err := gitrepo.LineBlame(ctx, repo, branch, file, line)
+	sha, err := git.LineBlame(ctx, repo, branch, file, line)
 	if err != nil {
 		return nil, err
 	}
@@ -215,7 +214,7 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
 	if err := pr.LoadBaseRepo(ctx); err != nil {
 		return nil, fmt.Errorf("LoadBaseRepo: %w", err)
 	}
-	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
+	gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, pr.BaseRepo)
 	if err != nil {
 		return nil, fmt.Errorf("RepositoryFromContextOrOpen: %w", err)
 	}

services/pull/temp_repo.go

@@ -16,7 +16,6 @@ import (
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/git/gitcmd"
-	"gitea.dev/modules/log"
 	repo_module "gitea.dev/modules/repository"
 )
 
@@ -31,6 +30,7 @@ const (
 type prTmpRepoContext struct {
 	context.Context
 	tmpBasePath string
+	tmpRepo     git.RepositoryFacade
 	pr          *issues_model.PullRequest
 	outbuf      *bytes.Buffer // we keep these around to help reduce needless buffer recreation, any use should be preceded by a Reset and preferably after use
 }
@@ -45,42 +45,37 @@ func (ctx *prTmpRepoContext) PrepareGitCmd(cmd *gitcmd.Command) *gitcmd.Command
 
 // createTemporaryRepoForPR creates a temporary repo with "base" for pr.BaseBranch and "tracking" for  pr.HeadBranch
 // it also create a second base branch called "original_base"
-func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) (prCtx *prTmpRepoContext, cancel context.CancelFunc, err error) {
+func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) (prCtx *prTmpRepoContext, cancel context.CancelFunc, retErr error) {
+	defer func() {
+		if retErr != nil && cancel != nil {
+			cancel()
+		}
+	}()
 	if err := pr.LoadHeadRepo(ctx); err != nil {
-		log.Error("%-v LoadHeadRepo: %v", pr, err)
-		return nil, nil, fmt.Errorf("%v LoadHeadRepo: %w", pr, err)
+		return nil, nil, fmt.Errorf("LoadHeadRepo[PR:%d]: %w", pr.ID, err)
 	} else if pr.HeadRepo == nil {
-		log.Error("%-v HeadRepo %d does not exist", pr, pr.HeadRepoID)
-		return nil, nil, &repo_model.ErrRepoNotExist{
-			ID: pr.HeadRepoID,
-		}
+		return nil, nil, &repo_model.ErrRepoNotExist{ID: pr.HeadRepoID}
 	} else if err := pr.LoadBaseRepo(ctx); err != nil {
-		log.Error("%-v LoadBaseRepo: %v", pr, err)
-		return nil, nil, fmt.Errorf("%v LoadBaseRepo: %w", pr, err)
+		return nil, nil, fmt.Errorf("LoadBaseRepo[PR:%d]: %w", pr.ID, err)
 	} else if pr.BaseRepo == nil {
-		log.Error("%-v BaseRepo %d does not exist", pr, pr.BaseRepoID)
-		return nil, nil, &repo_model.ErrRepoNotExist{
-			ID: pr.BaseRepoID,
-		}
+		return nil, nil, &repo_model.ErrRepoNotExist{ID: pr.BaseRepoID}
 	} else if err := pr.HeadRepo.LoadOwner(ctx); err != nil {
-		log.Error("%-v HeadRepo.LoadOwner: %v", pr, err)
-		return nil, nil, fmt.Errorf("%v HeadRepo.LoadOwner: %w", pr, err)
+		return nil, nil, fmt.Errorf("HeadRepo.LoadOwner[PR:%d]: %w", pr.ID, err)
 	} else if err := pr.BaseRepo.LoadOwner(ctx); err != nil {
-		log.Error("%-v BaseRepo.LoadOwner: %v", pr, err)
-		return nil, nil, fmt.Errorf("%v BaseRepo.LoadOwner: %w", pr, err)
+		return nil, nil, fmt.Errorf("BaseRepo.LoadOwner[PR:%d]: %w", pr.ID, err)
 	}
 
 	// Clone base repo.
-	tmpBasePath, cleanup, err := repo_module.CreateTemporaryPath("pull")
+	tmpBasePath, tmpRepo, cleanup, err := repo_module.CreateTemporaryGitRepo("pull")
 	if err != nil {
-		log.Error("CreateTemporaryPath[%-v]: %v", pr, err)
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("CreateTemporaryPath[PR:%d]: %w", pr.ID, err)
 	}
 	cancel = cleanup
 
 	prCtx = &prTmpRepoContext{
 		Context:     ctx,
 		tmpBasePath: tmpBasePath,
+		tmpRepo:     tmpRepo,
 		pr:          pr,
 		outbuf:      &bytes.Buffer{},
 	}
@@ -88,10 +83,8 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
 	baseRepoPath := pr.BaseRepo.RepoPath()
 	headRepoPath := pr.HeadRepo.RepoPath()
 
-	if err := git.InitRepository(ctx, tmpBasePath, false, pr.BaseRepo.ObjectFormatName); err != nil {
-		log.Error("Unable to init tmpBasePath for %-v: %v", pr, err)
-		cancel()
-		return nil, nil, err
+	if err := git.InitRepositoryLocal(ctx, tmpBasePath, false, pr.BaseRepo.ObjectFormatName); err != nil {
+		return nil, nil, fmt.Errorf("InitRepository[PR:%d]: %w", pr.ID, err)
 	}
 
 	remoteRepoName := "head_repo"
@@ -104,61 +97,49 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
 
 	// addCacheRepo adds git alternatives for the cacheRepoPath in the repoPath
 	addCacheRepo := func(repoPath, cacheRepoPath string) error {
-		p := filepath.Join(repoPath, ".git", "objects", "info", "alternates")
-		f, err := os.OpenFile(p, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600)
+		const altFileRelPath = ".git/objects/info/alternates"
+		gitInfoAltFile := filepath.Join(repoPath, filepath.FromSlash(altFileRelPath))
+		newAltLine := filepath.Join(cacheRepoPath, "objects")
+
+		f, err := os.OpenFile(gitInfoAltFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600)
 		if err != nil {
-			log.Error("Could not create .git/objects/info/alternates file in %s: %v", repoPath, err)
-			return err
+			return fmt.Errorf("unable to open %s file in %s: %w", altFileRelPath, filepath.Base(repoPath), err)
 		}
 		defer f.Close()
-		data := filepath.Join(cacheRepoPath, "objects")
-		if _, err := fmt.Fprintln(f, data); err != nil {
-			log.Error("Could not write to .git/objects/info/alternates file in %s: %v", repoPath, err)
-			return err
+		if _, err := fmt.Fprintln(f, newAltLine); err != nil {
+			return fmt.Errorf("unable to write %s file in %s: %w", altFileRelPath, filepath.Base(repoPath), err)
 		}
 		return nil
 	}
 
 	// Add head repo remote.
 	if err := addCacheRepo(tmpBasePath, baseRepoPath); err != nil {
-		log.Error("%-v Unable to add base repository to temporary repo [%s -> %s]: %v", pr, pr.BaseRepo.FullName(), tmpBasePath, err)
-		cancel()
-		return nil, nil, fmt.Errorf("Unable to add base repository to temporary repo [%s -> tmpBasePath]: %w", pr.BaseRepo.FullName(), err)
+		return nil, nil, fmt.Errorf("unable to add base repository to temporary repo [%s -> tmpBasePath]: %w", pr.BaseRepo.FullName(), err)
 	}
 
 	if err := prCtx.PrepareGitCmd(gitcmd.NewCommand("remote", "add", "-t").AddDynamicArguments(pr.BaseBranch).AddArguments("-m").AddDynamicArguments(pr.BaseBranch).AddDynamicArguments("origin", baseRepoPath)).
 		RunWithStderr(ctx); err != nil {
-		log.Error("%-v Unable to add base repository as origin [%s -> %s]: %v\n%s\n%s", pr, pr.BaseRepo.FullName(), tmpBasePath, err, prCtx.outbuf.String(), err.Stderr())
-		cancel()
-		return nil, nil, fmt.Errorf("Unable to add base repository as origin [%s -> tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), err, prCtx.outbuf.String(), err.Stderr())
+		return nil, nil, fmt.Errorf("unable to add base repository as origin [%s -> tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), err, prCtx.outbuf.String(), err.Stderr())
 	}
 
 	if err := prCtx.PrepareGitCmd(gitcmd.NewCommand("fetch", "origin").AddArguments(fetchArgs...).
 		AddDashesAndList(git.BranchPrefix+pr.BaseBranch+":"+git.BranchPrefix+tmpRepoBaseBranch, git.BranchPrefix+pr.BaseBranch+":"+git.BranchPrefix+"original_"+tmpRepoBaseBranch)).
 		RunWithStderr(ctx); err != nil {
-		log.Error("%-v Unable to fetch origin base branch [%s:%s -> base, original_base in %s]: %v:\n%s\n%s", pr, pr.BaseRepo.FullName(), pr.BaseBranch, tmpBasePath, err, prCtx.outbuf.String(), err.Stderr())
-		cancel()
-		return nil, nil, fmt.Errorf("Unable to fetch origin base branch [%s:%s -> base, original_base in tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, err, prCtx.outbuf.String(), err.Stderr())
+		return nil, nil, fmt.Errorf("unable to fetch origin base branch [%s:%s -> base, original_base in tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, err, prCtx.outbuf.String(), err.Stderr())
 	}
 
 	if err := prCtx.PrepareGitCmd(gitcmd.NewCommand("symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+tmpRepoBaseBranch)).
 		RunWithStderr(ctx); err != nil {
-		log.Error("%-v Unable to set HEAD as base branch in [%s]: %v\n%s\n%s", pr, tmpBasePath, err, prCtx.outbuf.String(), err.Stderr())
-		cancel()
-		return nil, nil, fmt.Errorf("Unable to set HEAD as base branch in tmpBasePath: %w\n%s\n%s", err, prCtx.outbuf.String(), err.Stderr())
+		return nil, nil, fmt.Errorf("unable to set HEAD as base branch in tmpBasePath: %w\n%s\n%s", err, prCtx.outbuf.String(), err.Stderr())
 	}
 
 	if err := addCacheRepo(tmpBasePath, headRepoPath); err != nil {
-		log.Error("%-v Unable to add head repository to temporary repo [%s -> %s]: %v", pr, pr.HeadRepo.FullName(), tmpBasePath, err)
-		cancel()
-		return nil, nil, fmt.Errorf("Unable to add head base repository to temporary repo [%s -> tmpBasePath]: %w", pr.HeadRepo.FullName(), err)
+		return nil, nil, fmt.Errorf("unable to add head base repository to temporary repo [%s -> tmpBasePath]: %w", pr.HeadRepo.FullName(), err)
 	}
 
 	if err := prCtx.PrepareGitCmd(gitcmd.NewCommand("remote", "add").AddDynamicArguments(remoteRepoName, headRepoPath)).
 		RunWithStderr(ctx); err != nil {
-		log.Error("%-v Unable to add head repository as head_repo [%s -> %s]: %v\n%s\n%s", pr, pr.HeadRepo.FullName(), tmpBasePath, err, prCtx.outbuf.String(), err.Stderr())
-		cancel()
-		return nil, nil, fmt.Errorf("Unable to add head repository as head_repo [%s -> tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), err, prCtx.outbuf.String(), err.Stderr())
+		return nil, nil, fmt.Errorf("unable to add head repository as head_repo [%s -> tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), err, prCtx.outbuf.String(), err.Stderr())
 	}
 
 	objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName)
@@ -173,14 +154,10 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
 	}
 	if err := prCtx.PrepareGitCmd(gitcmd.NewCommand("fetch").AddArguments(fetchArgs...).AddDynamicArguments(remoteRepoName, headBranch+":"+tmpRepoTrackingBranch)).
 		RunWithStderr(ctx); err != nil {
-		cancel()
 		if exist, _ := git_model.IsBranchExist(ctx, pr.HeadRepo.ID, pr.HeadBranch); !exist {
-			return nil, nil, git_model.ErrBranchNotExist{
-				BranchName: pr.HeadBranch,
-			}
+			return nil, nil, git_model.ErrBranchNotExist{BranchName: pr.HeadBranch}
 		}
-		log.Error("%-v Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr, pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, prCtx.outbuf.String(), err.Stderr())
-		return nil, nil, fmt.Errorf("Unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), headBranch, err, prCtx.outbuf.String(), err.Stderr())
+		return nil, nil, fmt.Errorf("unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), headBranch, err, prCtx.outbuf.String(), err.Stderr())
 	}
 	prCtx.outbuf.Reset()
 	return prCtx, cancel, nil

services/pull/update.go

@@ -14,7 +14,7 @@ import (
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/models/unit"
 	user_model "gitea.dev/models/user"
-	"gitea.dev/modules/gitrepo"
+	"gitea.dev/modules/git"
 	"gitea.dev/modules/globallock"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/repository"
@@ -41,7 +41,7 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
 
 	// TODO: FakePR: if the PR is a fake PR (for example: from Merge Upstream), then no need to check diverging
 	if pr.ID > 0 {
-		diffCount, err := gitrepo.GetDivergingCommits(ctx, pr.BaseRepo, pr.BaseBranch, pr.GetGitHeadRefName())
+		diffCount, err := git.GetDivergingCommits(ctx, pr.BaseRepo, pr.BaseBranch, pr.GetGitHeadRefName())
 		if err != nil {
 			return err
 		} else if diffCount.Behind == 0 {
@@ -213,7 +213,7 @@ func syncCommitDivergence(ctx context.Context, pr *issues_model.PullRequest) err
 	if err := pr.LoadBaseRepo(ctx); err != nil {
 		return err
 	}
-	divergence, err := gitrepo.GetDivergingCommits(ctx, pr.BaseRepo, pr.BaseBranch, pr.GetGitHeadRefName())
+	divergence, err := git.GetDivergingCommits(ctx, pr.BaseRepo, pr.BaseBranch, pr.GetGitHeadRefName())
 	if err != nil {
 		return err
 	}

services/pull/update_rebase.go

@@ -42,7 +42,7 @@ func updateHeadByRebaseOnToBase(ctx context.Context, pr *issues_model.PullReques
 		// It's questionable about where this should go - either after or before the push
 		// I think in the interests of data safety - failures to push to the lfs should prevent
 		// the push as you can always re-rebase.
-		if err := LFSPush(ctx, mergeCtx.tmpBasePath, tmpRepoBaseBranch, oldMergeBase, &issues_model.PullRequest{
+		if err := LFSPush(ctx, mergeCtx.tmpBasePath, mergeCtx.tmpRepo, tmpRepoBaseBranch, oldMergeBase, &issues_model.PullRequest{
 			HeadRepoID: pr.BaseRepoID,
 			BaseRepoID: pr.HeadRepoID,
 		}); err != nil {