Skip to content

Commit 18824a5

Browse files
zeripathzeripath
committed
Change diff to use go-routine reader
Signed-off-by: Andrew Thornton <[email protected]>
1 parent 023c465 commit 18824a5

File tree

5 files changed

+46
-26
lines changed

5 files changed

+46
-26
lines changed

‎modules/git/diff.go

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,23 @@ const (
2828
)
2929

3030
// GetRawDiff dumps diff results of repository in given commit ID to io.Writer.
31-
func GetRawDiff(repoPath, commitID string, diffType RawDiffType, writer io.Writer) error {
32-
return GetRawDiffForFile(repoPath, "", commitID, diffType, "", writer)
31+
func GetRawDiff(ctx context.Context, repoPath, commitID string, diffType RawDiffType, writer io.Writer) error {
32+
return GetRawDiffForFile(ctx, repoPath, "", commitID, diffType, "", writer)
3333
}
3434

3535
// GetRawDiffForFile dumps diff results of file in given commit ID to io.Writer.
36-
func GetRawDiffForFile(repoPath, startCommit, endCommit string, diffType RawDiffType, file string, writer io.Writer) error {
36+
func GetRawDiffForFile(ctx context.Context, repoPath, startCommit, endCommit string, diffType RawDiffType, file string, writer io.Writer) error {
3737
repo, err := OpenRepository(repoPath)
3838
if err != nil {
3939
return fmt.Errorf("OpenRepository: %v", err)
4040
}
4141
defer repo.Close()
4242

43-
return GetRepoRawDiffForFile(repo, startCommit, endCommit, diffType, file, writer)
43+
return GetRepoRawDiffForFile(ctx, repo, startCommit, endCommit, diffType, file, writer)
4444
}
4545

4646
// GetRepoRawDiffForFile dumps diff results of file in given commit ID to io.Writer according given repository
47-
func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diffType RawDiffType, file string, writer io.Writer) error {
47+
func GetRepoRawDiffForFile(ctx context.Context, repo *Repository, startCommit, endCommit string, diffType RawDiffType, file string, writer io.Writer) error {
4848
commit, err := repo.GetCommit(endCommit)
4949
if err != nil {
5050
return fmt.Errorf("GetCommit: %v", err)
@@ -54,7 +54,7 @@ func GetRepoRawDiffForFile(repo *Repository, startCommit, endCommit string, diff
5454
fileArgs = append(fileArgs, "--", file)
5555
}
5656
// FIXME: graceful: These commands should have a timeout
57-
ctx, cancel := context.WithCancel(DefaultContext)
57+
ctx, cancel := context.WithCancel(ctx)
5858
defer cancel()
5959

6060
var cmd *exec.Cmd
@@ -132,10 +132,10 @@ func isHeader(lof string) bool {
132132
// CutDiffAroundLine cuts a diff of a file in way that only the given line + numberOfLine above it will be shown
133133
// it also recalculates hunks and adds the appropriate headers to the new diff.
134134
// Warning: Only one-file diffs are allowed.
135-
func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLine int) string {
135+
func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLine int) (string, error) {
136136
if line == 0 || numbersOfLine == 0 {
137137
// no line or num of lines => no diff
138-
return ""
138+
return "", nil
139139
}
140140
scanner := bufio.NewScanner(originalDiff)
141141
hunk := make([]string, 0)
@@ -213,15 +213,18 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
213213
}
214214
}
215215
}
216+
if err := scanner.Err(); err != nil {
217+
return "", err
218+
}
216219

217220
// No hunk found
218221
if currentLine == 0 {
219-
return ""
222+
return "", nil
220223
}
221224
// headerLines + hunkLine (1) = totalNonCodeLines
222225
if len(hunk)-headerLines-1 <= numbersOfLine {
223226
// No need to cut the hunk => return existing hunk
224-
return strings.Join(hunk, "\n")
227+
return strings.Join(hunk, "\n"), nil
225228
}
226229
var oldBegin, oldNumOfLines, newBegin, newNumOfLines int64
227230
if old {
@@ -256,5 +259,5 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
256259
// construct the new hunk header
257260
newHunk[headerLines] = fmt.Sprintf("@@ -%d,%d +%d,%d @@",
258261
oldBegin, oldNumOfLines, newBegin, newNumOfLines)
259-
return strings.Join(newHunk, "\n")
262+
return strings.Join(newHunk, "\n"), nil
260263
}

‎modules/git/diff_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const exampleDiff = `diff --git a/README.md b/README.md
2424
+ cut off`
2525

2626
func TestCutDiffAroundLine(t *testing.T) {
27-
result := CutDiffAroundLine(strings.NewReader(exampleDiff), 4, false, 3)
27+
result, _ := CutDiffAroundLine(strings.NewReader(exampleDiff), 4, false, 3)
2828
resultByLine := strings.Split(result, "\n")
2929
assert.Len(t, resultByLine, 7)
3030
// Check if headers got transferred
@@ -37,17 +37,17 @@ func TestCutDiffAroundLine(t *testing.T) {
3737
assert.Equal(t, "+ Build Status", resultByLine[4])
3838

3939
// Must be same result as before since old line 3 == new line 5
40-
newResult := CutDiffAroundLine(strings.NewReader(exampleDiff), 3, true, 3)
40+
newResult, _ := CutDiffAroundLine(strings.NewReader(exampleDiff), 3, true, 3)
4141
assert.Equal(t, result, newResult, "Must be same result as before since old line 3 == new line 5")
4242

43-
newResult = CutDiffAroundLine(strings.NewReader(exampleDiff), 6, false, 300)
43+
newResult, _ = CutDiffAroundLine(strings.NewReader(exampleDiff), 6, false, 300)
4444
assert.Equal(t, exampleDiff, newResult)
4545

46-
emptyResult := CutDiffAroundLine(strings.NewReader(exampleDiff), 6, false, 0)
46+
emptyResult, _ := CutDiffAroundLine(strings.NewReader(exampleDiff), 6, false, 0)
4747
assert.Empty(t, emptyResult)
4848

4949
// Line is out of scope
50-
emptyResult = CutDiffAroundLine(strings.NewReader(exampleDiff), 434, false, 0)
50+
emptyResult, _ = CutDiffAroundLine(strings.NewReader(exampleDiff), 434, false, 0)
5151
assert.Empty(t, emptyResult)
5252
}
5353

@@ -69,7 +69,7 @@ func ExampleCutDiffAroundLine() {
6969
Docker Pulls
7070
+ cut off
7171
+ cut off`
72-
result := CutDiffAroundLine(strings.NewReader(diff), 4, false, 3)
72+
result, _ := CutDiffAroundLine(strings.NewReader(diff), 4, false, 3)
7373
println(result)
7474
}
7575

‎modules/migrations/gitea.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
package migrations
77

88
import (
9-
"bytes"
109
"context"
1110
"fmt"
1211
"io"
@@ -795,13 +794,20 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
795794
return fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
796795
}
797796

797+
preader, pwriter := io.Pipe()
798+
ctx, cancel := context.WithCancel(git.DefaultContext)
799+
798800
var patch string
799-
patchBuf := new(bytes.Buffer)
800-
if err := git.GetRepoRawDiffForFile(g.gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, comment.TreePath, patchBuf); err != nil {
801+
go func() {
802+
err := git.GetRepoRawDiffForFile(ctx, g.gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, comment.TreePath, pwriter)
803+
_ = pwriter.CloseWithError(err)
804+
}()
805+
patch, err = git.CutDiffAroundLine(preader, int64((&models.Comment{Line: int64(line + comment.Position - 1)}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
806+
_ = preader.Close()
807+
cancel()
808+
if err != nil {
801809
// We should ignore the error since the commit maybe removed when force push to the pull request
802810
log.Warn("GetRepoRawDiffForFile failed when migrating [%s, %s, %s, %s]: %v", g.gitRepo.Path, pr.MergeBase, headCommitID, comment.TreePath, err)
803-
} else {
804-
patch = git.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: int64(line + comment.Position - 1)}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
805811
}
806812

807813
var c = models.Comment{

‎routers/repo/commit.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,7 @@ func RawDiff(ctx *context.Context) {
329329
repoPath = models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
330330
}
331331
if err := git.GetRawDiff(
332+
ctx.Req.Context(),
332333
repoPath,
333334
ctx.Params(":sha"),
334335
git.RawDiffType(ctx.Params(":ext")),

‎services/pull/review.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
package pull
77

88
import (
9-
"bytes"
9+
"context"
1010
"fmt"
11+
"io"
1112
"strings"
1213

1314
"code.gitea.io/gitea/models"
@@ -138,11 +139,20 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models
138139
if err != nil {
139140
return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err)
140141
}
141-
patchBuf := new(bytes.Buffer)
142-
if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, patchBuf); err != nil {
142+
143+
ctx, cancel := context.WithCancel(git.DefaultContext)
144+
preader, pwriter := io.Pipe()
145+
go func() {
146+
err = git.GetRepoRawDiffForFile(ctx, gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, pwriter)
147+
_ = pwriter.CloseWithError(err)
148+
}()
149+
150+
patch, err = git.CutDiffAroundLine(preader, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
151+
_ = preader.Close()
152+
cancel()
153+
if err != nil {
143154
return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath)
144155
}
145-
patch = git.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines)
146156
}
147157
return models.CreateComment(&models.CreateCommentOptions{
148158
Type: models.CommentTypeCode,

0 commit comments

Comments
 (0)