refactor: remove Path field from git.Repository (#38552)

The "path" details should be hidden to other packages

By the way, fix a resource leaking in gogit's CommitNodeIndex
(the file was not closed in CacheCommit)

models/repo/repo_location_test.go

@@ -14,7 +14,6 @@ import (
 
 func TestRepository_GitRepo(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
-
 	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 
 	assert.Equal(t, "user2/repo1.git", repo_model.CodeRepoByName(repo.OwnerName, repo.Name).GitRepoLocation())

modules/git/attribute/batch.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"path/filepath"
 	"time"
 
 	"gitea.dev/modules/git"
@@ -90,7 +89,7 @@ func NewBatchChecker(ctx context.Context, repo *git.Repository, treeish string,
 func (c *BatchChecker) CheckPath(path string) (rs *Attributes, err error) {
 	defer func() {
 		if err != nil && err != c.ctx.Err() {
-			log.Error("Unexpected error when checking path %s in %s, error: %v", path, filepath.Base(c.repo.Path), err)
+			log.Error("Unexpected error when checking path %s in %s, error: %v", path, c.repo.LogString(), err)
 		}
 	}()
 
@@ -112,7 +111,7 @@ func (c *BatchChecker) CheckPath(path string) (rs *Attributes, err error) {
 			stdOutClosed = true
 		default:
 		}
-		debugMsg := fmt.Sprintf("check path %q in repo %q", path, filepath.Base(c.repo.Path))
+		debugMsg := fmt.Sprintf("check path %q in repo %q", path, c.repo.LogString())
 		debugMsg += fmt.Sprintf(", stdOut: tmp=%q, pos=%d, closed=%v", string(c.stdOut.tmp), c.stdOut.pos, stdOutClosed)
 		if c.cmd != nil {
 			debugMsg += fmt.Sprintf(", process state: %q", c.cmd.ProcessState())

modules/git/blob_nogogit.go

@@ -58,13 +58,13 @@ func (b *Blob) Size(ctx context.Context) int64 {
 
 	batch, cancel, err := b.repo.CatFileBatch(ctx)
 	if err != nil {
-		log.Debug("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.Path, err)
+		log.Debug("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.LogString(), err)
 		return 0
 	}
 	defer cancel()
 	info, err := batch.QueryInfo(b.ID.String())
 	if err != nil {
-		log.Debug("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.Path, err)
+		log.Debug("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.LogString(), err)
 		return 0
 	}
 	b.gotSize = true

modules/git/catfile_batch_test.go

@@ -25,7 +25,8 @@ func TestCatFileBatch(t *testing.T) {
 }
 
 func testCatFileBatch(t *testing.T) {
-	repo1 := gitcmd.RepositoryUnmanaged(filepath.Join(testReposDir, "repo1_bare"))
+	repo1Path, _ := filepath.Abs(filepath.Join(testReposDir, "repo1_bare"))
+	repo1 := gitcmd.RepositoryUnmanaged(repo1Path)
 	t.Run("CorruptedGitRepo", func(t *testing.T) {
 		tmpDir := t.TempDir()
 		batch, err := NewBatch(t.Context(), gitcmd.RepositoryUnmanaged(tmpDir))

modules/git/commit_info_gogit.go

@@ -153,10 +153,8 @@ func getLastCommitForPathsByCache(ctx context.Context, commitID, treePath string
 
 // GetLastCommitForPaths returns last commit information
 func GetLastCommitForPaths(ctx context.Context, gitRepo *Repository, commit *Commit, treePath string, paths []string) (map[string]*Commit, error) {
-	commitNodeIndex, commitGraphFile := gitRepo.CommitNodeIndex()
+	commitNodeIndex, closer := gitRepo.CommitNodeIndex()
-	if commitGraphFile != nil {
+	defer closer()
-		defer commitGraphFile.Close()
-	}
 
 	c, err := commitNodeIndex.Get(plumbing.Hash(commit.ID.RawValue()))
 	if err != nil {

modules/git/diff.go

@@ -325,7 +325,7 @@ func GetAffectedFiles(ctx context.Context, repo *Repository, branchName, oldComm
 		}).
 		Run(ctx)
 	if err != nil {
-		log.Error("Unable to get affected files for commits from %s to %s in %s: %v", oldCommitID, newCommitID, repo.Path, err)
+		log.Error("Unable to get affected files for commits from %s to %s in %s: %v", oldCommitID, newCommitID, repo.LogString(), err)
 	}
 
 	return affectedFiles, err

modules/git/gitcmd/command.go

@@ -205,6 +205,8 @@ func ToTrustedCmdArgs(args []string) TrustedCmdArgs {
 }
 
 type runOpts struct {
+	// TODO: this struct should be removed, the fields can be just merged into the command
+
 	Env     []string
 	Timeout time.Duration
 
@@ -213,7 +215,7 @@ type runOpts struct {
 	// * /some/path/.git
 	// * /some/path/.git/gitea-data/data/repositories/user/repo.git
 	// If "user/repo.git" is invalid/broken, then running git command in it will use "/some/path/.git", and produce unexpected results
-	// The correct approach is to use `--git-dir" global argument
+	// The correct approach is to use `--git-dir" global argument or "GIT_DIR=..." environment variable.
 	Dir string
 
 	PipelineFunc func(Context) error

modules/git/gitcmd/repo.go

@@ -5,6 +5,7 @@ package gitcmd
 
 import (
 	"path/filepath"
+	"sync/atomic"
 
 	"gitea.dev/modules/setting"
 )
@@ -19,6 +20,8 @@ type RepositoryFacade interface {
 	// * absolute path: will be used as-is
 	// * in the future: maybe URI for more flexible definitions
 	GitRepoLocation() string
+
+	LogString() string
 }
 
 func (c *Command) WithRepo(repo RepositoryFacade) *Command {
@@ -26,31 +29,69 @@ func (c *Command) WithRepo(repo RepositoryFacade) *Command {
 	return c
 }
 
+// RepoLocalPath returns an absolute path for a RepositoryFacade.
+// TODO: most of the calls to this function should be replaced with a "Repo FS" in the future
+// to handle file accesses in the git repo (e.g.: read, write, list, remove).
 func RepoLocalPath(repo RepositoryFacade) string {
 	repoLoc := repo.GitRepoLocation()
 	if filepath.IsAbs(repoLoc) {
 		return repoLoc
 	}
-	return filepath.Join(setting.RepoRootPath, filepath.FromSlash(repoLoc))
+	if setting.RepoRootPath == "" {
+		panic("repo root path is not initialized")
+	}
+	// the repo root path and the repo loc should all have been cleaned, so we can safely join them together
+	return setting.RepoRootPath + string(filepath.Separator) + filepath.FromSlash(repoLoc)
 }
 
-type repositoryUnmanaged string
+func repoLogNameByLocation(loc string) string {
+	t := filepath.FromSlash(loc)
+	// hide the parent paths, then the name should be safe for end users
+	return ".../" + filepath.Base(filepath.Dir(t)) + "/" + filepath.Base(t)
+}
 
-func (r repositoryUnmanaged) GitRepoManagedID() string {
+type repositoryUnmanaged struct {
+	loc     string
+	logName atomic.Pointer[string]
+}
+
+func (r *repositoryUnmanaged) LogString() string {
+	s := r.logName.Load()
+	if s == nil {
+		s = new(repoLogNameByLocation(r.loc))
+		r.logName.Store(s)
+	}
+	return *s
+}
+
+func (r *repositoryUnmanaged) GitRepoManagedID() string {
 	panic("this repo is not managed by Gitea, can't be used in this managed context")
 }
 
-func (r repositoryUnmanaged) GitRepoLocation() string {
+func (r *repositoryUnmanaged) GitRepoLocation() string {
-	return string(r)
+	return r.loc
 }
 
+// RepositoryUnmanaged returns a RepositoryFacade for a repository that might not be managed by Gitea.
+// If the path is not absolute, then it is relative to setting.RepoRootPath
+// This function is mainly for maintaining the owner's repo when the repo is not managed yet.
+// e.g.: init, clone, transfer, rename, adopt, etc., and temp repo creation and modification.
 func RepositoryUnmanaged(s string) RepositoryFacade {
-	return repositoryUnmanaged(s)
+	return &repositoryUnmanaged{loc: filepath.Clean(s)}
 }
 
 type repositoryManaged struct {
-	id  string
+	id, loc string
-	loc string
+	logName atomic.Pointer[string]
+}
+
+func (r *repositoryManaged) LogString() string {
+	s := r.logName.Load()
+	if s == nil {
+		s = new(repoLogNameByLocation(r.loc))
+		r.logName.Store(s)
+	}
+	return *s
 }
 
 func (r *repositoryManaged) GitRepoManagedID() string {
@@ -62,5 +103,5 @@ func (r *repositoryManaged) GitRepoLocation() string {
 }
 
 func RepositoryManaged(id, loc string) RepositoryFacade {
-	return &repositoryManaged{id, loc}
+	return &repositoryManaged{id: id, loc: filepath.Clean(loc)}
 }

modules/git/grep_test.go

@@ -8,11 +8,14 @@ import (
 	"testing"
 
 	"gitea.dev/modules/git/gitcmd"
+	"gitea.dev/modules/setting"
+	"gitea.dev/modules/test"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestGrepSearch(t *testing.T) {
+	defer test.MockVariableValue(&setting.RepoRootPath, t.TempDir())()
 	repo, err := OpenRepositoryLocal(filepath.Join(testReposDir, "language_stats_repo"))
 	assert.NoError(t, err)
 	defer repo.Close()
@@ -76,7 +79,7 @@ func TestGrepSearch(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Empty(t, res)
 
-	nonExistingRepo := &Repository{RepositoryBase: RepositoryBase{Path: "no-such-git-repo", repoFacade: gitcmd.RepositoryUnmanaged("no-such-git-repo")}}
+	nonExistingRepo := &Repository{RepositoryBase: RepositoryBase{repoFacade: gitcmd.RepositoryUnmanaged("no-such-git-repo")}}
 	res, err = GrepSearch(t.Context(), nonExistingRepo, "no-such-content", GrepOptions{})
 	assert.Error(t, err)
 	assert.Empty(t, res)

modules/git/hook.go

@@ -11,6 +11,7 @@ import (
 	"slices"
 	"strings"
 
+	"gitea.dev/modules/git/gitcmd"
 	"gitea.dev/modules/util"
 )
 
@@ -39,7 +40,8 @@ type Hook struct {
 }
 
 // GetHook returns a Git hook by given name and repository.
-func GetHook(repoPath, name string) (*Hook, error) {
+func GetHook(repo RepositoryFacade, name string) (*Hook, error) {
+	repoPath := gitcmd.RepoLocalPath(repo)
 	if !IsValidHookName(name) {
 		return nil, ErrNotValidHook
 	}
@@ -97,8 +99,8 @@ func (h *Hook) Update() error {
 }
 
 // ListHooks returns a list of Git hooks of given repository.
-func ListHooks(repoPath string) (_ []*Hook, err error) {
+func ListHooks(repo RepositoryFacade) (_ []*Hook, err error) {
-	exist, err := util.IsDir(filepath.Join(repoPath, "hooks"))
+	exist, err := util.IsDir(filepath.Join(gitcmd.RepoLocalPath(repo), "hooks"))
 	if err != nil {
 		return nil, err
 	} else if !exist {
@@ -107,7 +109,7 @@ func ListHooks(repoPath string) (_ []*Hook, err error) {
 
 	hooks := make([]*Hook, len(hookNames))
 	for i, name := range hookNames {
-		hooks[i], err = GetHook(repoPath, name)
+		hooks[i], err = GetHook(repo, name)
 		if err != nil {
 			return nil, err
 		}

modules/git/last_commit_cache_gogit.go

@@ -17,7 +17,8 @@ func (c *Commit) CacheCommit(ctx context.Context, gitRepo *Repository) error {
 	if gitRepo.LastCommitCache == nil {
 		return nil
 	}
-	commitNodeIndex, _ := gitRepo.CommitNodeIndex()
+	commitNodeIndex, closer := gitRepo.CommitNodeIndex()
+	defer closer()
 
 	index, err := commitNodeIndex.Get(plumbing.Hash(c.ID.RawValue()))
 	if err != nil {

modules/git/log_name_status_nogogit.go

@@ -21,7 +21,7 @@ import (
 )
 
 // logNameStatusRepo opens git log --raw in the provided repo and returns a parser
-func logNameStatusRepo(ctx context.Context, repository, head, treepath string, paths ...string) *logNameStatusRepoParser {
+func logNameStatusRepo(ctx context.Context, repo RepositoryFacade, head, treepath string, paths ...string) *logNameStatusRepoParser {
 	cmd := gitcmd.NewCommand()
 	cmd.AddArguments("log", "--name-status", "-c", "--format=commit%x00%H %P%x00", "--parents", "--no-renames", "-t", "-z").AddDynamicArguments(head)
 
@@ -53,7 +53,7 @@ func logNameStatusRepo(ctx context.Context, repository, head, treepath string, p
 	stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe()
 	ctx, ctxCancel := context.WithCancel(ctx)
 	go func() {
-		err := cmd.WithDir(repository).RunWithStderr(ctx)
+		err := cmd.WithRepo(repo).RunWithStderr(ctx)
 		if err != nil && !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) {
 			log.Error("Unable to run git command %v: %v", cmd.LogString(), err)
 		}
@@ -303,7 +303,7 @@ func walkGitLog(ctx context.Context, repo *Repository, head *Commit, treepath st
 		}
 	}
 
-	g := logNameStatusRepo(ctx, repo.Path, head.ID.String(), treepath, paths...)
+	g := logNameStatusRepo(ctx, repo, head.ID.String(), treepath, paths...)
 	// don't use defer g.cancel() here as g may change its value - instead wrap in a func
 	defer func() { g.close() }()
 
@@ -372,7 +372,7 @@ heaploop:
 					}
 				}
 				g.close()
-				g = logNameStatusRepo(ctx, repo.Path, lastEmptyParent, treepath, remainingPaths...)
+				g = logNameStatusRepo(ctx, repo, lastEmptyParent, treepath, remainingPaths...)
 				parentRemaining = make(container.Set[string])
 				nextRestart = (remaining * 3) / 4
 				continue heaploop

modules/git/notes.go

@@ -24,7 +24,7 @@ type Note struct {
 // GetNote retrieves the git-notes data for a given commit.
 // FIXME: Add LastCommitCache support
 func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note) error {
-	log.Trace("Searching for git note corresponding to the commit %q in the repository %q", commitID, repo.Path)
+	log.Trace("Searching for git note corresponding to the commit %q in the repository %q", commitID, repo.LogString())
 	notes, err := repo.GetCommit(ctx, NotesRef)
 	if err != nil {
 		if IsErrNotExist(err) {

modules/git/repo.go

@@ -25,11 +25,6 @@ import (
 type RepositoryFacade = gitcmd.RepositoryFacade
 
 type RepositoryBase struct {
-	// TODO: refactor it to a private field "localPath" in the future
-	// * for repo accessing purpose, in most causes, use "WithRepo", or RepoLocalPath(repo) if the local path must be used
-	// * for error handling & logging purpose, it needs to introduce a new function "git.RepoLogName()" to handle various cases
-	Path string
-
 	LastCommitCache *LastCommitCache
 
 	repoFacade        RepositoryFacade
@@ -51,6 +46,10 @@ func (repo *Repository) GitRepoLocation() string {
 	return repo.repoFacade.GitRepoLocation()
 }
 
+func (repo *Repository) LogString() string {
+	return repo.repoFacade.LogString()
+}
+
 func OpenRepository(repo RepositoryFacade) (*Repository, error) {
 	repoPath := gitcmd.RepoLocalPath(repo)
 	exist, err := util.IsDir(repoPath)
@@ -61,7 +60,7 @@ func OpenRepository(repo RepositoryFacade) (*Repository, error) {
 		return nil, util.NewNotExistErrorf("no such file or directory")
 	}
 	gitRepo := &Repository{
-		RepositoryBase: RepositoryBase{Path: repoPath, tagCache: newObjectCache[*Tag](), repoFacade: repo},
+		RepositoryBase: RepositoryBase{tagCache: newObjectCache[*Tag](), repoFacade: repo},
 	}
 	if err = openRepositoryInternal(gitRepo); err != nil {
 		return nil, err
@@ -69,6 +68,8 @@ func OpenRepository(repo RepositoryFacade) (*Repository, error) {
 	return gitRepo, nil
 }
 
+// OpenRepositoryLocal opens a local repository that is not managed by Gitea
+// If the path is relative, it will be converted to an absolute path using filepath.Abs (base on current working path)
 func OpenRepositoryLocal(localPath string) (_ *Repository, err error) {
 	if !filepath.IsAbs(localPath) {
 		localPath, err = filepath.Abs(localPath)
@@ -129,7 +130,7 @@ func InitRepositoryLocal(ctx context.Context, repoPath string, bare bool, object
 // IsEmpty Check if repository is empty.
 func (repo *Repository) IsEmpty(ctx context.Context) (bool, error) {
 	stdout, _, err := gitcmd.NewCommand().
-		AddOptionFormat("--git-dir=%s", repo.Path).
+		AddOptionFormat("--git-dir=%s", gitcmd.RepoLocalPath(repo)). // TODO: all git commands should use "--git-dir" or "GIT_DIR=..."
 		AddArguments("rev-list", "-n", "1", "--all").
 		WithRepo(repo).
 		RunStdString(ctx)

modules/git/repo_base_gogit.go

@@ -9,6 +9,7 @@ package git
 import (
 	"path/filepath"
 
+	"gitea.dev/modules/git/gitcmd"
 	"gitea.dev/modules/setting"
 
 	"github.com/go-git/go-billy/v5"
@@ -29,7 +30,8 @@ type Repository struct {
 }
 
 func openRepositoryInternal(gitRepo *Repository) error {
-	fs := osfs.New(gitRepo.Path)
+	repoPath := gitcmd.RepoLocalPath(gitRepo)
+	fs := osfs.New(repoPath)
 	_, err := fs.Stat(".git")
 	if err == nil {
 		fs, err = fs.Chroot(".git")

modules/git/repo_branch_nogogit.go

@@ -65,7 +65,7 @@ func (repo *Repository) IsBranchExist(ctx context.Context, name string) bool {
 // GetBranchNames returns branches from the repository, skipping "skip" initial branches and
 // returning at most "limit" branches, or all branches if "limit" is 0.
 func (repo *Repository) GetBranchNames(ctx context.Context, skip, limit int) ([]string, int, error) {
-	return callShowRef(ctx, repo.Path, BranchPrefix, gitcmd.TrustedCmdArgs{BranchPrefix, "--sort=-committerdate"}, skip, limit)
+	return callShowRef(ctx, repo, BranchPrefix, gitcmd.TrustedCmdArgs{BranchPrefix, "--sort=-committerdate"}, skip, limit)
 }
 
 // WalkReferences walks all the references from the repository
@@ -79,12 +79,12 @@ func (repo *Repository) WalkReferences(ctx context.Context, refType ObjectType,
 		args = gitcmd.TrustedCmdArgs{BranchPrefix, "--sort=-committerdate"}
 	}
 
-	return WalkShowRef(ctx, repo.Path, args, skip, limit, walkfn)
+	return WalkShowRef(ctx, repo, args, skip, limit, walkfn)
 }
 
 // callShowRef return refs, if limit = 0 it will not limit
-func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs gitcmd.TrustedCmdArgs, skip, limit int) (branchNames []string, countAll int, err error) {
+func callShowRef(ctx context.Context, repo RepositoryFacade, trimPrefix string, extraArgs gitcmd.TrustedCmdArgs, skip, limit int) (branchNames []string, countAll int, err error) {
-	countAll, err = WalkShowRef(ctx, repoPath, extraArgs, skip, limit, func(_, branchName string) error {
+	countAll, err = WalkShowRef(ctx, repo, extraArgs, skip, limit, func(_, branchName string) error {
 		branchName = strings.TrimPrefix(branchName, trimPrefix)
 		branchNames = append(branchNames, branchName)
 
@@ -93,14 +93,14 @@ func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs git
 	return branchNames, countAll, err
 }
 
-func WalkShowRef(ctx context.Context, repoPath string, extraArgs gitcmd.TrustedCmdArgs, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) {
+func WalkShowRef(ctx context.Context, repo RepositoryFacade, extraArgs gitcmd.TrustedCmdArgs, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) {
 	i := 0
 	args := gitcmd.TrustedCmdArgs{"for-each-ref", "--format=%(objectname) %(refname)"}
 	args = append(args, extraArgs...)
 	cmd := gitcmd.NewCommand(args...)
 	stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe()
 	defer stdoutReaderClose()
-	cmd.WithDir(repoPath).
+	cmd.WithRepo(repo).
 		WithPipelineFunc(func(gitcmd.Context) error {
 			bufReader := bufio.NewReader(stdoutReader)
 			for i < skip {
@@ -174,7 +174,7 @@ func WalkShowRef(ctx context.Context, repoPath string, extraArgs gitcmd.TrustedC
 // GetRefsBySha returns all references filtered with prefix that belong to a sha commit hash
 func (repo *Repository) GetRefsBySha(ctx context.Context, sha, prefix string) ([]string, error) {
 	var revList []string
-	_, err := WalkShowRef(ctx, repo.Path, nil, 0, 0, func(walkSha, refname string) error {
+	_, err := WalkShowRef(ctx, repo, nil, 0, 0, func(walkSha, refname string) error {
 		if walkSha == sha && strings.HasPrefix(refname, prefix) {
 			revList = append(revList, refname)
 		}

modules/git/repo_commitgraph_gogit.go

@@ -10,28 +10,29 @@ import (
 	"os"
 	"path/filepath"
 
-	gitealog "gitea.dev/modules/log"
+	"gitea.dev/modules/git/gitcmd"
+	"gitea.dev/modules/log"
 
 	commitgraph "github.com/go-git/go-git/v5/plumbing/format/commitgraph/v2"
 	cgobject "github.com/go-git/go-git/v5/plumbing/object/commitgraph"
 )
 
 // CommitNodeIndex returns the index for walking commit graph
-func (repo *Repository) CommitNodeIndex() (cgobject.CommitNodeIndex, *os.File) {
+func (repo *Repository) CommitNodeIndex() (_ cgobject.CommitNodeIndex, closer func()) {
-	indexPath := filepath.Join(repo.Path, "objects", "info", "commit-graph")
+	indexPath := filepath.Join(gitcmd.RepoLocalPath(repo), "objects", "info", "commit-graph")
-
 	file, err := os.Open(indexPath)
 	if err == nil {
 		var index commitgraph.Index
 		index, err = commitgraph.OpenFileIndex(file)
 		if err == nil {
-			return cgobject.NewGraphCommitNodeIndex(index, repo.gogitRepo.Storer), file
+			return cgobject.NewGraphCommitNodeIndex(index, repo.gogitRepo.Storer), func() { _ = file.Close() }
 		}
+		_ = file.Close()
 	}
 
 	if !os.IsNotExist(err) {
-		gitealog.Warn("Unable to read commit-graph for %s: %v", repo.Path, err)
+		log.Warn("Unable to read commit-graph for %s: %v", repo.LogString(), err)
 	}
 
-	return cgobject.NewObjectCommitNodeIndex(repo.gogitRepo.Storer), nil
+	return cgobject.NewObjectCommitNodeIndex(repo.gogitRepo.Storer), func() {}
 }

modules/git/repo_hook.go

@@ -1,14 +0,0 @@
-// Copyright 2015 The Gogs Authors. All rights reserved.
-// SPDX-License-Identifier: MIT
-
-package git
-
-// GetHook get one hook according the name on a repository
-func (repo *Repository) GetHook(name string) (*Hook, error) {
-	return GetHook(repo.Path, name)
-}
-
-// Hooks get all the hooks on the repository
-func (repo *Repository) Hooks() ([]*Hook, error) {
-	return ListHooks(repo.Path)
-}

modules/git/repo_ref_gogit.go

@@ -9,16 +9,12 @@ import (
 	"context"
 	"strings"
 
-	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
 )
 
 // GetRefsFiltered returns all references of the repository that matches patterm exactly or starting with.
 func (repo *Repository) GetRefsFiltered(ctx context.Context, pattern string) ([]*Reference, error) {
-	r, err := git.PlainOpen(repo.Path)
+	r := repo.gogitRepo
-	if err != nil {
-		return nil, err
-	}
 
 	refsIter, err := r.References()
 	if err != nil {

modules/git/tree_entry_nogogit.go

@@ -20,13 +20,13 @@ func (te *TreeEntry) GetSize(ctx context.Context, gitRepo *Repository) int64 {
 
 	batch, cancel, err := gitRepo.CatFileBatch(ctx)
 	if err != nil {
-		log.Debug("error whilst reading size for %s in %s. Error: %v", te.ID.String(), gitRepo.Path, err)
+		log.Debug("error whilst reading size for %s in %s. Error: %v", te.ID.String(), gitRepo.LogString(), err)
 		return 0
 	}
 	defer cancel()
 	info, err := batch.QueryInfo(te.ID.String())
 	if err != nil {
-		log.Debug("error whilst reading size for %s in %s. Error: %v", te.ID.String(), gitRepo.Path, err)
+		log.Debug("error whilst reading size for %s in %s. Error: %v", te.ID.String(), gitRepo.LogString(), err)
 		return 0
 	}
 

modules/templates/util_render_test.go

@@ -68,6 +68,7 @@ func newTestRenderUtils(t *testing.T) *RenderUtils {
 }
 
 func TestRenderRepoComment(t *testing.T) {
+	defer test.MockVariableValue(&setting.RepoRootPath, t.TempDir())()
 	mockRepo := &repo.Repository{
 		ID: 1, OwnerName: "user13", Name: "repo11",
 		Owner: &user_model.User{ID: 13, Name: "user13"},

routers/api/v1/repo/git_hook.go

@@ -38,7 +38,7 @@ func ListGitHooks(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	hooks, err := ctx.Repo.GitRepo.Hooks()
+	hooks, err := git.ListHooks(ctx.Repo.GitRepo)
 	if err != nil {
 		ctx.APIErrorInternal(err)
 		return
@@ -81,7 +81,7 @@ func GetGitHook(ctx *context.APIContext) {
 	//     "$ref": "#/responses/notFound"
 
 	hookID := ctx.PathParam("id")
-	hook, err := ctx.Repo.GitRepo.GetHook(hookID)
+	hook, err := git.GetHook(ctx.Repo.GitRepo, hookID)
 	if err != nil {
 		if errors.Is(err, git.ErrNotValidHook) {
 			ctx.APIErrorNotFound()
@@ -128,7 +128,7 @@ func EditGitHook(ctx *context.APIContext) {
 
 	form := web.GetForm(ctx).(*api.EditGitHookOption)
 	hookID := ctx.PathParam("id")
-	hook, err := ctx.Repo.GitRepo.GetHook(hookID)
+	hook, err := git.GetHook(ctx.Repo.GitRepo, hookID)
 	if err != nil {
 		if errors.Is(err, git.ErrNotValidHook) {
 			ctx.APIErrorNotFound()
@@ -177,7 +177,7 @@ func DeleteGitHook(ctx *context.APIContext) {
 	//     "$ref": "#/responses/notFound"
 
 	hookID := ctx.PathParam("id")
-	hook, err := ctx.Repo.GitRepo.GetHook(hookID)
+	hook, err := git.GetHook(ctx.Repo.GitRepo, hookID)
 	if err != nil {
 		if errors.Is(err, git.ErrNotValidHook) {
 			ctx.APIErrorNotFound()

routers/private/hook_verification.go

@@ -39,7 +39,7 @@ func verifyCommits(ctx context.Context, oldCommitID, newCommitID string, repo *g
 		}).
 		Run(ctx)
 	if err != nil && !isErrUnverifiedCommit(err) {
-		log.Error("Unable to check commits from %s to %s in %s: %v", oldCommitID, newCommitID, repo.Path, err)
+		log.Error("Unable to check commits from %s to %s in %s: %v", oldCommitID, newCommitID, repo.LogString(), err)
 	}
 	return err
 }

routers/web/repo/setting/git_hooks.go

@@ -16,7 +16,7 @@ func GitHooks(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("repo.settings.githooks")
 	ctx.Data["PageIsSettingsGitHooks"] = true
 
-	hooks, err := ctx.Repo.GitRepo.Hooks()
+	hooks, err := git.ListHooks(ctx.Repo.GitRepo)
 	if err != nil {
 		ctx.ServerError("Hooks", err)
 		return
@@ -32,7 +32,7 @@ func GitHooksEdit(ctx *context.Context) {
 	ctx.Data["PageIsSettingsGitHooks"] = true
 
 	name := ctx.PathParam("name")
-	hook, err := ctx.Repo.GitRepo.GetHook(name)
+	hook, err := git.GetHook(ctx.Repo.GitRepo, name)
 	if err != nil {
 		if err == git.ErrNotValidHook {
 			ctx.NotFound(err)
@@ -49,7 +49,7 @@ func GitHooksEdit(ctx *context.Context) {
 // GitHooksEditPost response for editing a git hook of a repository
 func GitHooksEditPost(ctx *context.Context) {
 	name := ctx.PathParam("name")
-	hook, err := ctx.Repo.GitRepo.GetHook(name)
+	hook, err := git.GetHook(ctx.Repo.GitRepo, name)
 	if err != nil {
 		if err == git.ErrNotValidHook {
 			ctx.NotFound(err)

services/gitdiff/gitdiff.go

@@ -1359,7 +1359,7 @@ func getDiffBasic(ctx context.Context, gitRepo *git.Repository, opts *DiffOption
 		if err := cmdDiff.
 			WithRepo(gitRepo).
 			RunWithStderr(cmdCtx); err != nil && !gitcmd.IsErrorCanceledOrKilled(err) {
-			log.Error("error during GetDiff(git diff dir: %s): %v", gitRepo.Path, err)
+			log.Error("error during GetDiff(git diff dir: %s): %v", gitRepo.LogString(), err)
 		}
 	}()
 
@@ -1530,7 +1530,7 @@ func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model.
 	// For SOME of the errors such as the gc'ed commit, it would be best to mark all files as changed
 	// But as that does not work for all potential errors, we simply mark all files as unchanged and drop the error which always works, even if not as good as possible
 	if errIgnored != nil {
-		log.Error("Could not get changed files between %s and %s for pull request %d in repo with path %s. Assuming no changes. Error: %w", review.CommitSHA, latestCommit, pull.Index, gitRepo.Path, err)
+		log.Error("Could not get changed files between %s and %s for pull request %d in repo with path %s. Assuming no changes. Error: %w", review.CommitSHA, latestCommit, pull.Index, gitRepo.LogString(), err)
 	}
 	changedFilesSet := make(map[string]struct{}, len(changedFiles))
 	for _, changedFile := range changedFiles {

services/migrations/dump.go

@@ -136,8 +136,11 @@ func (g *RepositoryDumper) CreateRepo(ctx context.Context, repo *base.Repository
 		return err
 	}
 
-	repoPath := g.gitPath()
+	repoAbsPath, err := filepath.Abs(g.gitPath())
-	if err := os.MkdirAll(repoPath, os.ModePerm); err != nil {
+	if err != nil {
+		return err
+	}
+	if err := os.MkdirAll(repoAbsPath, os.ModePerm); err != nil {
 		return err
 	}
 
@@ -148,39 +151,42 @@ func (g *RepositoryDumper) CreateRepo(ctx context.Context, repo *base.Repository
 		return err
 	}
 
-	err = git.Clone(ctx, remoteAddr, repoPath, git.CloneRepoOptions{
+	err = git.Clone(ctx, remoteAddr, repoAbsPath, git.CloneRepoOptions{
 		Mirror:        true,
 		Quiet:         true,
 		Timeout:       migrateTimeout,
 		SkipTLSVerify: setting.Migrations.SkipTLSVerify,
 	})
 	if err != nil {
-		return fmt.Errorf("Clone: %w", err)
+		return fmt.Errorf("clone code: %w", err)
 	}
 
-	repoLocal := gitcmd.RepositoryUnmanaged(repoPath)
+	repoLocal := gitcmd.RepositoryUnmanaged(repoAbsPath)
 	if err := git.WriteCommitGraph(ctx, repoLocal); err != nil {
 		return err
 	}
 
 	if opts.Wiki {
-		wikiPath := g.wikiPath()
+		wikiAbsPath, err := filepath.Abs(g.wikiPath())
+		if err != nil {
+			return err
+		}
 		wikiRemotePath := repository.WikiRemoteURL(ctx, remoteAddr)
 		if len(wikiRemotePath) > 0 {
-			if err := os.MkdirAll(wikiPath, os.ModePerm); err != nil {
+			if err := os.MkdirAll(wikiAbsPath, os.ModePerm); err != nil {
-				return fmt.Errorf("Failed to remove %s: %w", wikiPath, err)
+				return fmt.Errorf("failed to create %s: %w", wikiAbsPath, err)
 			}
-			wikiLocal := gitcmd.RepositoryUnmanaged(wikiPath)
+			wikiLocal := gitcmd.RepositoryUnmanaged(wikiAbsPath)
-			if err := git.Clone(ctx, wikiRemotePath, wikiPath, git.CloneRepoOptions{
+			if err := git.Clone(ctx, wikiRemotePath, wikiAbsPath, git.CloneRepoOptions{
 				Mirror:        true,
 				Quiet:         true,
 				Timeout:       migrateTimeout,
 				Branch:        "master",
 				SkipTLSVerify: setting.Migrations.SkipTLSVerify,
 			}); err != nil {
-				log.Warn("Clone wiki: %v", err)
+				log.Warn("Failed to clone wiki: %v", err)
-				if err := os.RemoveAll(wikiPath); err != nil {
+				if err := os.RemoveAll(wikiAbsPath); err != nil {
-					return fmt.Errorf("Failed to remove %s: %w", wikiPath, err)
+					return fmt.Errorf("failed to remove %s: %w", wikiAbsPath, err)
 				}
 			} else if err := git.WriteCommitGraph(ctx, wikiLocal); err != nil {
 				return err
@@ -188,7 +194,7 @@ func (g *RepositoryDumper) CreateRepo(ctx context.Context, repo *base.Repository
 		}
 	}
 
-	g.gitRepo, err = git.OpenRepositoryLocal(g.gitPath())
+	g.gitRepo, err = git.OpenRepositoryLocal(repoAbsPath)
 	return err
 }
 
@@ -502,7 +508,7 @@ func (g *RepositoryDumper) handlePullRequest(ctx context.Context, pr *base.PullR
 	if pr.Head.CloneURL == "" || pr.Head.Ref == "" {
 		// Set head information if pr.Head.SHA is available
 		if pr.Head.SHA != "" {
-			_, _, err = gitcmd.NewCommand("update-ref", "--no-deref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.Head.SHA).WithDir(g.gitPath()).RunStdString(ctx)
+			_, _, err = gitcmd.NewCommand("update-ref", "--no-deref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.Head.SHA).WithRepo(g.gitRepo).RunStdString(ctx)
 			if err != nil {
 				log.Error("PR #%d in %s/%s unable to update-ref for pr HEAD: %v", pr.Number, g.repoOwner, g.repoName, err)
 			}
@@ -532,7 +538,7 @@ func (g *RepositoryDumper) handlePullRequest(ctx context.Context, pr *base.PullR
 	if !ok {
 		// Set head information if pr.Head.SHA is available
 		if pr.Head.SHA != "" {
-			_, _, err = gitcmd.NewCommand("update-ref", "--no-deref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.Head.SHA).WithDir(g.gitPath()).RunStdString(ctx)
+			_, _, err = gitcmd.NewCommand("update-ref", "--no-deref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.Head.SHA).WithRepo(g.gitRepo).RunStdString(ctx)
 			if err != nil {
 				log.Error("PR #%d in %s/%s unable to update-ref for pr HEAD: %v", pr.Number, g.repoOwner, g.repoName, err)
 			}
@@ -567,7 +573,7 @@ func (g *RepositoryDumper) handlePullRequest(ctx context.Context, pr *base.PullR
 			fetchArg = git.BranchPrefix + fetchArg
 		}
 
-		_, _, err = gitcmd.NewCommand("fetch", "--no-tags").AddDashesAndList(remote, fetchArg).WithDir(g.gitPath()).RunStdString(ctx)
+		_, _, err = gitcmd.NewCommand("fetch", "--no-tags").AddDashesAndList(remote, fetchArg).WithRepo(g.gitRepo).RunStdString(ctx)
 		if err != nil {
 			log.Error("Fetch branch from %s failed: %v", pr.Head.CloneURL, err)
 			// We need to continue here so that the Head.Ref is reset and we attempt to set the gitref for the PR
@@ -591,7 +597,7 @@ func (g *RepositoryDumper) handlePullRequest(ctx context.Context, pr *base.PullR
 		pr.Head.SHA = headSha
 	}
 	if pr.Head.SHA != "" {
-		_, _, err = gitcmd.NewCommand("update-ref", "--no-deref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.Head.SHA).WithDir(g.gitPath()).RunStdString(ctx)
+		_, _, err = gitcmd.NewCommand("update-ref", "--no-deref").AddDynamicArguments(pr.GetGitHeadRefName(), pr.Head.SHA).WithRepo(g.gitRepo).RunStdString(ctx)
 		if 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)
 		}

services/pull/review.go

@@ -258,7 +258,7 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo
 			if err == nil {
 				commitID = commit.ID.String()
 			} else if !isErrBlameNotFoundOrNotEnoughLines(err) {
-				return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %w", pr.GetGitHeadRefName(), gitRepo.Path, treePath, line, err)
+				return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %w", pr.GetGitHeadRefName(), gitRepo.LogString(), treePath, line, err)
 			}
 		}
 	}

services/repository/hooks.go

@@ -64,13 +64,13 @@ func GenerateGitHooks(ctx context.Context, templateRepo, generateRepo *repo_mode
 	}
 	defer templateGitRepo.Close()
 
-	templateHooks, err := templateGitRepo.Hooks()
+	templateHooks, err := git.ListHooks(templateGitRepo)
 	if err != nil {
 		return err
 	}
 
 	for _, templateHook := range templateHooks {
-		generateHook, err := generateGitRepo.GetHook(templateHook.Name())
+		generateHook, err := git.GetHook(generateGitRepo, templateHook.Name())
 		if err != nil {
 			return err
 		}