diff --git a/modules/git/commit_info.go b/modules/git/commit_info.go index 87a6ddd8d1..19ed12fb9d 100644 --- a/modules/git/commit_info.go +++ b/modules/git/commit_info.go @@ -3,7 +3,15 @@ package git -import "context" +import ( + "context" + "maps" + "path" + "time" + + "gitea.dev/modules/log" + "gitea.dev/modules/util" +) // CommitInfo describes the first commit with the provided entry type CommitInfo struct { @@ -23,3 +31,91 @@ func GetCommitInfoSubmoduleFile(ctx context.Context, repoLink, fullPath string, } return NewCommitSubmoduleFile(repoLink, fullPath, submodule.URL, refCommitID.String()), nil } + +func getLastCommitForPathsWithTimeout(ctx context.Context, timeout time.Duration, gitRepo *Repository, commit *Commit, treePath string, entryNames []string) (map[string]*Commit, error) { + // Actually using "timeout ctx" here is not ideal: it creates a new git cat-file batch process + // (the other one uses the request's ctx and is cached by the gitRepo). + // Ideally, the underlying functions only need to return in predictable time, + // so it should use the shared (cached) git cat-file batcher from the request context and just use "time.After" for deadline. + // However, it's difficult to make it right at the moment. + ctxInner := ctx + if timeout > 0 { + var cancel context.CancelFunc + ctxInner, cancel = context.WithTimeout(ctx, timeout) + defer cancel() + } + revs, err := GetLastCommitForPaths(ctxInner, gitRepo, commit, treePath, entryNames) + if err != nil && ctxInner.Err() == nil { + // only return the err if context is not canceled + return nil, err + } + return revs, nil +} + +// GetCommitsInfo gets information of all commits that are corresponding to these entries +func (tes Entries) GetCommitsInfo(ctx context.Context, timeout time.Duration, repoLink string, gitRepo *Repository, commit *Commit, treePath string) (commitsInfo []CommitInfo, treeLastCommit *Commit, err error) { + entryNames := make([]string, 0, len(tes)+1) + entryNames = append(entryNames, "") // include the commit for the treePath itself + for _, entry := range tes { + entryNames = append(entryNames, entry.Name()) + } + + var revs map[string]*Commit + var remainingEntryNames []string + if gitRepo.LastCommitCache != nil { + revs, remainingEntryNames, err = getLastCommitForPathsByCache(ctx, commit.ID.String(), treePath, entryNames, gitRepo.LastCommitCache) + if err != nil { + return nil, nil, err + } + } else { + revs, remainingEntryNames = map[string]*Commit{}, entryNames + } + + if len(remainingEntryNames) > 0 { + remainingRevs, err := getLastCommitForPathsWithTimeout(ctx, timeout, gitRepo, commit, treePath, remainingEntryNames) + if err != nil { + return nil, nil, err + } + maps.Copy(revs, remainingRevs) + } + + for _, entry := range tes { + ci := CommitInfo{Entry: entry, Commit: revs[entry.Name()]} + if ci.Commit == nil { + log.Debug("missing commit for %s", entry.Name()) + } + // If the entry is a submodule, add a submodule file for this + if entry.IsSubModule() { + ci.SubmoduleFile, err = GetCommitInfoSubmoduleFile(ctx, repoLink, path.Join(treePath, entry.Name()), gitRepo, commit, entry.ID) + if err != nil { + return nil, nil, err + } + } + commitsInfo = append(commitsInfo, ci) + } + + // Retrieve the commit for the treePath itself (see above). We basically + // get it for free during the tree traversal, and it's used for listing + // pages to display information about the newest commit for a given path. + treeLastCommit = util.Iif(treePath == "", commit, revs[""]) + return commitsInfo, treeLastCommit, nil +} + +func getLastCommitForPathsByCache(ctx context.Context, commitID, treePath string, paths []string, cache *LastCommitCache) (map[string]*Commit, []string, error) { + var unHitEntryPaths []string + results := make(map[string]*Commit) + for _, p := range paths { + lastCommit, err := cache.Get(ctx, commitID, path.Join(treePath, p)) + if err != nil { + return nil, nil, err + } + if lastCommit != nil { + results[p] = lastCommit + continue + } + + unHitEntryPaths = append(unHitEntryPaths, p) + } + + return results, unHitEntryPaths, nil +} diff --git a/modules/git/commit_info_gogit.go b/modules/git/commit_info_gogit.go index c0233b7f82..7c578580c8 100644 --- a/modules/git/commit_info_gogit.go +++ b/modules/git/commit_info_gogit.go @@ -7,7 +7,6 @@ package git import ( "context" - "maps" "path" "github.com/emirpasic/gods/trees/binaryheap" @@ -16,72 +15,6 @@ import ( cgobject "github.com/go-git/go-git/v5/plumbing/object/commitgraph" ) -// GetCommitsInfo gets information of all commits that are corresponding to these entries -func (tes Entries) GetCommitsInfo(ctx context.Context, repoLink string, gitRepo *Repository, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) { - entryPaths := make([]string, len(tes)+1) - // Get the commit for the treePath itself - entryPaths[0] = "" - for i, entry := range tes { - entryPaths[i+1] = entry.Name() - } - - var revs map[string]*Commit - var err error - if gitRepo.LastCommitCache != nil { - var unHitPaths []string - revs, unHitPaths, err = getLastCommitForPathsByCache(ctx, commit.ID.String(), treePath, entryPaths, gitRepo.LastCommitCache) - if err != nil { - return nil, nil, err - } - if len(unHitPaths) > 0 { - revs2, err := GetLastCommitForPaths(ctx, gitRepo, commit, treePath, unHitPaths) - if err != nil { - return nil, nil, err - } - - maps.Copy(revs, revs2) - } - } else { - revs, err = GetLastCommitForPaths(ctx, gitRepo, commit, treePath, entryPaths) - } - if err != nil { - return nil, nil, err - } - - gitRepo.gogitStorage.Close() - - commitsInfo := make([]CommitInfo, len(tes)) - for i, entry := range tes { - commitsInfo[i] = CommitInfo{ - Entry: entry, - } - - // Check if we have found a commit for this entry in time - if entryCommit, ok := revs[entry.Name()]; ok { - commitsInfo[i].Commit = entryCommit - } - - // If the entry is a submodule, add a submodule file for this - if entry.IsSubModule() { - commitsInfo[i].SubmoduleFile, err = GetCommitInfoSubmoduleFile(ctx, repoLink, path.Join(treePath, entry.Name()), gitRepo, commit, entry.ID) - if err != nil { - return nil, nil, err - } - } - } - - // Retrieve the commit for the treePath itself (see above). We basically - // get it for free during the tree traversal and it's used for listing - // pages to display information about newest commit for a given path. - var treeCommit *Commit - if treePath == "" { - treeCommit = commit - } else { - treeCommit = revs[""] - } - return commitsInfo, treeCommit, nil -} - type commitAndPaths struct { commit cgobject.CommitNode // Paths that are still on the branch represented by commit @@ -132,25 +65,6 @@ func getFileHashes(c cgobject.CommitNode, treePath string, paths []string) (map[ return hashes, nil } -func getLastCommitForPathsByCache(ctx context.Context, commitID, treePath string, paths []string, cache *LastCommitCache) (map[string]*Commit, []string, error) { - var unHitEntryPaths []string - results := make(map[string]*Commit) - for _, p := range paths { - lastCommit, err := cache.Get(ctx, commitID, path.Join(treePath, p)) - if err != nil { - return nil, nil, err - } - if lastCommit != nil { - results[p] = lastCommit - continue - } - - unHitEntryPaths = append(unHitEntryPaths, p) - } - - return results, unHitEntryPaths, nil -} - // GetLastCommitForPaths returns last commit information func GetLastCommitForPaths(ctx context.Context, gitRepo *Repository, commit *Commit, treePath string, paths []string) (map[string]*Commit, error) { commitNodeIndex, closer := gitRepo.CommitNodeIndex() diff --git a/modules/git/commit_info_nogogit.go b/modules/git/commit_info_nogogit.go index 2475388273..c6ceb78d91 100644 --- a/modules/git/commit_info_nogogit.go +++ b/modules/git/commit_info_nogogit.go @@ -7,101 +7,8 @@ package git import ( "context" - "maps" - "path" - "sort" - - "gitea.dev/modules/log" ) -// GetCommitsInfo gets information of all commits that are corresponding to these entries -func (tes Entries) GetCommitsInfo(ctx context.Context, repoLink string, gitRepo *Repository, commit *Commit, treePath string) ([]CommitInfo, *Commit, error) { - entryPaths := make([]string, len(tes)+1) - // Get the commit for the treePath itself - entryPaths[0] = "" - for i, entry := range tes { - entryPaths[i+1] = entry.Name() - } - - var err error - - var revs map[string]*Commit - if gitRepo.LastCommitCache != nil { - var unHitPaths []string - revs, unHitPaths, err = getLastCommitForPathsByCache(ctx, commit.ID.String(), treePath, entryPaths, gitRepo.LastCommitCache) - if err != nil { - return nil, nil, err - } - if len(unHitPaths) > 0 { - sort.Strings(unHitPaths) - commits, err := GetLastCommitForPaths(ctx, gitRepo, commit, treePath, unHitPaths) - if err != nil { - return nil, nil, err - } - - maps.Copy(revs, commits) - } - } else { - sort.Strings(entryPaths) - revs, err = GetLastCommitForPaths(ctx, gitRepo, commit, treePath, entryPaths) - } - if err != nil { - return nil, nil, err - } - - commitsInfo := make([]CommitInfo, len(tes)) - for i, entry := range tes { - commitsInfo[i] = CommitInfo{ - Entry: entry, - } - - // Check if we have found a commit for this entry in time - if entryCommit, ok := revs[entry.Name()]; ok { - commitsInfo[i].Commit = entryCommit - } else { - log.Debug("missing commit for %s", entry.Name()) - } - - // If the entry is a submodule, add a submodule file for this - if entry.IsSubModule() { - commitsInfo[i].SubmoduleFile, err = GetCommitInfoSubmoduleFile(ctx, repoLink, path.Join(treePath, entry.Name()), gitRepo, commit, entry.ID) - if err != nil { - return nil, nil, err - } - } - } - - // Retrieve the commit for the treePath itself (see above). We basically - // get it for free during the tree traversal, and it's used for listing - // pages to display information about the newest commit for a given path. - var treeCommit *Commit - if treePath == "" { - treeCommit = commit - } else { - treeCommit = revs[""] - } - return commitsInfo, treeCommit, nil -} - -func getLastCommitForPathsByCache(ctx context.Context, commitID, treePath string, paths []string, cache *LastCommitCache) (map[string]*Commit, []string, error) { - var unHitEntryPaths []string - results := make(map[string]*Commit) - for _, p := range paths { - lastCommit, err := cache.Get(ctx, commitID, path.Join(treePath, p)) - if err != nil { - return nil, nil, err - } - if lastCommit != nil { - results[p] = lastCommit - continue - } - - unHitEntryPaths = append(unHitEntryPaths, p) - } - - return results, unHitEntryPaths, nil -} - // GetLastCommitForPaths returns last commit information func GetLastCommitForPaths(ctx context.Context, gitRepo *Repository, commit *Commit, treePath string, paths []string) (map[string]*Commit, error) { // We read backwards from the commit to obtain all of the commits diff --git a/modules/git/commit_info_nogogit_test.go b/modules/git/commit_info_nogogit_test.go index f1f9bfcb1e..5ec14e8752 100644 --- a/modules/git/commit_info_nogogit_test.go +++ b/modules/git/commit_info_nogogit_test.go @@ -9,6 +9,7 @@ import ( "context" "path/filepath" "testing" + "time" "gitea.dev/modules/test" "gitea.dev/modules/util" @@ -39,14 +40,14 @@ func TestEntries_GetCommitsInfo_ContextErr(t *testing.T) { defer test.MockVariableValue(&walkGitLogDebugBeforeNext)() walkGitLogDebugBeforeNext = cancel - commitInfos, _, err := entries.GetCommitsInfo(ctx, "/any/repo-link", repo, commit, "") + commitInfos, _, err := entries.GetCommitsInfo(ctx, time.Second, "/any/repo-link", repo, commit, "") assert.NoError(t, err) nilCommits, nonNilCommits := countCommitInfosCommit(commitInfos) assert.Equal(t, 0, nonNilCommits) // no commit info due to canceled (or deadline-exceeded) context assert.Equal(t, 3, nilCommits) walkGitLogDebugBeforeNext = nil - commitInfos, _, err = entries.GetCommitsInfo(t.Context(), "/any/repo-link", repo, commit, "") + commitInfos, _, err = entries.GetCommitsInfo(t.Context(), time.Second, "/any/repo-link", repo, commit, "") assert.NoError(t, err) nilCommits, nonNilCommits = countCommitInfosCommit(commitInfos) assert.Equal(t, 3, nonNilCommits) diff --git a/modules/git/commit_info_test.go b/modules/git/commit_info_test.go index ceed96867f..80d4a63983 100644 --- a/modules/git/commit_info_test.go +++ b/modules/git/commit_info_test.go @@ -105,8 +105,7 @@ func testGetCommitsInfo(t *testing.T, repo1 *Repository) { continue } - // FIXME: Context.TODO() - if graceful has started we should use its Shutdown context otherwise use install signals in TestMain. - commitsInfo, treeCommit, err := entries.GetCommitsInfo(t.Context(), "/any/repo-link", repo1, commit, testCase.Path) + commitsInfo, treeCommit, err := entries.GetCommitsInfo(t.Context(), time.Second, "/any/repo-link", repo1, commit, testCase.Path) assert.NoError(t, err, "Unable to get commit information for entries of subtree: %s in commit: %s from testcase due to error: %v", testCase.Path, testCase.CommitID, err) if err != nil { t.FailNow() diff --git a/modules/git/repo.go b/modules/git/repo.go index e9cf974381..23a5272dab 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -280,14 +280,14 @@ func (repo *Repository) CatFileBatch(ctx context.Context) (_ CatFileBatch, close repo.mu.Lock() defer repo.mu.Unlock() - if repo.catFileBatchCloser != nil && !repo.catFileBatchInUse { - if ctx != repo.catFileBatchCloser.Context() { - repo.catFileBatchCloser.Close() - repo.catFileBatchCloser = nil - repo.catFileBatchInUse = false - } + // clean up the cached but canceled batcher + if repo.catFileBatchCloser != nil && repo.catFileBatchCloser.Context().Err() != nil && !repo.catFileBatchInUse { + repo.catFileBatchCloser.Close() + repo.catFileBatchCloser = nil + repo.catFileBatchInUse = false } + // if no cached batcher, make a new managed one, and cache it if repo.catFileBatchCloser == nil { repo.catFileBatchCloser, err = NewBatch(ctx, repo) if err != nil { @@ -296,7 +296,10 @@ func (repo *Repository) CatFileBatch(ctx context.Context) (_ CatFileBatch, close } } - if !repo.catFileBatchInUse { + // if the cached batcher is in use, or the cached ctx is not the current ctx, then we need a new temp one + // for example: the cached one is from request context, the current ctx is a cancelable ctx with timeout + needTemp := repo.catFileBatchInUse || repo.catFileBatchCloser.Context() != ctx + if !needTemp { repo.catFileBatchInUse = true return CatFileBatch(repo.catFileBatchCloser), func() { repo.mu.Lock() diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index e3583fd332..cc10322101 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -298,14 +298,7 @@ func renderDirectoryFiles(ctx *context.Context, timeout time.Duration) (*git.Tre } subEntries.CustomSort(base.NaturalSortCompare) - commitInfoCtx := gocontext.Context(ctx) - if timeout > 0 { - var cancel gocontext.CancelFunc - commitInfoCtx, cancel = gocontext.WithTimeout(ctx, timeout) - defer cancel() - } - - files, latestCommit, err := subEntries.GetCommitsInfo(commitInfoCtx, ctx.Repo.RepoLink, ctx.Repo.GitRepo, ctx.Repo.Commit, ctx.Repo.TreePath) + files, latestCommit, err := subEntries.GetCommitsInfo(ctx, timeout, ctx.Repo.RepoLink, ctx.Repo.GitRepo, ctx.Repo.Commit, ctx.Repo.TreePath) if err != nil { ctx.ServerError("GetCommitsInfo", err) return nil, nil diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index 13e4be88c2..4d82c67fd1 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -563,7 +563,7 @@ func WikiPages(ctx *context.Context) { } allEntries.CustomSort(base.NaturalSortCompare) - entries, _, err := allEntries.GetCommitsInfo(ctx, ctx.Repo.RepoLink, wikiGitRepo, commit, treePath) + entries, _, err := allEntries.GetCommitsInfo(ctx, 0, ctx.Repo.RepoLink, wikiGitRepo, commit, treePath) if err != nil { ctx.ServerError("GetCommitsInfo", err) return