From 14354e4f8edb35b6ff46ea066098fc1d9fe93d5a Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Fri, 1 Dec 2023 02:26:52 +0100 Subject: [PATCH] Read `previous` info from git blame (#28306) Fixes #28280 Reads the `previous` info from the `git blame` output instead of calculating it afterwards. --- modules/git/blame.go | 37 +++++++++++++++++++++++++------------ modules/git/blame_test.go | 28 ++++++++++++++++------------ routers/web/repo/blame.go | 38 ++++++++------------------------------ 3 files changed, 49 insertions(+), 54 deletions(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index 6728a6bed..93c7f184f 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -11,6 +11,7 @@ import ( "io" "os" "regexp" + "strings" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" @@ -18,8 +19,10 @@ import ( // BlamePart represents block of blame - continuous lines with one sha type BlamePart struct { - Sha string - Lines []string + Sha string + Lines []string + PreviousSha string + PreviousPath string } // BlameReader returns part of file blame one by one @@ -43,30 +46,38 @@ func (r *BlameReader) NextPart() (*BlamePart, error) { var blamePart *BlamePart if r.lastSha != nil { - blamePart = &BlamePart{*r.lastSha, make([]string, 0)} + blamePart = &BlamePart{ + Sha: *r.lastSha, + Lines: make([]string, 0), + } } - var line []byte + var lineBytes []byte var isPrefix bool var err error for err != io.EOF { - line, isPrefix, err = r.bufferedReader.ReadLine() + lineBytes, isPrefix, err = r.bufferedReader.ReadLine() if err != nil && err != io.EOF { return blamePart, err } - if len(line) == 0 { + if len(lineBytes) == 0 { // isPrefix will be false continue } - lines := shaLineRegex.FindSubmatch(line) + line := string(lineBytes) + + lines := shaLineRegex.FindStringSubmatch(line) if lines != nil { - sha1 := string(lines[1]) + sha1 := lines[1] if blamePart == nil { - blamePart = &BlamePart{sha1, make([]string, 0)} + blamePart = &BlamePart{ + Sha: sha1, + Lines: make([]string, 0), + } } if blamePart.Sha != sha1 { @@ -81,9 +92,11 @@ func (r *BlameReader) NextPart() (*BlamePart, error) { return blamePart, nil } } else if line[0] == '\t' { - code := line[1:] - - blamePart.Lines = append(blamePart.Lines, string(code)) + blamePart.Lines = append(blamePart.Lines, line[1:]) + } else if strings.HasPrefix(line, "previous ") { + parts := strings.SplitN(line[len("previous "):], " ", 2) + blamePart.PreviousSha = parts[0] + blamePart.PreviousPath = parts[1] } // need to munch to end of line... diff --git a/modules/git/blame_test.go b/modules/git/blame_test.go index 013350ac2..040f4e822 100644 --- a/modules/git/blame_test.go +++ b/modules/git/blame_test.go @@ -24,15 +24,17 @@ func TestReadingBlameOutput(t *testing.T) { parts := []*BlamePart{ { - "72866af952e98d02a73003501836074b286a78f6", - []string{ + Sha: "72866af952e98d02a73003501836074b286a78f6", + Lines: []string{ "# test_repo", "Test repository for testing migration from github to gitea", }, }, { - "f32b0a9dfd09a60f616f29158f772cedd89942d2", - []string{"", "Do not make any changes to this repo it is used for unit testing"}, + Sha: "f32b0a9dfd09a60f616f29158f772cedd89942d2", + Lines: []string{"", "Do not make any changes to this repo it is used for unit testing"}, + PreviousSha: "72866af952e98d02a73003501836074b286a78f6", + PreviousPath: "README.md", }, } @@ -64,16 +66,18 @@ func TestReadingBlameOutput(t *testing.T) { full := []*BlamePart{ { - "af7486bd54cfc39eea97207ca666aa69c9d6df93", - []string{"line", "line"}, + Sha: "af7486bd54cfc39eea97207ca666aa69c9d6df93", + Lines: []string{"line", "line"}, }, { - "45fb6cbc12f970b04eacd5cd4165edd11c8d7376", - []string{"changed line"}, + Sha: "45fb6cbc12f970b04eacd5cd4165edd11c8d7376", + Lines: []string{"changed line"}, + PreviousSha: "af7486bd54cfc39eea97207ca666aa69c9d6df93", + PreviousPath: "blame.txt", }, { - "af7486bd54cfc39eea97207ca666aa69c9d6df93", - []string{"line", "line", ""}, + Sha: "af7486bd54cfc39eea97207ca666aa69c9d6df93", + Lines: []string{"line", "line", ""}, }, } @@ -89,8 +93,8 @@ func TestReadingBlameOutput(t *testing.T) { Bypass: false, Parts: []*BlamePart{ { - "af7486bd54cfc39eea97207ca666aa69c9d6df93", - []string{"line", "line", "changed line", "line", "line", ""}, + Sha: "af7486bd54cfc39eea97207ca666aa69c9d6df93", + Lines: []string{"line", "line", "changed line", "line", "line", ""}, }, }, }, diff --git a/routers/web/repo/blame.go b/routers/web/repo/blame.go index 1f1cca897..52d350ff6 100644 --- a/routers/web/repo/blame.go +++ b/routers/web/repo/blame.go @@ -114,12 +114,12 @@ func RefBlame(ctx *context.Context) { return } - commitNames, previousCommits := processBlameParts(ctx, result.Parts) + commitNames := processBlameParts(ctx, result.Parts) if ctx.Written() { return } - renderBlame(ctx, result.Parts, commitNames, previousCommits) + renderBlame(ctx, result.Parts, commitNames) ctx.HTML(http.StatusOK, tplRepoHome) } @@ -185,12 +185,9 @@ func fillBlameResult(br *git.BlameReader, r *blameResult) error { return nil } -func processBlameParts(ctx *context.Context, blameParts []git.BlamePart) (map[string]*user_model.UserCommit, map[string]string) { +func processBlameParts(ctx *context.Context, blameParts []git.BlamePart) map[string]*user_model.UserCommit { // store commit data by SHA to look up avatar info etc commitNames := make(map[string]*user_model.UserCommit) - // previousCommits contains links from SHA to parent SHA, - // if parent also contains the current TreePath. - previousCommits := make(map[string]string) // and as blameParts can reference the same commits multiple // times, we cache the lookup work locally commits := make([]*git.Commit, 0, len(blameParts)) @@ -214,29 +211,11 @@ func processBlameParts(ctx *context.Context, blameParts []git.BlamePart) (map[st } else { ctx.ServerError("Repo.GitRepo.GetCommit", err) } - return nil, nil + return nil } commitCache[sha] = commit } - // find parent commit - if commit.ParentCount() > 0 { - psha := commit.Parents[0] - previousCommit, ok := commitCache[psha.String()] - if !ok { - previousCommit, _ = commit.Parent(0) - if previousCommit != nil { - commitCache[psha.String()] = previousCommit - } - } - // only store parent commit ONCE, if it has the file - if previousCommit != nil { - if haz1, _ := previousCommit.HasFile(ctx.Repo.TreePath); haz1 { - previousCommits[commit.ID.String()] = previousCommit.ID.String() - } - } - } - commits = append(commits, commit) } @@ -245,10 +224,10 @@ func processBlameParts(ctx *context.Context, blameParts []git.BlamePart) (map[st commitNames[c.ID.String()] = c } - return commitNames, previousCommits + return commitNames } -func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames map[string]*user_model.UserCommit, previousCommits map[string]string) { +func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames map[string]*user_model.UserCommit) { repoLink := ctx.Repo.RepoLink language := "" @@ -295,7 +274,6 @@ func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames m } commit := commitNames[part.Sha] - previousSha := previousCommits[part.Sha] if index == 0 { // Count commit number commitCnt++ @@ -313,8 +291,8 @@ func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames m br.Avatar = gotemplate.HTML(avatar) br.RepoLink = repoLink br.PartSha = part.Sha - br.PreviousSha = previousSha - br.PreviousShaURL = fmt.Sprintf("%s/blame/commit/%s/%s", repoLink, url.PathEscape(previousSha), util.PathEscapeSegments(ctx.Repo.TreePath)) + br.PreviousSha = part.PreviousSha + br.PreviousShaURL = fmt.Sprintf("%s/blame/commit/%s/%s", repoLink, url.PathEscape(part.PreviousSha), util.PathEscapeSegments(part.PreviousPath)) br.CommitURL = fmt.Sprintf("%s/commit/%s", repoLink, url.PathEscape(part.Sha)) br.CommitMessage = commit.CommitMessage br.CommitSince = commitSince