egtyl/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source 
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 <art27@cantab.net>
Co-authored-by: Brecht Van Lommel <brecht@blender.org>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
This commit is contained in:
zeripath 2023-02-17 02:19:24 +00:00 committed by GitHub
parent 1d191f9b5a
commit 9da4642c8c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 22 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

40
modules/git/blame.go
View file

@ -24,12 +24,12 @@ type BlamePart struct {
// BlameReader returns part of file blame one by one // BlameReader returns part of file blame one by one
type BlameReader struct { type BlameReader struct {
cmd *exec.Cmd cmd *exec.Cmd
output io.ReadCloser reader io.ReadCloser
reader *bufio.Reader lastSha *string
lastSha *string cancel context.CancelFunc // Cancels the context that this reader runs in
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
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})") 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) { func (r *BlameReader) NextPart() (*BlamePart, error) {
var blamePart *BlamePart var blamePart *BlamePart
reader := r.reader
if r.lastSha != nil { if r.lastSha != nil {
blamePart = &BlamePart{*r.lastSha, make([]string, 0)} blamePart = &BlamePart{*r.lastSha, make([]string, 0)}
} }
@ -49,7 +47,7 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
var err error var err error
for err != io.EOF { for err != io.EOF {
line, isPrefix, err = reader.ReadLine() line, isPrefix, err = r.bufferedReader.ReadLine()
if err != nil && err != io.EOF { if err != nil && err != io.EOF {
return blamePart, err return blamePart, err
} }
@ -71,7 +69,7 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
r.lastSha = &sha1 r.lastSha = &sha1
// need to munch to end of line... // need to munch to end of line...
for isPrefix { for isPrefix {
_, isPrefix, err = reader.ReadLine() _, isPrefix, err = r.bufferedReader.ReadLine()
if err != nil && err != io.EOF { if err != nil && err != io.EOF {
return blamePart, err return blamePart, err
} }
@ -86,7 +84,7 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
// need to munch to end of line... // need to munch to end of line...
for isPrefix { for isPrefix {
_, isPrefix, err = reader.ReadLine() _, isPrefix, err = r.bufferedReader.ReadLine()
if err != nil && err != io.EOF { if err != nil && err != io.EOF {
return blamePart, err return blamePart, err
} }
@ -102,9 +100,9 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
func (r *BlameReader) Close() error { func (r *BlameReader) Close() error {
defer r.finished() // Only remove the process from the process table when the underlying command is closed 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.cancel() // However, first cancel our own context early
r.bufferedReader = nil
_ = r.output.Close() _ = r.reader.Close()
if err := r.cmd.Wait(); err != nil { if err := r.cmd.Wait(); err != nil {
return fmt.Errorf("Wait: %w", err) return fmt.Errorf("Wait: %w", err)
} }
@ -126,25 +124,27 @@ func createBlameReader(ctx context.Context, dir string, command ...string) (*Bla
cmd.Stderr = os.Stderr cmd.Stderr = os.Stderr
process.SetSysProcAttribute(cmd) process.SetSysProcAttribute(cmd)
stdout, err := cmd.StdoutPipe() reader, stdout, err := os.Pipe()
if err != nil { if err != nil {
defer finished() defer finished()
return nil, fmt.Errorf("StdoutPipe: %w", err) return nil, fmt.Errorf("StdoutPipe: %w", err)
} }
cmd.Stdout = stdout
if err = cmd.Start(); err != nil { if err = cmd.Start(); err != nil {
defer finished() defer finished()
_ = stdout.Close() _ = stdout.Close()
return nil, fmt.Errorf("Start: %w", err) return nil, fmt.Errorf("Start: %w", err)
} }
_ = stdout.Close()
reader := bufio.NewReader(stdout) bufferedReader := bufio.NewReader(reader)
return &BlameReader{ return &BlameReader{
cmd: cmd, cmd: cmd,
output: stdout, reader: reader,
reader: reader, cancel: cancel,
cancel: cancel, finished: finished,
finished: finished, bufferedReader: bufferedReader,
}, nil }, nil
} }

6
modules/git/blame_test.go
View file

@ -65,7 +65,7 @@ summary Add code of delete user
previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go previous be0ba9ea88aff8a658d0495d36accf944b74888d gogs.go
filename gogs.go filename gogs.go
// license that can be found in the LICENSE file. // license that can be found in the LICENSE file.
` + `
e2aa991e10ffd924a828ec149951f2f20eecead2 6 6 2 e2aa991e10ffd924a828ec149951f2f20eecead2 6 6 2
author Lunny Xiao author Lunny Xiao
author-mail <xiaolunwen@gmail.com> author-mail <xiaolunwen@gmail.com>
@ -112,9 +112,7 @@ func TestReadingBlameOutput(t *testing.T) {
}, },
{ {
"ce21ed6c3490cdfad797319cbb1145e2330a8fef", "ce21ed6c3490cdfad797319cbb1145e2330a8fef",
[]string{ []string{"// Copyright 2016 The Gitea Authors. All rights reserved."},
"// Copyright 2016 The Gitea Authors. All rights reserved.",
},
}, },
{ {
"4b92a6c2df28054ad766bc262f308db9f6066596", "4b92a6c2df28054ad766bc262f308db9f6066596",