From 9da4642c8c9feca2019c09fba14b0d32b64be47e Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 17 Feb 2023 02:19:24 +0000 Subject: [PATCH] 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 Co-authored-by: Brecht Van Lommel Co-authored-by: Lunny Xiao --- modules/git/blame.go | 40 +++++++++++++++++++-------------------- modules/git/blame_test.go | 6 ++---- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index 832b12213..535710c4c 100644 --- 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 index 4bee8cd27..a37b7a45e 100644 --- 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 @@ -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",