Skip to content

Commit e09aaad

Browse files
bohdebohde
committed
refactor to have a simpler ParsePatch API
1 parent 7f21b3f commit e09aaad

File tree

4 files changed

+20
-27
lines changed

4 files changed

+20
-27
lines changed

‎services/gitdiff/csv_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ c,d,e`,
191191
}
192192

193193
for n, c := range cases {
194-
diff, err := ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(c.diff), "", nil)
194+
diff, err := ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(c.diff), "")
195195
if err != nil {
196196
t.Errorf("ParsePatch failed: %s", err)
197197
}

‎services/gitdiff/gitdiff.go‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ func (diff *Diff) LoadComments(ctx context.Context, issue *issues_model.Issue, c
487487
const cmdDiffHead = "diff --git "
488488

489489
// ParsePatch builds a Diff object from a io.Reader and some parameters.
490-
func ParsePatch(ctx context.Context, maxLines, maxLineCharacters, maxFiles int, reader io.Reader, skipToFile string, cancel context.CancelFunc) (*Diff, error) {
490+
func ParsePatch(ctx context.Context, maxLines, maxLineCharacters, maxFiles int, reader io.Reader, skipToFile string) (*Diff, error) {
491491
log.Debug("ParsePatch(%d, %d, %d, ..., %s)", maxLines, maxLineCharacters, maxFiles, skipToFile)
492492
var curFile *DiffFile
493493

@@ -529,16 +529,6 @@ parsingLoop:
529529
lastFile := createDiffFile(diff, line)
530530
diff.End = lastFile.Name
531531
diff.IsIncomplete = true
532-
533-
// signal that we are exiting this diff early
534-
if cancel != nil {
535-
cancel()
536-
}
537-
_, err := io.Copy(io.Discard, reader)
538-
if err != nil {
539-
// By the definition of io.Copy this never returns io.EOF
540-
return diff, fmt.Errorf("error during io.Copy: %w", err)
541-
}
542532
break parsingLoop
543533
}
544534

@@ -1186,7 +1176,9 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi
11861176
_ = writer.Close()
11871177
}()
11881178

1189-
diff, err := ParsePatch(ctx, opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, reader, parsePatchSkipToFile, cancel)
1179+
diff, err := ParsePatch(ctx, opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, reader, parsePatchSkipToFile)
1180+
// Ensure the git process is killed if it didn't exit already
1181+
cancel()
11901182
if err != nil {
11911183
return nil, fmt.Errorf("unable to ParsePatch: %w", err)
11921184
}
@@ -1352,7 +1344,7 @@ outer:
13521344
// CommentAsDiff returns c.Patch as *Diff
13531345
func CommentAsDiff(ctx context.Context, c *issues_model.Comment) (*Diff, error) {
13541346
diff, err := ParsePatch(ctx, setting.Git.MaxGitDiffLines,
1355-
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(c.Patch), "", nil)
1347+
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(c.Patch), "")
13561348
if err != nil {
13571349
log.Error("Unable to parse patch: %v", err)
13581350
return nil, err

‎services/gitdiff/gitdiff_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ diff --git "\\a/README.md" "\\b/README.md"
175175
}
176176
for _, testcase := range tests {
177177
t.Run(testcase.name, func(t *testing.T) {
178-
got, err := ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), testcase.skipTo, nil)
178+
got, err := ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), testcase.skipTo)
179179
if (err != nil) != testcase.wantErr {
180180
t.Errorf("ParsePatch(%q) error = %v, wantErr %v", testcase.name, err, testcase.wantErr)
181181
return
@@ -400,7 +400,7 @@ index 6961180..9ba1a00 100644
400400

401401
for _, testcase := range tests {
402402
t.Run(testcase.name, func(t *testing.T) {
403-
got, err := ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), "", nil)
403+
got, err := ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), "")
404404
if (err != nil) != testcase.wantErr {
405405
t.Errorf("ParsePatch(%q) error = %v, wantErr %v", testcase.name, err, testcase.wantErr)
406406
return
@@ -449,21 +449,21 @@ index 0000000..6bb8f39
449449
diffBuilder.WriteString("+line" + strconv.Itoa(i) + "\n")
450450
}
451451
diff = diffBuilder.String()
452-
result, err := ParsePatch(db.DefaultContext, 20, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "", nil)
452+
result, err := ParsePatch(db.DefaultContext, 20, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "")
453453
if err != nil {
454454
t.Errorf("There should not be an error: %v", err)
455455
}
456456
if !result.Files[0].IsIncomplete {
457457
t.Errorf("Files should be incomplete! %v", result.Files[0])
458458
}
459-
result, err = ParsePatch(db.DefaultContext, 40, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "", nil)
459+
result, err = ParsePatch(db.DefaultContext, 40, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "")
460460
if err != nil {
461461
t.Errorf("There should not be an error: %v", err)
462462
}
463463
if result.Files[0].IsIncomplete {
464464
t.Errorf("Files should not be incomplete! %v", result.Files[0])
465465
}
466-
result, err = ParsePatch(db.DefaultContext, 40, 5, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "", nil)
466+
result, err = ParsePatch(db.DefaultContext, 40, 5, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "")
467467
if err != nil {
468468
t.Errorf("There should not be an error: %v", err)
469469
}
@@ -494,14 +494,14 @@ index 0000000..6bb8f39
494494
diffBuilder.WriteString("+line" + strconv.Itoa(35) + "\n")
495495
diff = diffBuilder.String()
496496

497-
result, err = ParsePatch(db.DefaultContext, 20, 4096, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "", nil)
497+
result, err = ParsePatch(db.DefaultContext, 20, 4096, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "")
498498
if err != nil {
499499
t.Errorf("There should not be an error: %v", err)
500500
}
501501
if !result.Files[0].IsIncomplete {
502502
t.Errorf("Files should be incomplete! %v", result.Files[0])
503503
}
504-
result, err = ParsePatch(db.DefaultContext, 40, 4096, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "", nil)
504+
result, err = ParsePatch(db.DefaultContext, 40, 4096, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "")
505505
if err != nil {
506506
t.Errorf("There should not be an error: %v", err)
507507
}
@@ -520,7 +520,7 @@ index 0000000..6bb8f39
520520
Docker Pulls
521521
+ cut off
522522
+ cut off`
523-
_, err = ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "", nil)
523+
_, err = ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff), "")
524524
if err != nil {
525525
t.Errorf("ParsePatch failed: %s", err)
526526
}
@@ -536,7 +536,7 @@ index 0000000..6bb8f39
536536
Docker Pulls
537537
+ cut off
538538
+ cut off`
539-
_, err = ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff2), "", nil)
539+
_, err = ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff2), "")
540540
if err != nil {
541541
t.Errorf("ParsePatch failed: %s", err)
542542
}
@@ -552,7 +552,7 @@ index 0000000..6bb8f39
552552
Docker Pulls
553553
+ cut off
554554
+ cut off`
555-
_, err = ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff2a), "", nil)
555+
_, err = ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff2a), "")
556556
if err != nil {
557557
t.Errorf("ParsePatch failed: %s", err)
558558
}
@@ -568,7 +568,7 @@ index 0000000..6bb8f39
568568
Docker Pulls
569569
+ cut off
570570
+ cut off`
571-
_, err = ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff3), "", nil)
571+
_, err = ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(diff3), "")
572572
if err != nil {
573573
t.Errorf("ParsePatch failed: %s", err)
574574
}
@@ -665,6 +665,6 @@ func TestNoCrashes(t *testing.T) {
665665
}
666666
for _, testcase := range tests {
667667
// It shouldn't crash, so don't care about the output.
668-
ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), "", nil)
668+
ParsePatch(db.DefaultContext, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, strings.NewReader(testcase.gitdiff), "")
669669
}
670670
}

‎services/repository/files/temp_repo.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,8 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) {
358358
Stderr: stderr,
359359
PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error {
360360
_ = stdoutWriter.Close()
361-
diff, finalErr = gitdiff.ParsePatch(t.ctx, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader, "", cancel)
361+
defer cancel()
362+
diff, finalErr = gitdiff.ParsePatch(t.ctx, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader, "")
362363
if finalErr != nil {
363364
log.Error("ParsePatch: %v", finalErr)
364365
cancel()

0 commit comments

Comments
 (0)