From 51775f65bc933843199320b040186703a2bb9f51 Mon Sep 17 00:00:00 2001 From: zeripath Date: Mon, 7 Jun 2021 00:44:58 +0100 Subject: [PATCH] Make commit info cancelable (#16032) * Make modules/context.Context a context.Context Signed-off-by: Andrew Thornton * Simplify context calls Signed-off-by: Andrew Thornton * Set the base context for requests to the HammerContext Signed-off-by: Andrew Thornton * pass context into get-last-commit Signed-off-by: Andrew Thornton * Make commit_info cancellable Signed-off-by: Andrew Thornton * use context as context Signed-off-by: Andrew Thornton Co-authored-by: 6543 <6543@obermui.de> --- modules/git/commit_info_gogit.go | 14 ++++++++++---- modules/git/commit_info_nogogit.go | 23 ++++++++++++++++------- modules/git/commit_info_test.go | 5 +++-- modules/git/last_commit_cache_gogit.go | 11 ++++++----- modules/git/last_commit_cache_nogogit.go | 11 ++++++----- modules/git/notes_gogit.go | 5 +++-- modules/git/notes_nogogit.go | 5 +++-- modules/git/notes_test.go | 7 ++++--- modules/repository/cache.go | 5 +++-- routers/repo/commit.go | 2 +- routers/repo/view.go | 2 +- services/repository/push.go | 2 +- 12 files changed, 57 insertions(+), 35 deletions(-) diff --git a/modules/git/commit_info_gogit.go b/modules/git/commit_info_gogit.go index 6d95e22d0..a8006dcef 100644 --- a/modules/git/commit_info_gogit.go +++ b/modules/git/commit_info_gogit.go @@ -7,6 +7,7 @@ package git import ( + "context" "path" "github.com/emirpasic/gods/trees/binaryheap" @@ -16,7 +17,7 @@ import ( ) // GetCommitsInfo gets information of all commits that are corresponding to these entries -func (tes Entries) GetCommitsInfo(commit *Commit, treePath string, cache *LastCommitCache) ([]CommitInfo, *Commit, error) { +func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath string, cache *LastCommitCache) ([]CommitInfo, *Commit, error) { entryPaths := make([]string, len(tes)+1) // Get the commit for the treePath itself entryPaths[0] = "" @@ -42,7 +43,7 @@ func (tes Entries) GetCommitsInfo(commit *Commit, treePath string, cache *LastCo return nil, nil, err } if len(unHitPaths) > 0 { - revs2, err := GetLastCommitForPaths(c, treePath, unHitPaths) + revs2, err := GetLastCommitForPaths(ctx, c, treePath, unHitPaths) if err != nil { return nil, nil, err } @@ -55,7 +56,7 @@ func (tes Entries) GetCommitsInfo(commit *Commit, treePath string, cache *LastCo } } } else { - revs, err = GetLastCommitForPaths(c, treePath, entryPaths) + revs, err = GetLastCommitForPaths(ctx, c, treePath, entryPaths) } if err != nil { return nil, nil, err @@ -173,7 +174,7 @@ func getLastCommitForPathsByCache(commitID, treePath string, paths []string, cac } // GetLastCommitForPaths returns last commit information -func GetLastCommitForPaths(c cgobject.CommitNode, treePath string, paths []string) (map[string]*object.Commit, error) { +func GetLastCommitForPaths(ctx context.Context, c cgobject.CommitNode, treePath string, paths []string) (map[string]*object.Commit, error) { // We do a tree traversal with nodes sorted by commit time heap := binaryheap.NewWith(func(a, b interface{}) int { if a.(*commitAndPaths).commit.CommitTime().Before(b.(*commitAndPaths).commit.CommitTime()) { @@ -192,6 +193,11 @@ func GetLastCommitForPaths(c cgobject.CommitNode, treePath string, paths []strin heap.Push(&commitAndPaths{c, paths, initialHashes}) for { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } cIn, ok := heap.Pop() if !ok { break diff --git a/modules/git/commit_info_nogogit.go b/modules/git/commit_info_nogogit.go index 485271f14..f34bef9f0 100644 --- a/modules/git/commit_info_nogogit.go +++ b/modules/git/commit_info_nogogit.go @@ -9,6 +9,7 @@ package git import ( "bufio" "bytes" + "context" "fmt" "io" "math" @@ -18,7 +19,7 @@ import ( ) // GetCommitsInfo gets information of all commits that are corresponding to these entries -func (tes Entries) GetCommitsInfo(commit *Commit, treePath string, cache *LastCommitCache) ([]CommitInfo, *Commit, error) { +func (tes Entries) GetCommitsInfo(ctx context.Context, commit *Commit, treePath string, cache *LastCommitCache) ([]CommitInfo, *Commit, error) { entryPaths := make([]string, len(tes)+1) // Get the commit for the treePath itself entryPaths[0] = "" @@ -31,13 +32,13 @@ func (tes Entries) GetCommitsInfo(commit *Commit, treePath string, cache *LastCo var revs map[string]*Commit if cache != nil { var unHitPaths []string - revs, unHitPaths, err = getLastCommitForPathsByCache(commit.ID.String(), treePath, entryPaths, cache) + revs, unHitPaths, err = getLastCommitForPathsByCache(ctx, commit.ID.String(), treePath, entryPaths, cache) if err != nil { return nil, nil, err } if len(unHitPaths) > 0 { sort.Strings(unHitPaths) - commits, err := GetLastCommitForPaths(commit, treePath, unHitPaths) + commits, err := GetLastCommitForPaths(ctx, commit, treePath, unHitPaths) if err != nil { return nil, nil, err } @@ -53,7 +54,7 @@ func (tes Entries) GetCommitsInfo(commit *Commit, treePath string, cache *LastCo sort.Strings(entryPaths) revs = map[string]*Commit{} var foundCommits []*Commit - foundCommits, err = GetLastCommitForPaths(commit, treePath, entryPaths) + foundCommits, err = GetLastCommitForPaths(ctx, commit, treePath, entryPaths) for i, found := range foundCommits { revs[entryPaths[i]] = found } @@ -101,7 +102,7 @@ func (tes Entries) GetCommitsInfo(commit *Commit, treePath string, cache *LastCo return commitsInfo, treeCommit, nil } -func getLastCommitForPathsByCache(commitID, treePath string, paths []string, cache *LastCommitCache) (map[string]*Commit, []string, error) { +func getLastCommitForPathsByCache(ctx context.Context, commitID, treePath string, paths []string, cache *LastCommitCache) (map[string]*Commit, []string, error) { wr, rd, cancel := cache.repo.CatFileBatch() defer cancel() @@ -124,7 +125,7 @@ func getLastCommitForPathsByCache(commitID, treePath string, paths []string, cac } // GetLastCommitForPaths returns last commit information -func GetLastCommitForPaths(commit *Commit, treePath string, paths []string) ([]*Commit, error) { +func GetLastCommitForPaths(ctx context.Context, commit *Commit, treePath string, paths []string) ([]*Commit, error) { // We read backwards from the commit to obtain all of the commits // We'll do this by using rev-list to provide us with parent commits in order @@ -136,7 +137,7 @@ func GetLastCommitForPaths(commit *Commit, treePath string, paths []string) ([]* go func() { stderr := strings.Builder{} - err := NewCommand("rev-list", "--format=%T", commit.ID.String()).RunInDirPipeline(commit.repo.Path, revListWriter, &stderr) + err := NewCommand("rev-list", "--format=%T", commit.ID.String()).SetParentContext(ctx).RunInDirPipeline(commit.repo.Path, revListWriter, &stderr) if err != nil { _ = revListWriter.CloseWithError(ConcatenateError(err, (&stderr).String())) } else { @@ -202,6 +203,11 @@ revListLoop: treeReadingLoop: for { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } _, _, size, err := ReadBatchLine(batchReader) if err != nil { return nil, err @@ -321,6 +327,9 @@ revListLoop: } } } + if scan.Err() != nil { + return nil, scan.Err() + } commitsMap := make(map[string]*Commit, len(commits)) commitsMap[commit.ID.String()] = commit diff --git a/modules/git/commit_info_test.go b/modules/git/commit_info_test.go index 3966419bc..0608801f9 100644 --- a/modules/git/commit_info_test.go +++ b/modules/git/commit_info_test.go @@ -5,6 +5,7 @@ package git import ( + "context" "os" "path/filepath" "testing" @@ -69,7 +70,7 @@ func testGetCommitsInfo(t *testing.T, repo1 *Repository) { assert.NoError(t, err) entries, err := tree.ListEntries() assert.NoError(t, err) - commitsInfo, treeCommit, err := entries.GetCommitsInfo(commit, testCase.Path, nil) + commitsInfo, treeCommit, err := entries.GetCommitsInfo(context.Background(), commit, testCase.Path, nil) assert.NoError(t, err) if err != nil { t.FailNow() @@ -136,7 +137,7 @@ func BenchmarkEntries_GetCommitsInfo(b *testing.B) { b.ResetTimer() b.Run(benchmark.name, func(b *testing.B) { for i := 0; i < b.N; i++ { - _, _, err := entries.GetCommitsInfo(commit, "", nil) + _, _, err := entries.GetCommitsInfo(context.Background(), commit, "", nil) if err != nil { b.Fatal(err) } diff --git a/modules/git/last_commit_cache_gogit.go b/modules/git/last_commit_cache_gogit.go index 65d6299be..16fb1c988 100644 --- a/modules/git/last_commit_cache_gogit.go +++ b/modules/git/last_commit_cache_gogit.go @@ -7,6 +7,7 @@ package git import ( + "context" "path" "github.com/go-git/go-git/v5/plumbing/object" @@ -60,7 +61,7 @@ func (c *LastCommitCache) Get(ref, entryPath string) (interface{}, error) { } // CacheCommit will cache the commit from the gitRepository -func (c *LastCommitCache) CacheCommit(commit *Commit) error { +func (c *LastCommitCache) CacheCommit(ctx context.Context, commit *Commit) error { commitNodeIndex, _ := commit.repo.CommitNodeIndex() @@ -69,10 +70,10 @@ func (c *LastCommitCache) CacheCommit(commit *Commit) error { return err } - return c.recursiveCache(index, &commit.Tree, "", 1) + return c.recursiveCache(ctx, index, &commit.Tree, "", 1) } -func (c *LastCommitCache) recursiveCache(index cgobject.CommitNode, tree *Tree, treePath string, level int) error { +func (c *LastCommitCache) recursiveCache(ctx context.Context, index cgobject.CommitNode, tree *Tree, treePath string, level int) error { if level == 0 { return nil } @@ -89,7 +90,7 @@ func (c *LastCommitCache) recursiveCache(index cgobject.CommitNode, tree *Tree, entryMap[entry.Name()] = entry } - commits, err := GetLastCommitForPaths(index, treePath, entryPaths) + commits, err := GetLastCommitForPaths(ctx, index, treePath, entryPaths) if err != nil { return err } @@ -103,7 +104,7 @@ func (c *LastCommitCache) recursiveCache(index cgobject.CommitNode, tree *Tree, if err != nil { return err } - if err := c.recursiveCache(index, subTree, entry, level-1); err != nil { + if err := c.recursiveCache(ctx, index, subTree, entry, level-1); err != nil { return err } } diff --git a/modules/git/last_commit_cache_nogogit.go b/modules/git/last_commit_cache_nogogit.go index 9808216a8..3cbb0cca3 100644 --- a/modules/git/last_commit_cache_nogogit.go +++ b/modules/git/last_commit_cache_nogogit.go @@ -8,6 +8,7 @@ package git import ( "bufio" + "context" "path" ) @@ -61,11 +62,11 @@ func (c *LastCommitCache) Get(ref, entryPath string, wr WriteCloserError, rd *bu } // CacheCommit will cache the commit from the gitRepository -func (c *LastCommitCache) CacheCommit(commit *Commit) error { - return c.recursiveCache(commit, &commit.Tree, "", 1) +func (c *LastCommitCache) CacheCommit(ctx context.Context, commit *Commit) error { + return c.recursiveCache(ctx, commit, &commit.Tree, "", 1) } -func (c *LastCommitCache) recursiveCache(commit *Commit, tree *Tree, treePath string, level int) error { +func (c *LastCommitCache) recursiveCache(ctx context.Context, commit *Commit, tree *Tree, treePath string, level int) error { if level == 0 { return nil } @@ -82,7 +83,7 @@ func (c *LastCommitCache) recursiveCache(commit *Commit, tree *Tree, treePath st entryMap[entry.Name()] = entry } - commits, err := GetLastCommitForPaths(commit, treePath, entryPaths) + commits, err := GetLastCommitForPaths(ctx, commit, treePath, entryPaths) if err != nil { return err } @@ -97,7 +98,7 @@ func (c *LastCommitCache) recursiveCache(commit *Commit, tree *Tree, treePath st if err != nil { return err } - if err := c.recursiveCache(commit, subTree, entry, level-1); err != nil { + if err := c.recursiveCache(ctx, commit, subTree, entry, level-1); err != nil { return err } } diff --git a/modules/git/notes_gogit.go b/modules/git/notes_gogit.go index 173d29cee..534a5d517 100644 --- a/modules/git/notes_gogit.go +++ b/modules/git/notes_gogit.go @@ -7,13 +7,14 @@ package git import ( + "context" "io/ioutil" "github.com/go-git/go-git/v5/plumbing/object" ) // GetNote retrieves the git-notes data for a given commit. -func GetNote(repo *Repository, commitID string, note *Note) error { +func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note) error { notes, err := repo.GetCommit(NotesRef) if err != nil { return err @@ -62,7 +63,7 @@ func GetNote(repo *Repository, commitID string, note *Note) error { return err } - lastCommits, err := GetLastCommitForPaths(commitNode, "", []string{path}) + lastCommits, err := GetLastCommitForPaths(ctx, commitNode, "", []string{path}) if err != nil { return err } diff --git a/modules/git/notes_nogogit.go b/modules/git/notes_nogogit.go index d5d194b23..2b9272499 100644 --- a/modules/git/notes_nogogit.go +++ b/modules/git/notes_nogogit.go @@ -7,12 +7,13 @@ package git import ( + "context" "io/ioutil" "strings" ) // GetNote retrieves the git-notes data for a given commit. -func GetNote(repo *Repository, commitID string, note *Note) error { +func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note) error { notes, err := repo.GetCommit(NotesRef) if err != nil { return err @@ -63,7 +64,7 @@ func GetNote(repo *Repository, commitID string, note *Note) error { path = path[idx+1:] } - lastCommits, err := GetLastCommitForPaths(notes, treePath, []string{path}) + lastCommits, err := GetLastCommitForPaths(ctx, notes, treePath, []string{path}) if err != nil { return err } diff --git a/modules/git/notes_test.go b/modules/git/notes_test.go index b7939e691..f66a191e6 100644 --- a/modules/git/notes_test.go +++ b/modules/git/notes_test.go @@ -5,6 +5,7 @@ package git import ( + "context" "path/filepath" "testing" @@ -18,7 +19,7 @@ func TestGetNotes(t *testing.T) { defer bareRepo1.Close() note := Note{} - err = GetNote(bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653", ¬e) + err = GetNote(context.Background(), bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653", ¬e) assert.NoError(t, err) assert.Equal(t, []byte("Note contents\n"), note.Message) assert.Equal(t, "Vladimir Panteleev", note.Commit.Author.Name) @@ -31,10 +32,10 @@ func TestGetNestedNotes(t *testing.T) { defer repo.Close() note := Note{} - err = GetNote(repo, "3e668dbfac39cbc80a9ff9c61eb565d944453ba4", ¬e) + err = GetNote(context.Background(), repo, "3e668dbfac39cbc80a9ff9c61eb565d944453ba4", ¬e) assert.NoError(t, err) assert.Equal(t, []byte("Note 2"), note.Message) - err = GetNote(repo, "ba0a96fa63532d6c5087ecef070b0250ed72fa47", ¬e) + err = GetNote(context.Background(), repo, "ba0a96fa63532d6c5087ecef070b0250ed72fa47", ¬e) assert.NoError(t, err) assert.Equal(t, []byte("Note 1"), note.Message) } diff --git a/modules/repository/cache.go b/modules/repository/cache.go index 5d6dea8fc..e574f1adb 100644 --- a/modules/repository/cache.go +++ b/modules/repository/cache.go @@ -5,6 +5,7 @@ package repository import ( + "context" "strings" "code.gitea.io/gitea/models" @@ -23,7 +24,7 @@ func getRefName(fullRefName string) string { } // CacheRef cachhe last commit information of the branch or the tag -func CacheRef(repo *models.Repository, gitRepo *git.Repository, fullRefName string) error { +func CacheRef(ctx context.Context, repo *models.Repository, gitRepo *git.Repository, fullRefName string) error { if !setting.CacheService.LastCommit.Enabled { return nil } @@ -43,5 +44,5 @@ func CacheRef(repo *models.Repository, gitRepo *git.Repository, fullRefName stri commitCache := git.NewLastCommitCache(repo.FullName(), gitRepo, setting.LastCommitCacheTTLSeconds, cache.GetCache()) - return commitCache.CacheCommit(commit) + return commitCache.CacheCommit(ctx, commit) } diff --git a/routers/repo/commit.go b/routers/repo/commit.go index c47195263..3e6148bcb 100644 --- a/routers/repo/commit.go +++ b/routers/repo/commit.go @@ -355,7 +355,7 @@ func Diff(ctx *context.Context) { } note := &git.Note{} - err = git.GetNote(ctx.Repo.GitRepo, commitID, note) + err = git.GetNote(ctx, ctx.Repo.GitRepo, commitID, note) if err == nil { ctx.Data["Note"] = string(charset.ToUTF8WithFallback(note.Message)) ctx.Data["NoteCommit"] = note.Commit diff --git a/routers/repo/view.go b/routers/repo/view.go index 30d7de407..cd5b0f43e 100644 --- a/routers/repo/view.go +++ b/routers/repo/view.go @@ -146,7 +146,7 @@ func renderDirectory(ctx *context.Context, treeLink string) { } var latestCommit *git.Commit - ctx.Data["Files"], latestCommit, err = entries.GetCommitsInfo(ctx.Repo.Commit, ctx.Repo.TreePath, c) + ctx.Data["Files"], latestCommit, err = entries.GetCommitsInfo(ctx, ctx.Repo.Commit, ctx.Repo.TreePath, c) if err != nil { ctx.ServerError("GetCommitsInfo", err) return diff --git a/services/repository/push.go b/services/repository/push.go index bed5c575f..f031073b2 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -208,7 +208,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } // Cache for big repository - if err := repo_module.CacheRef(repo, gitRepo, opts.RefFullName); err != nil { + if err := repo_module.CacheRef(graceful.GetManager().HammerContext(), repo, gitRepo, opts.RefFullName); err != nil { log.Error("repo_module.CacheRef %s/%s failed: %v", repo.ID, branch, err) } } else {