Repare and Improve GetDiffRangeWithWhitespaceBehavior (#16894)

* repare and improve GetDiffRangeWithWhitespaceBehavior

* Context with Timeout
This commit is contained in:
6543 2021-08-31 06:16:23 +02:00 committed by GitHub
parent f2b4b0f491
commit bb4cc876b1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 26 additions and 37 deletions

View file

@ -259,9 +259,8 @@ func Diff(ctx *context.Context) {
repoName := ctx.Repo.Repository.Name repoName := ctx.Repo.Repository.Name
commitID := ctx.Params(":sha") commitID := ctx.Params(":sha")
var ( var (
gitRepo *git.Repository gitRepo *git.Repository
err error err error
repoPath string
) )
if ctx.Data["PageIsWiki"] != nil { if ctx.Data["PageIsWiki"] != nil {
@ -270,10 +269,9 @@ func Diff(ctx *context.Context) {
ctx.ServerError("Repo.GitRepo.GetCommit", err) ctx.ServerError("Repo.GitRepo.GetCommit", err)
return return
} }
repoPath = ctx.Repo.Repository.WikiPath() defer gitRepo.Close()
} else { } else {
gitRepo = ctx.Repo.GitRepo gitRepo = ctx.Repo.GitRepo
repoPath = models.RepoPath(userName, repoName)
} }
commit, err := gitRepo.GetCommit(commitID) commit, err := gitRepo.GetCommit(commitID)
@ -297,7 +295,7 @@ func Diff(ctx *context.Context) {
ctx.Data["CommitStatus"] = models.CalcCommitStatus(statuses) ctx.Data["CommitStatus"] = models.CalcCommitStatus(statuses)
ctx.Data["CommitStatuses"] = statuses ctx.Data["CommitStatuses"] = statuses
diff, err := gitdiff.GetDiffCommitWithWhitespaceBehavior(repoPath, diff, err := gitdiff.GetDiffCommitWithWhitespaceBehavior(gitRepo,
commitID, setting.Git.MaxGitDiffLines, commitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string))) gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))

View file

@ -526,7 +526,7 @@ func PrepareCompareDiff(
return true return true
} }
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(models.RepoPath(headUser.Name, headRepo.Name), diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(headGitRepo,
compareInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines, compareInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, whitespaceBehavior) setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, whitespaceBehavior)
if err != nil { if err != nil {
@ -616,11 +616,15 @@ func getBranchesAndTagsForRepo(user *models.User, repo *models.Repository) (bool
// CompareDiff show different from one commit to another commit // CompareDiff show different from one commit to another commit
func CompareDiff(ctx *context.Context) { func CompareDiff(ctx *context.Context) {
headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch := ParseCompareInfo(ctx) headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch := ParseCompareInfo(ctx)
defer func() {
if headGitRepo != nil {
headGitRepo.Close()
}
}()
if ctx.Written() { if ctx.Written() {
return return
} }
defer headGitRepo.Close()
nothingToCompare := PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch, nothingToCompare := PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string))) gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))

View file

@ -599,10 +599,9 @@ func ViewPullFiles(ctx *context.Context) {
pull := issue.PullRequest pull := issue.PullRequest
var ( var (
diffRepoPath string
startCommitID string startCommitID string
endCommitID string endCommitID string
gitRepo *git.Repository gitRepo = ctx.Repo.GitRepo
) )
var prInfo *git.CompareInfo var prInfo *git.CompareInfo
@ -619,9 +618,6 @@ func ViewPullFiles(ctx *context.Context) {
return return
} }
diffRepoPath = ctx.Repo.GitRepo.Path
gitRepo = ctx.Repo.GitRepo
headCommitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName()) headCommitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName())
if err != nil { if err != nil {
ctx.ServerError("GetRefCommitID", err) ctx.ServerError("GetRefCommitID", err)
@ -635,7 +631,7 @@ func ViewPullFiles(ctx *context.Context) {
ctx.Data["Reponame"] = ctx.Repo.Repository.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name
ctx.Data["AfterCommitID"] = endCommitID ctx.Data["AfterCommitID"] = endCommitID
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(diffRepoPath, diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(gitRepo,
startCommitID, endCommitID, setting.Git.MaxGitDiffLines, startCommitID, endCommitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string))) gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))

View file

@ -20,6 +20,7 @@ import (
"regexp" "regexp"
"sort" "sort"
"strings" "strings"
"time"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/charset"
@ -1205,31 +1206,20 @@ func readFileName(rd *strings.Reader) (string, bool) {
return name[2:], ambiguity return name[2:], ambiguity
} }
// GetDiffRange builds a Diff between two commits of a repository.
// passing the empty string as beforeCommitID returns a diff from the
// parent commit.
func GetDiffRange(repoPath, beforeCommitID, afterCommitID string, maxLines, maxLineCharacters, maxFiles int) (*Diff, error) {
return GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID, maxLines, maxLineCharacters, maxFiles, "")
}
// GetDiffRangeWithWhitespaceBehavior builds a Diff between two commits of a repository. // GetDiffRangeWithWhitespaceBehavior builds a Diff between two commits of a repository.
// Passing the empty string as beforeCommitID returns a diff from the parent commit. // Passing the empty string as beforeCommitID returns a diff from the parent commit.
// The whitespaceBehavior is either an empty string or a git flag // The whitespaceBehavior is either an empty string or a git flag
func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) { func GetDiffRangeWithWhitespaceBehavior(gitRepo *git.Repository, beforeCommitID, afterCommitID string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) {
gitRepo, err := git.OpenRepository(repoPath) repoPath := gitRepo.Path
if err != nil {
return nil, err
}
defer gitRepo.Close()
commit, err := gitRepo.GetCommit(afterCommitID) commit, err := gitRepo.GetCommit(afterCommitID)
if err != nil { if err != nil {
return nil, err return nil, err
} }
// FIXME: graceful: These commands should likely have a timeout ctx, cancel := context.WithTimeout(git.DefaultContext, time.Duration(setting.Git.Timeout.Default)*time.Second)
ctx, cancel := context.WithCancel(git.DefaultContext)
defer cancel() defer cancel()
var cmd *exec.Cmd var cmd *exec.Cmd
if (len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 { if (len(beforeCommitID) == 0 || beforeCommitID == git.EmptySHA) && commit.ParentCount() == 0 {
diffArgs := []string{"diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M"} diffArgs := []string{"diff", "--src-prefix=\\a/", "--dst-prefix=\\b/", "-M"}
@ -1303,15 +1293,10 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID
return diff, nil return diff, nil
} }
// GetDiffCommit builds a Diff representing the given commitID.
func GetDiffCommit(repoPath, commitID string, maxLines, maxLineCharacters, maxFiles int) (*Diff, error) {
return GetDiffRangeWithWhitespaceBehavior(repoPath, "", commitID, maxLines, maxLineCharacters, maxFiles, "")
}
// GetDiffCommitWithWhitespaceBehavior builds a Diff representing the given commitID. // GetDiffCommitWithWhitespaceBehavior builds a Diff representing the given commitID.
// The whitespaceBehavior is either an empty string or a git flag // The whitespaceBehavior is either an empty string or a git flag
func GetDiffCommitWithWhitespaceBehavior(repoPath, commitID string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) { func GetDiffCommitWithWhitespaceBehavior(gitRepo *git.Repository, commitID string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) {
return GetDiffRangeWithWhitespaceBehavior(repoPath, "", commitID, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior) return GetDiffRangeWithWhitespaceBehavior(gitRepo, "", commitID, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior)
} }
// CommentAsDiff returns c.Patch as *Diff // CommentAsDiff returns c.Patch as *Diff

View file

@ -13,6 +13,7 @@ import (
"testing" "testing"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/highlight" "code.gitea.io/gitea/modules/highlight"
"code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
@ -514,8 +515,13 @@ func TestDiffLine_GetCommentSide(t *testing.T) {
} }
func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) { func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) {
gitRepo, err := git.OpenRepository("./testdata/academic-module")
if !assert.NoError(t, err) {
return
}
defer gitRepo.Close()
for _, behavior := range []string{"-w", "--ignore-space-at-eol", "-b", ""} { for _, behavior := range []string{"-w", "--ignore-space-at-eol", "-b", ""} {
diffs, err := GetDiffRangeWithWhitespaceBehavior("./testdata/academic-module", "559c156f8e0178b71cb44355428f24001b08fc68", "bd7063cc7c04689c4d082183d32a604ed27a24f9", diffs, err := GetDiffRangeWithWhitespaceBehavior(gitRepo, "559c156f8e0178b71cb44355428f24001b08fc68", "bd7063cc7c04689c4d082183d32a604ed27a24f9",
setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles, behavior) setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles, behavior)
assert.NoError(t, err, fmt.Sprintf("Error when diff with %s", behavior)) assert.NoError(t, err, fmt.Sprintf("Error when diff with %s", behavior))
for _, f := range diffs.Files { for _, f := range diffs.Files {