diff --git a/models/repo/repo_location_test.go b/models/repo/repo_location_test.go index 349db95567..bc74d7d815 100644 --- a/models/repo/repo_location_test.go +++ b/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()) diff --git a/modules/git/attribute/batch.go b/modules/git/attribute/batch.go index cfe43269f8..60db21d0c7 100644 --- a/modules/git/attribute/batch.go +++ b/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()) diff --git a/modules/git/blob_nogogit.go b/modules/git/blob_nogogit.go index 4803a50efe..9f2d91b879 100644 --- a/modules/git/blob_nogogit.go +++ b/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 diff --git a/modules/git/catfile_batch_test.go b/modules/git/catfile_batch_test.go index a7902124e5..955782bc3c 100644 --- a/modules/git/catfile_batch_test.go +++ b/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)) diff --git a/modules/git/commit_info_gogit.go b/modules/git/commit_info_gogit.go index f5c01ad2cc..c0233b7f82 100644 --- a/modules/git/commit_info_gogit.go +++ b/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() - if commitGraphFile != nil { - defer commitGraphFile.Close() - } + commitNodeIndex, closer := gitRepo.CommitNodeIndex() + defer closer() c, err := commitNodeIndex.Get(plumbing.Hash(commit.ID.RawValue())) if err != nil { diff --git a/modules/git/diff.go b/modules/git/diff.go index 7b85532f38..442cf742ac 100644 --- a/modules/git/diff.go +++ b/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 diff --git a/modules/git/gitcmd/command.go b/modules/git/gitcmd/command.go index 28ed20ece0..184874bde6 100644 --- a/modules/git/gitcmd/command.go +++ b/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 diff --git a/modules/git/gitcmd/repo.go b/modules/git/gitcmd/repo.go index 0a5ab9865b..6e76e760eb 100644 --- a/modules/git/gitcmd/repo.go +++ b/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 { - return string(r) +func (r *repositoryUnmanaged) GitRepoLocation() string { + 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 - loc string + id, 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)} } diff --git a/modules/git/grep_test.go b/modules/git/grep_test.go index 430a27536e..08cebaf825 100644 --- a/modules/git/grep_test.go +++ b/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) diff --git a/modules/git/hook.go b/modules/git/hook.go index b8cf186dc1..bba8f423a9 100644 --- a/modules/git/hook.go +++ b/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) { - exist, err := util.IsDir(filepath.Join(repoPath, "hooks")) +func ListHooks(repo RepositoryFacade) (_ []*Hook, err error) { + 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 } diff --git a/modules/git/last_commit_cache_gogit.go b/modules/git/last_commit_cache_gogit.go index b4b2368d2d..eec9545785 100644 --- a/modules/git/last_commit_cache_gogit.go +++ b/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 { diff --git a/modules/git/log_name_status_nogogit.go b/modules/git/log_name_status_nogogit.go index 05924fca11..acb0860b4a 100644 --- a/modules/git/log_name_status_nogogit.go +++ b/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 diff --git a/modules/git/notes.go b/modules/git/notes.go index 95bdeac8aa..3603b76017 100644 --- a/modules/git/notes.go +++ b/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) { diff --git a/modules/git/repo.go b/modules/git/repo.go index 1754e7b97f..e96c612e02 100644 --- a/modules/git/repo.go +++ b/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) diff --git a/modules/git/repo_base_gogit.go b/modules/git/repo_base_gogit.go index b7d6c69d0f..82929014fa 100644 --- a/modules/git/repo_base_gogit.go +++ b/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") diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go index 47f320d800..442ee0c54f 100644 --- a/modules/git/repo_branch_nogogit.go +++ b/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) { - countAll, err = WalkShowRef(ctx, repoPath, extraArgs, skip, limit, func(_, branchName string) 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, 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) } diff --git a/modules/git/repo_commitgraph_gogit.go b/modules/git/repo_commitgraph_gogit.go index 68646737a2..2c025881a3 100644 --- a/modules/git/repo_commitgraph_gogit.go +++ b/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) { - indexPath := filepath.Join(repo.Path, "objects", "info", "commit-graph") - +func (repo *Repository) CommitNodeIndex() (_ cgobject.CommitNodeIndex, closer func()) { + 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() {} } diff --git a/modules/git/repo_hook.go b/modules/git/repo_hook.go deleted file mode 100644 index cdf076505d..0000000000 --- a/modules/git/repo_hook.go +++ /dev/null @@ -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) -} diff --git a/modules/git/repo_ref_gogit.go b/modules/git/repo_ref_gogit.go index 2c2407e980..c66644c549 100644 --- a/modules/git/repo_ref_gogit.go +++ b/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) - if err != nil { - return nil, err - } + r := repo.gogitRepo refsIter, err := r.References() if err != nil { diff --git a/modules/git/tree_entry_nogogit.go b/modules/git/tree_entry_nogogit.go index 53e1e1aef5..471ec6d3a1 100644 --- a/modules/git/tree_entry_nogogit.go +++ b/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 } diff --git a/modules/templates/util_render_test.go b/modules/templates/util_render_test.go index 1db87feb79..d9dabf8ec8 100644 --- a/modules/templates/util_render_test.go +++ b/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"}, diff --git a/routers/api/v1/repo/git_hook.go b/routers/api/v1/repo/git_hook.go index a47c71320d..656fa9c9df 100644 --- a/routers/api/v1/repo/git_hook.go +++ b/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() diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go index 9bb77e9f99..5e82675852 100644 --- a/routers/private/hook_verification.go +++ b/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 } diff --git a/routers/web/repo/setting/git_hooks.go b/routers/web/repo/setting/git_hooks.go index 9ea9e825ab..53b7e9e359 100644 --- a/routers/web/repo/setting/git_hooks.go +++ b/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) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 6c32c7c8fd..930ef6719c 100644 --- a/services/gitdiff/gitdiff.go +++ b/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 { diff --git a/services/migrations/dump.go b/services/migrations/dump.go index 3e3b537ebd..752d229615 100644 --- a/services/migrations/dump.go +++ b/services/migrations/dump.go @@ -136,8 +136,11 @@ func (g *RepositoryDumper) CreateRepo(ctx context.Context, repo *base.Repository return err } - repoPath := g.gitPath() - if err := os.MkdirAll(repoPath, os.ModePerm); err != nil { + repoAbsPath, err := filepath.Abs(g.gitPath()) + 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 { - return fmt.Errorf("Failed to remove %s: %w", wikiPath, err) + if err := os.MkdirAll(wikiAbsPath, os.ModePerm); err != nil { + return fmt.Errorf("failed to create %s: %w", wikiAbsPath, err) } - wikiLocal := gitcmd.RepositoryUnmanaged(wikiPath) - if err := git.Clone(ctx, wikiRemotePath, wikiPath, git.CloneRepoOptions{ + wikiLocal := gitcmd.RepositoryUnmanaged(wikiAbsPath) + 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) - if err := os.RemoveAll(wikiPath); err != nil { - return fmt.Errorf("Failed to remove %s: %w", wikiPath, err) + log.Warn("Failed to clone wiki: %v", err) + if err := os.RemoveAll(wikiAbsPath); err != nil { + 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) } diff --git a/services/pull/review.go b/services/pull/review.go index aebd7ed7e9..e07e896034 100644 --- a/services/pull/review.go +++ b/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) } } } diff --git a/services/repository/hooks.go b/services/repository/hooks.go index 81473c48c4..d2c1a1faa5 100644 --- a/services/repository/hooks.go +++ b/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 }