Skip to content

Commit d3ca4d6

Browse files
65436543
committed
Repare and Improve GetDiffRangeWithWhitespaceBehavior (go-gitea#16894)
1 parent 6cd3d1e commit d3ca4d6

File tree

5 files changed

+26
-37
lines changed

5 files changed

+26
-37
lines changed

‎routers/web/repo/commit.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,8 @@ func Diff(ctx *context.Context) {
268268
repoName := ctx.Repo.Repository.Name
269269
commitID := ctx.Params(":sha")
270270
var (
271-
gitRepo *git.Repository
272-
err error
273-
repoPath string
271+
gitRepo *git.Repository
272+
err error
274273
)
275274

276275
if ctx.Data["PageIsWiki"] != nil {
@@ -279,10 +278,9 @@ func Diff(ctx *context.Context) {
279278
ctx.ServerError("Repo.GitRepo.GetCommit", err)
280279
return
281280
}
282-
repoPath = ctx.Repo.Repository.WikiPath()
281+
defer gitRepo.Close()
283282
} else {
284283
gitRepo = ctx.Repo.GitRepo
285-
repoPath = models.RepoPath(userName, repoName)
286284
}
287285

288286
commit, err := gitRepo.GetCommit(commitID)
@@ -306,7 +304,7 @@ func Diff(ctx *context.Context) {
306304
ctx.Data["CommitStatus"] = models.CalcCommitStatus(statuses)
307305
ctx.Data["CommitStatuses"] = statuses
308306

309-
diff, err := gitdiff.GetDiffCommitWithWhitespaceBehavior(repoPath,
307+
diff, err := gitdiff.GetDiffCommitWithWhitespaceBehavior(gitRepo,
310308
commitID, setting.Git.MaxGitDiffLines,
311309
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
312310
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))

‎routers/web/repo/compare.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ func PrepareCompareDiff(
526526
return true
527527
}
528528

529-
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(models.RepoPath(headUser.Name, headRepo.Name),
529+
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(headGitRepo,
530530
compareInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines,
531531
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, whitespaceBehavior)
532532
if err != nil {
@@ -618,11 +618,15 @@ func getBranchesAndTagsForRepo(user *models.User, repo *models.Repository) (bool
618618
// CompareDiff show different from one commit to another commit
619619
func CompareDiff(ctx *context.Context) {
620620
headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch := ParseCompareInfo(ctx)
621+
defer func() {
622+
if headGitRepo != nil {
623+
headGitRepo.Close()
624+
}
625+
}()
621626

622627
if ctx.Written() {
623628
return
624629
}
625-
defer headGitRepo.Close()
626630

627631
nothingToCompare := PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch,
628632
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))

‎routers/web/repo/pull.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -587,10 +587,9 @@ func ViewPullFiles(ctx *context.Context) {
587587
pull := issue.PullRequest
588588

589589
var (
590-
diffRepoPath string
591590
startCommitID string
592591
endCommitID string
593-
gitRepo *git.Repository
592+
gitRepo = ctx.Repo.GitRepo
594593
)
595594

596595
var prInfo *git.CompareInfo
@@ -607,9 +606,6 @@ func ViewPullFiles(ctx *context.Context) {
607606
return
608607
}
609608

610-
diffRepoPath = ctx.Repo.GitRepo.Path
611-
gitRepo = ctx.Repo.GitRepo
612-
613609
headCommitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName())
614610
if err != nil {
615611
ctx.ServerError("GetRefCommitID", err)
@@ -623,7 +619,7 @@ func ViewPullFiles(ctx *context.Context) {
623619
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
624620
ctx.Data["AfterCommitID"] = endCommitID
625621

626-
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(diffRepoPath,
622+
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(gitRepo,
627623
startCommitID, endCommitID, setting.Git.MaxGitDiffLines,
628624
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
629625
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))

‎services/gitdiff/gitdiff.go‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"regexp"
2121
"sort"
2222
"strings"
23+
"time"
2324

2425
"code.gitea.io/gitea/models"
2526
"code.gitea.io/gitea/modules/charset"
@@ -1191,31 +1192,20 @@ func readFileName(rd *strings.Reader) (string, bool) {
11911192
return name[2:], ambiguity
11921193
}
11931194

1194-
// GetDiffRange builds a Diff between two commits of a repository.
1195-
// passing the empty string as beforeCommitID returns a diff from the
1196-
// parent commit.
1197-
func GetDiffRange(repoPath, beforeCommitID, afterCommitID string, maxLines, maxLineCharacters, maxFiles int) (*Diff, error) {
1198-
return GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID, maxLines, maxLineCharacters, maxFiles, "")
1199-
}
1200-
12011195
// GetDiffRangeWithWhitespaceBehavior builds a Diff between two commits of a repository.
12021196
// Passing the empty string as beforeCommitID returns a diff from the parent commit.
12031197
// The whitespaceBehavior is either an empty string or a git flag
1204-
func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) {
1205-
gitRepo, err := git.OpenRepository(repoPath)
1206-
if err != nil {
1207-
return nil, err
1208-
}
1209-
defer gitRepo.Close()
1198+
func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, afterCommitID string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) {
1199+
repoPath := gitRepo.Path
12101200

12111201
commit, err := gitRepo.GetCommit(afterCommitID)
12121202
if err != nil {
12131203
return nil, err
12141204
}
12151205

1216-
// FIXME: graceful: These commands should likely have a timeout
1217-
ctx, cancel := context.WithCancel(git.DefaultContext)
1206+
ctx, cancel := context.WithTimeout(git.DefaultContext, time.Duration(setting.Git.Timeout.Default)*time.Second)
12181207
defer cancel()
1208+
12191209
var cmd *exec.Cmd
12201210
if (len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 {
12211211
diffArgs := []string{"diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M"}
@@ -1289,15 +1279,10 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID
12891279
return diff, nil
12901280
}
12911281

1292-
// GetDiffCommit builds a Diff representing the given commitID.
1293-
func GetDiffCommit(repoPath, commitID string, maxLines, maxLineCharacters, maxFiles int) (*Diff, error) {
1294-
return GetDiffRangeWithWhitespaceBehavior(repoPath, "", commitID, maxLines, maxLineCharacters, maxFiles, "")
1295-
}
1296-
12971282
// GetDiffCommitWithWhitespaceBehavior builds a Diff representing the given commitID.
12981283
// The whitespaceBehavior is either an empty string or a git flag
1299-
func GetDiffCommitWithWhitespaceBehavior(repoPath, commitID string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) {
1300-
return GetDiffRangeWithWhitespaceBehavior(repoPath, "", commitID, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior)
1284+
func GetDiffCommitWithWhitespaceBehavior(gitRepo *git.Repository, commitID string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) {
1285+
return GetDiffRangeWithWhitespaceBehavior(gitRepo, "", commitID, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior)
13011286
}
13021287

13031288
// CommentAsDiff returns c.Patch as *Diff

‎services/gitdiff/gitdiff_test.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"testing"
1414

1515
"code.gitea.io/gitea/models"
16+
"code.gitea.io/gitea/modules/git"
1617
"code.gitea.io/gitea/modules/highlight"
1718
"code.gitea.io/gitea/modules/setting"
1819
jsoniter "github.com/json-iterator/go"
@@ -513,8 +514,13 @@ func TestDiffLine_GetCommentSide(t *testing.T) {
513514
}
514515

515516
func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) {
517+
gitRepo, err := git.OpenRepository("./testdata/academic-module")
518+
if !assert.NoError(t, err) {
519+
return
520+
}
521+
defer gitRepo.Close()
516522
for _, behavior := range []string{"-w", "--ignore-space-at-eol", "-b", ""} {
517-
diffs, err := GetDiffRangeWithWhitespaceBehavior("./testdata/academic-module", "559c156f8e0178b71cb44355428f24001b08fc68", "bd7063cc7c04689c4d082183d32a604ed27a24f9",
523+
diffs, err := GetDiffRangeWithWhitespaceBehavior(gitRepo, "559c156f8e0178b71cb44355428f24001b08fc68", "bd7063cc7c04689c4d082183d32a604ed27a24f9",
518524
setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles, behavior)
519525
assert.NoError(t, err, fmt.Sprintf("Error when diff with %s", behavior))
520526
for _, f := range diffs.Files {

0 commit comments

Comments
 (0)