Skip to content

Commit 8511eec

Browse files
zeripathzeripath
authored
Allow Loading of Diffs that are too large (#17739)
* Allow Loading of Diffs that are too large This PR allows the loading of diffs that are suppressed because the file is too large. It does not handle diffs of files which have lines which are too long. Fix #17738 Signed-off-by: Andrew Thornton <[email protected]>
1 parent d710af6 commit 8511eec

File tree

8 files changed

+146
-62
lines changed

8 files changed

+146
-62
lines changed

‎options/locale/locale_en-US.ini‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2043,6 +2043,7 @@ diff.file_suppressed = File diff suppressed because it is too large
20432043
diff.file_suppressed_line_too_long = File diff suppressed because one or more lines are too long
20442044
diff.too_many_files = Some files were not shown because too many files have changed in this diff
20452045
diff.show_more = Show More
2046+
diff.load = Load Diff
20462047
diff.generated = generated
20472048
diff.vendored = vendored
20482049
diff.comment.placeholder = Leave a comment

‎routers/web/repo/commit.go‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,6 @@ func Diff(ctx *context.Context) {
262262
err error
263263
)
264264

265-
fileOnly := ctx.FormBool("file-only")
266-
267265
if ctx.Data["PageIsWiki"] != nil {
268266
gitRepo, err = git.OpenRepository(ctx.Repo.Repository.WikiPath())
269267
if err != nil {
@@ -288,13 +286,23 @@ func Diff(ctx *context.Context) {
288286
commitID = commit.ID.String()
289287
}
290288

291-
diff, err := gitdiff.GetDiffCommitWithWhitespaceBehavior(gitRepo,
292-
commitID, ctx.FormString("skip-to"), setting.Git.MaxGitDiffLines,
293-
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
294-
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
295-
false)
289+
fileOnly := ctx.FormBool("file-only")
290+
maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles
291+
files := ctx.FormStrings("files")
292+
if fileOnly && (len(files) == 2 || len(files) == 1) {
293+
maxLines, maxFiles = -1, -1
294+
}
295+
296+
diff, err := gitdiff.GetDiff(gitRepo, &gitdiff.DiffOptions{
297+
AfterCommitID: commitID,
298+
SkipTo: ctx.FormString("skip-to"),
299+
MaxLines: maxLines,
300+
MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters,
301+
MaxFiles: maxFiles,
302+
WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
303+
}, files...)
296304
if err != nil {
297-
ctx.NotFound("GetDiffCommitWithWhitespaceBehavior", err)
305+
ctx.NotFound("GetDiff", err)
298306
return
299307
}
300308

@@ -325,10 +333,6 @@ func Diff(ctx *context.Context) {
325333
ctx.Data["Title"] = commit.Summary() + " · " + base.ShortSha(commitID)
326334
ctx.Data["Commit"] = commit
327335
ctx.Data["Diff"] = diff
328-
if fileOnly {
329-
ctx.HTML(http.StatusOK, tplDiffBox)
330-
return
331-
}
332336

333337
statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, commitID, db.ListOptions{})
334338
if err != nil {

‎routers/web/repo/compare.go‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -569,9 +569,23 @@ func PrepareCompareDiff(
569569
beforeCommitID = ci.CompareInfo.BaseCommitID
570570
}
571571

572-
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(ci.HeadGitRepo,
573-
beforeCommitID, headCommitID, ctx.FormString("skip-to"), setting.Git.MaxGitDiffLines,
574-
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, whitespaceBehavior, ci.DirectComparison)
572+
maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles
573+
files := ctx.FormStrings("files")
574+
if len(files) == 2 || len(files) == 1 {
575+
maxLines, maxFiles = -1, -1
576+
}
577+
578+
diff, err := gitdiff.GetDiff(ci.HeadGitRepo,
579+
&gitdiff.DiffOptions{
580+
BeforeCommitID: beforeCommitID,
581+
AfterCommitID: headCommitID,
582+
SkipTo: ctx.FormString("skip-to"),
583+
MaxLines: maxLines,
584+
MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters,
585+
MaxFiles: maxFiles,
586+
WhitespaceBehavior: whitespaceBehavior,
587+
DirectComparison: ci.DirectComparison,
588+
}, ctx.FormStrings("files")...)
575589
if err != nil {
576590
ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)
577591
return false

‎routers/web/repo/pull.go‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -633,10 +633,24 @@ func ViewPullFiles(ctx *context.Context) {
633633
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
634634
ctx.Data["AfterCommitID"] = endCommitID
635635

636-
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(gitRepo,
637-
startCommitID, endCommitID, ctx.FormString("skip-to"), setting.Git.MaxGitDiffLines,
638-
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
639-
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), false)
636+
fileOnly := ctx.FormBool("file-only")
637+
638+
maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles
639+
files := ctx.FormStrings("files")
640+
if fileOnly && (len(files) == 2 || len(files) == 1) {
641+
maxLines, maxFiles = -1, -1
642+
}
643+
644+
diff, err := gitdiff.GetDiff(gitRepo,
645+
&gitdiff.DiffOptions{
646+
BeforeCommitID: startCommitID,
647+
AfterCommitID: endCommitID,
648+
SkipTo: ctx.FormString("skip-to"),
649+
MaxLines: maxLines,
650+
MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters,
651+
MaxFiles: maxFiles,
652+
WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)),
653+
}, ctx.FormStrings("files")...)
640654
if err != nil {
641655
ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)
642656
return

‎services/gitdiff/gitdiff.go‎

Lines changed: 56 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,7 @@ const cmdDiffHead = "diff --git "
695695

696696
// ParsePatch builds a Diff object from a io.Reader and some parameters.
697697
func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader, skipToFile string) (*Diff, error) {
698+
log.Debug("ParsePatch(%d, %d, %d, ..., %s)", maxLines, maxLineCharacters, maxFiles, skipToFile)
698699
var curFile *DiffFile
699700

700701
skipping := skipToFile != ""
@@ -726,7 +727,7 @@ parsingLoop:
726727
return diff, fmt.Errorf("invalid first file line: %s", line)
727728
}
728729

729-
if len(diff.Files) >= maxFiles {
730+
if maxFiles > -1 && len(diff.Files) >= maxFiles {
730731
lastFile := createDiffFile(diff, line)
731732
diff.End = lastFile.Name
732733
diff.IsIncomplete = true
@@ -1038,7 +1039,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio
10381039

10391040
switch lineBytes[0] {
10401041
case '@':
1041-
if curFileLinesCount >= maxLines {
1042+
if maxLines > -1 && curFileLinesCount >= maxLines {
10421043
curFile.IsIncomplete = true
10431044
continue
10441045
}
@@ -1075,7 +1076,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio
10751076
rightLine = lineSectionInfo.RightIdx
10761077
continue
10771078
case '\\':
1078-
if curFileLinesCount >= maxLines {
1079+
if maxLines > -1 && curFileLinesCount >= maxLines {
10791080
curFile.IsIncomplete = true
10801081
continue
10811082
}
@@ -1090,7 +1091,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio
10901091
case '+':
10911092
curFileLinesCount++
10921093
curFile.Addition++
1093-
if curFileLinesCount >= maxLines {
1094+
if maxLines > -1 && curFileLinesCount >= maxLines {
10941095
curFile.IsIncomplete = true
10951096
continue
10961097
}
@@ -1114,7 +1115,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio
11141115
case '-':
11151116
curFileLinesCount++
11161117
curFile.Deletion++
1117-
if curFileLinesCount >= maxLines {
1118+
if maxLines > -1 && curFileLinesCount >= maxLines {
11181119
curFile.IsIncomplete = true
11191120
continue
11201121
}
@@ -1134,7 +1135,7 @@ func parseHunks(curFile *DiffFile, maxLines, maxLineCharacters int, input *bufio
11341135
curSection.Lines = append(curSection.Lines, diffLine)
11351136
case ' ':
11361137
curFileLinesCount++
1137-
if curFileLinesCount >= maxLines {
1138+
if maxLines > -1 && curFileLinesCount >= maxLines {
11381139
curFile.IsIncomplete = true
11391140
continue
11401141
}
@@ -1278,13 +1279,25 @@ func readFileName(rd *strings.Reader) (string, bool) {
12781279
return name[2:], ambiguity
12791280
}
12801281

1281-
// GetDiffRangeWithWhitespaceBehavior builds a Diff between two commits of a repository.
1282+
// DiffOptions represents the options for a DiffRange
1283+
type DiffOptions struct {
1284+
BeforeCommitID string
1285+
AfterCommitID string
1286+
SkipTo string
1287+
MaxLines int
1288+
MaxLineCharacters int
1289+
MaxFiles int
1290+
WhitespaceBehavior string
1291+
DirectComparison bool
1292+
}
1293+
1294+
// GetDiff builds a Diff between two commits of a repository.
12821295
// Passing the empty string as beforeCommitID returns a diff from the parent commit.
12831296
// The whitespaceBehavior is either an empty string or a git flag
1284-
func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, afterCommitID, skipTo string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string, directComparison bool) (*Diff, error) {
1297+
func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff, error) {
12851298
repoPath := gitRepo.Path
12861299

1287-
commit, err := gitRepo.GetCommit(afterCommitID)
1300+
commit, err := gitRepo.GetCommit(opts.AfterCommitID)
12881301
if err != nil {
12891302
return nil, err
12901303
}
@@ -1293,45 +1306,54 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,
12931306
defer cancel()
12941307

12951308
argsLength := 6
1296-
if len(whitespaceBehavior) > 0 {
1309+
if len(opts.WhitespaceBehavior) > 0 {
12971310
argsLength++
12981311
}
1299-
if len(skipTo) > 0 {
1312+
if len(opts.SkipTo) > 0 {
13001313
argsLength++
13011314
}
1315+
if len(files) > 0 {
1316+
argsLength += len(files) + 1
1317+
}
13021318

13031319
diffArgs := make([]string, 0, argsLength)
1304-
if (len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 {
1320+
if (len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 {
13051321
diffArgs = append(diffArgs, "diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M")
1306-
if len(whitespaceBehavior) != 0 {
1307-
diffArgs = append(diffArgs, whitespaceBehavior)
1322+
if len(opts.WhitespaceBehavior) != 0 {
1323+
diffArgs = append(diffArgs, opts.WhitespaceBehavior)
13081324
}
13091325
// append empty tree ref
13101326
diffArgs = append(diffArgs, "4b825dc642cb6eb9a060e54bf8d69288fbee4904")
1311-
diffArgs = append(diffArgs, afterCommitID)
1327+
diffArgs = append(diffArgs, opts.AfterCommitID)
13121328
} else {
1313-
actualBeforeCommitID := beforeCommitID
1329+
actualBeforeCommitID := opts.BeforeCommitID
13141330
if len(actualBeforeCommitID) == 0 {
13151331
parentCommit, _ := commit.Parent(0)
13161332
actualBeforeCommitID = parentCommit.ID.String()
13171333
}
13181334
diffArgs = append(diffArgs, "diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M")
1319-
if len(whitespaceBehavior) != 0 {
1320-
diffArgs = append(diffArgs, whitespaceBehavior)
1335+
if len(opts.WhitespaceBehavior) != 0 {
1336+
diffArgs = append(diffArgs, opts.WhitespaceBehavior)
13211337
}
13221338
diffArgs = append(diffArgs, actualBeforeCommitID)
1323-
diffArgs = append(diffArgs, afterCommitID)
1324-
beforeCommitID = actualBeforeCommitID
1339+
diffArgs = append(diffArgs, opts.AfterCommitID)
1340+
opts.BeforeCommitID = actualBeforeCommitID
13251341
}
13261342

13271343
// In git 2.31, git diff learned --skip-to which we can use to shortcut skip to file
13281344
// so if we are using at least this version of git we don't have to tell ParsePatch to do
13291345
// the skipping for us
1330-
parsePatchSkipToFile := skipTo
1331-
if skipTo != "" && git.CheckGitVersionAtLeast("2.31") == nil {
1332-
diffArgs = append(diffArgs, "--skip-to="+skipTo)
1346+
parsePatchSkipToFile := opts.SkipTo
1347+
if opts.SkipTo != "" && git.CheckGitVersionAtLeast("2.31") == nil {
1348+
diffArgs = append(diffArgs, "--skip-to="+opts.SkipTo)
13331349
parsePatchSkipToFile = ""
13341350
}
1351+
1352+
if len(files) > 0 {
1353+
diffArgs = append(diffArgs, "--")
1354+
diffArgs = append(diffArgs, files...)
1355+
}
1356+
13351357
cmd := exec.CommandContext(ctx, git.GitExecutable, diffArgs...)
13361358

13371359
cmd.Dir = repoPath
@@ -1349,16 +1371,16 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,
13491371
pid := process.GetManager().Add(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath), cancel)
13501372
defer process.GetManager().Remove(pid)
13511373

1352-
diff, err := ParsePatch(maxLines, maxLineCharacters, maxFiles, stdout, parsePatchSkipToFile)
1374+
diff, err := ParsePatch(opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, stdout, parsePatchSkipToFile)
13531375
if err != nil {
13541376
return nil, fmt.Errorf("unable to ParsePatch: %w", err)
13551377
}
1356-
diff.Start = skipTo
1378+
diff.Start = opts.SkipTo
13571379

13581380
var checker *git.CheckAttributeReader
13591381

13601382
if git.CheckGitVersionAtLeast("1.7.8") == nil {
1361-
indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(afterCommitID)
1383+
indexFilename, worktree, deleteTemporaryFile, err := gitRepo.ReadTreeToTemporaryIndex(opts.AfterCommitID)
13621384
if err == nil {
13631385
defer deleteTemporaryFile()
13641386

@@ -1370,12 +1392,12 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,
13701392
}
13711393
ctx, cancel := context.WithCancel(git.DefaultContext)
13721394
if err := checker.Init(ctx); err != nil {
1373-
log.Error("Unable to open checker for %s. Error: %v", afterCommitID, err)
1395+
log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err)
13741396
} else {
13751397
go func() {
13761398
err := checker.Run()
13771399
if err != nil && err != ctx.Err() {
1378-
log.Error("Unable to open checker for %s. Error: %v", afterCommitID, err)
1400+
log.Error("Unable to open checker for %s. Error: %v", opts.AfterCommitID, err)
13791401
}
13801402
cancel()
13811403
}()
@@ -1426,7 +1448,7 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,
14261448
diffFile.IsGenerated = analyze.IsGenerated(diffFile.Name)
14271449
}
14281450

1429-
tailSection := diffFile.GetTailSection(gitRepo, beforeCommitID, afterCommitID)
1451+
tailSection := diffFile.GetTailSection(gitRepo, opts.BeforeCommitID, opts.AfterCommitID)
14301452
if tailSection != nil {
14311453
diffFile.Sections = append(diffFile.Sections, tailSection)
14321454
}
@@ -1437,19 +1459,19 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,
14371459
}
14381460

14391461
separator := "..."
1440-
if directComparison {
1462+
if opts.DirectComparison {
14411463
separator = ".."
14421464
}
14431465

1444-
shortstatArgs := []string{beforeCommitID + separator + afterCommitID}
1445-
if len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA {
1446-
shortstatArgs = []string{git.EmptyTreeSHA, afterCommitID}
1466+
shortstatArgs := []string{opts.BeforeCommitID + separator + opts.AfterCommitID}
1467+
if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == git.EmptySHA {
1468+
shortstatArgs = []string{git.EmptyTreeSHA, opts.AfterCommitID}
14471469
}
14481470
diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(repoPath, shortstatArgs...)
14491471
if err != nil && strings.Contains(err.Error(), "no merge base") {
14501472
// git >= 2.28 now returns an error if base and head have become unrelated.
14511473
// previously it would return the results of git diff --shortstat base head so let's try that...
1452-
shortstatArgs = []string{beforeCommitID, afterCommitID}
1474+
shortstatArgs = []string{opts.BeforeCommitID, opts.AfterCommitID}
14531475
diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(repoPath, shortstatArgs...)
14541476
}
14551477
if err != nil {
@@ -1459,12 +1481,6 @@ func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID,
14591481
return diff, nil
14601482
}
14611483

1462-
// GetDiffCommitWithWhitespaceBehavior builds a Diff representing the given commitID.
1463-
// The whitespaceBehavior is either an empty string or a git flag
1464-
func GetDiffCommitWithWhitespaceBehavior(gitRepo *git.Repository, commitID, skipTo string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string, directComparison bool) (*Diff, error) {
1465-
return GetDiffRangeWithWhitespaceBehavior(gitRepo, "", commitID, skipTo, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior, directComparison)
1466-
}
1467-
14681484
// CommentAsDiff returns c.Patch as *Diff
14691485
func CommentAsDiff(c *models.Comment) (*Diff, error) {
14701486
diff, err := ParsePatch(setting.Git.MaxGitDiffLines,

‎services/gitdiff/gitdiff_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -693,8 +693,15 @@ func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) {
693693
}
694694
defer gitRepo.Close()
695695
for _, behavior := range []string{"-w", "--ignore-space-at-eol", "-b", ""} {
696-
diffs, err := GetDiffRangeWithWhitespaceBehavior(gitRepo, "559c156f8e0178b71cb44355428f24001b08fc68", "bd7063cc7c04689c4d082183d32a604ed27a24f9", "",
697-
setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles, behavior, false)
696+
diffs, err := GetDiff(gitRepo,
697+
&DiffOptions{
698+
AfterCommitID: "bd7063cc7c04689c4d082183d32a604ed27a24f9",
699+
BeforeCommitID: "559c156f8e0178b71cb44355428f24001b08fc68",
700+
MaxLines: setting.Git.MaxGitDiffLines,
701+
MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters,
702+
MaxFiles: setting.Git.MaxGitDiffFiles,
703+
WhitespaceBehavior: behavior,
704+
})
698705
assert.NoError(t, err, fmt.Sprintf("Error when diff with %s", behavior))
699706
for _, f := range diffs.Files {
700707
assert.True(t, len(f.Sections) > 0, fmt.Sprintf("%s should have sections", f.Name))

‎templates/repo/diff/box.tmpl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@
112112
{{$.i18n.Tr "repo.diff.file_suppressed_line_too_long"}}
113113
{{else}}
114114
{{$.i18n.Tr "repo.diff.file_suppressed"}}
115+
<a class="ui basic tiny button diff-show-more-button" data-href="{{$.Link}}?file-only=true&files={{$file.Name}}&files={{$file.OldName}}">{{$.i18n.Tr "repo.diff.load"}}</a>
115116
{{end}}
116117
{{else}}
117118
{{$.i18n.Tr "repo.diff.bin_not_shown"}}

0 commit comments

Comments
 (0)