Fix blame view missing lines (#22826) (#22929)

Backport #22826 Creating a new buffered reader for every part of the blame can miss lines, as it will read and buffer bytes that the next buffered reader will not get. --------- Signed-off-by: Andrew Thornton <[email protected]> Co-authored-by: Brecht Van Lommel <[email protected]> Co-authored-by: Lunny Xiao <[email protected]>
go-gitea · Feb 17, 2023 · 9da4642 · 9da4642
1 parent 1d191f9
commit 9da4642
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 24 deletions.
diff --git a/modules/git/blame.go b/modules/git/blame.go
@@ -24,12 +24,12 @@ type BlamePart struct {
 
 // BlameReader returns part of file blame one by one
 type BlameReader struct {
-	cmd      *exec.Cmd
-	output   io.ReadCloser
-	reader   *bufio.Reader
-	lastSha  *string
-	cancel   context.CancelFunc   // Cancels the context that this reader runs in
-	finished process.FinishedFunc // Tells the process manager we're finished and it can remove the associated process from the process table
+	cmd            *exec.Cmd
+	reader         io.ReadCloser
+	lastSha        *string
+	cancel         context.CancelFunc   // Cancels the context that this reader runs in
+	finished       process.FinishedFunc // Tells the process manager we're finished and it can remove the associated process from the process table
+	bufferedReader *bufio.Reader
 }
 
 var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})")
@@ -38,8 +38,6 @@ var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})")
 func (r *BlameReader) NextPart() (*BlamePart, error) {
 	var blamePart *BlamePart
 
-	reader := r.reader
-
 	if r.lastSha != nil {
 		blamePart = &BlamePart{*r.lastSha, make([]string, 0)}
 	}
@@ -49,7 +47,7 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
 	var err error
 
 	for err != io.EOF {
-		line, isPrefix, err = reader.ReadLine()
+		line, isPrefix, err = r.bufferedReader.ReadLine()
 		if err != nil && err != io.EOF {
 			return blamePart, err
 		}
@@ -71,7 +69,7 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
 				r.lastSha = &sha1
 				// need to munch to end of line...
 				for isPrefix {
-					_, isPrefix, err = reader.ReadLine()
+					_, isPrefix, err = r.bufferedReader.ReadLine()
 					if err != nil && err != io.EOF {
 						return blamePart, err
 					}
@@ -86,7 +84,7 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
 
 		// need to munch to end of line...
 		for isPrefix {
-			_, isPrefix, err = reader.ReadLine()
+			_, isPrefix, err = r.bufferedReader.ReadLine()
 			if err != nil && err != io.EOF {
 				return blamePart, err
 			}
@@ -102,9 +100,9 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
 func (r *BlameReader) Close() error {
 	defer r.finished() // Only remove the process from the process table when the underlying command is closed
 	r.cancel()         // However, first cancel our own context early
+	r.bufferedReader = nil
 
-	_ = r.output.Close()
-
+	_ = r.reader.Close()
 	if err := r.cmd.Wait(); err != nil {
 		return fmt.Errorf("Wait: %w", err)
 	}
@@ -126,25 +124,27 @@ func createBlameReader(ctx context.Context, dir string, command ...string) (*Bla
 	cmd.Stderr = os.Stderr
 	process.SetSysProcAttribute(cmd)
 
-	stdout, err := cmd.StdoutPipe()
+	reader, stdout, err := os.Pipe()
 	if err != nil {
 		defer finished()
 		return nil, fmt.Errorf("StdoutPipe: %w", err)
 	}
+	cmd.Stdout = stdout
 
 	if err = cmd.Start(); err != nil {
 		defer finished()
 		_ = stdout.Close()
 		return nil, fmt.Errorf("Start: %w", err)
 	}
+	_ = stdout.Close()
 
-	reader := bufio.NewReader(stdout)
+	bufferedReader := bufio.NewReader(reader)
 
 	return &BlameReader{
-		cmd:      cmd,
-		output:   stdout,
-		reader:   reader,
-		cancel:   cancel,
-		finished: finished,
+		cmd:            cmd,
+		reader:         reader,
+		cancel:         cancel,
+		finished:       finished,
+		bufferedReader: bufferedReader,
 	}, nil
 }
diff --git a/modules/git/blame_test.go b/modules/git/blame_test.go
@@ -65,7 +65,7 @@ summary Add code of delete user
 previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go
 filename gogs.go
 	// license that can be found in the LICENSE file.
-	
+	` + `
 e2aa991e10ffd924a828ec149951f2f20eecead2 6 6 2
 author Lunny Xiao
 author-mail <[email protected]>
@@ -112,9 +112,7 @@ func TestReadingBlameOutput(t *testing.T) {
 		},
 		{
 			"ce21ed6c3490cdfad797319cbb1145e2330a8fef",
-			[]string{
-				"// Copyright 2016 The Gitea Authors. All rights reserved.",
-			},
+			[]string{"// Copyright 2016 The Gitea Authors. All rights reserved."},
 		},
 		{
 			"4b92a6c2df28054ad766bc262f308db9f6066596",