From 71c864a9887488b84c666b2580f549dc5eb5e2cb Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Thu, 9 Feb 2023 04:51:02 +0100 Subject: [PATCH 1/3] Fix blame view missing lines (#22826) 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. Co-authored-by: Lunny Xiao --- modules/git/blame.go | 40 +++++++++++++++++++-------------------- modules/git/blame_test.go | 8 +++----- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index 832b12213c889..535710c4cd70b 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 4bee8cd27a962..f1c0ec0ccc502 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 @@ -111,10 +111,8 @@ func TestReadingBlameOutput(t *testing.T) { }, }, { - "ce21ed6c3490cdfad797319cbb1145e2330a8fef", - []string{ - "// Copyright 2016 The Gitea Authors. All rights reserved.", - }, + "f32b0a9dfd09a60f616f29158f772cedd89942d2", + []string{"", "Do not make any changes to this repo it is used for unit testing"}, }, { "4b92a6c2df28054ad766bc262f308db9f6066596", From cf33e56a40bdf7856c0dbed6c0edaafa259445e1 Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 15 Feb 2023 23:12:13 +0000 Subject: [PATCH 2/3] Update modules/git/blame_test.go --- modules/git/blame_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/git/blame_test.go b/modules/git/blame_test.go index f1c0ec0ccc502..51d1c79a33298 100644 --- a/modules/git/blame_test.go +++ b/modules/git/blame_test.go @@ -111,8 +111,8 @@ func TestReadingBlameOutput(t *testing.T) { }, }, { - "f32b0a9dfd09a60f616f29158f772cedd89942d2", - []string{"", "Do not make any changes to this repo it is used for unit testing"}, + "ce21ed6c3490cdfad797319cbb1145e2330a8fef", + []string{"// Copyright 2016 The Gitea Authors. All rights reserved."}, }, { "4b92a6c2df28054ad766bc262f308db9f6066596", From cae08c08de505e8d0c701c727db8b721fd9cb794 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 16 Feb 2023 21:25:22 +0000 Subject: [PATCH 3/3] fix missing indentation Signed-off-by: Andrew Thornton --- modules/git/blame_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/blame_test.go b/modules/git/blame_test.go index 51d1c79a33298..a37b7a45ea520 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