Read previous info from git blame (#28306) (#28310)

Backport #28306 by @KN4CK3R

Fixes #28280

Reads the `previous` info from the `git blame` output instead of
calculating it afterwards.

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>

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"
@@ -20,6 +21,8 @@ import (
 type BlamePart struct {
 	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, line[1:])
-
+		} else if strings.HasPrefix(line, "previous ") {
-			blamePart.Lines = append(blamePart.Lines, string(code))
+			parts := strings.SplitN(line[len("previous "):], " ", 2)
+			blamePart.PreviousSha = parts[0]
+			blamePart.PreviousPath = parts[1]
 		}
 
 		// need to munch to end of line...

modules/git/blame_test.go

@@ -24,15 +24,17 @@ func TestReadingBlameOutput(t *testing.T) {
 
 		parts := []*BlamePart{
 			{
-				"72866af952e98d02a73003501836074b286a78f6",
+				Sha: "72866af952e98d02a73003501836074b286a78f6",
-				[]string{
+				Lines: []string{
 					"# test_repo",
 					"Test repository for testing migration from github to gitea",
 				},
 			},
 			{
-				"f32b0a9dfd09a60f616f29158f772cedd89942d2",
+				Sha:          "f32b0a9dfd09a60f616f29158f772cedd89942d2",
-				[]string{"", "Do not make any changes to this repo it is used for unit testing"},
+				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",
+				Sha:   "af7486bd54cfc39eea97207ca666aa69c9d6df93",
-				[]string{"line", "line"},
+				Lines: []string{"line", "line"},
 			},
 			{
-				"45fb6cbc12f970b04eacd5cd4165edd11c8d7376",
+				Sha:          "45fb6cbc12f970b04eacd5cd4165edd11c8d7376",
-				[]string{"changed line"},
+				Lines:        []string{"changed line"},
+				PreviousSha:  "af7486bd54cfc39eea97207ca666aa69c9d6df93",
+				PreviousPath: "blame.txt",
 			},
 			{
-				"af7486bd54cfc39eea97207ca666aa69c9d6df93",
+				Sha:   "af7486bd54cfc39eea97207ca666aa69c9d6df93",
-				[]string{"line", "line", ""},
+				Lines: []string{"line", "line", ""},
 			},
 		}
 
@@ -89,8 +93,8 @@ func TestReadingBlameOutput(t *testing.T) {
 				Bypass:         false,
 				Parts: []*BlamePart{
 					{
-						"af7486bd54cfc39eea97207ca666aa69c9d6df93",
+						Sha:   "af7486bd54cfc39eea97207ca666aa69c9d6df93",
-						[]string{"line", "line", "changed line", "line", "line", ""},
+						Lines: []string{"line", "line", "changed line", "line", "line", ""},
 					},
 				},
 			},

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.PreviousSha = part.PreviousSha
-				br.PreviousShaURL = fmt.Sprintf("%s/blame/commit/%s/%s", repoLink, url.PathEscape(previousSha), util.PathEscapeSegments(ctx.Repo.TreePath))
+				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