From 5cb201dc93bf41556556f2154ea28755907fd550 Mon Sep 17 00:00:00 2001 From: zeripath Date: Tue, 26 May 2020 06:58:07 +0100 Subject: [PATCH] Fix numbr of files, total additions, and deletions (#11614) Signed-off-by: Andrew Thornton Co-authored-by: techknowlogick --- modules/git/repo_compare.go | 89 +++++++++++++++++++++++++++++++++- modules/repofiles/diff_test.go | 1 + modules/repofiles/temp_repo.go | 5 ++ routers/repo/commit.go | 2 +- routers/repo/compare.go | 2 +- routers/repo/editor.go | 2 +- routers/repo/pull.go | 2 +- services/gitdiff/gitdiff.go | 18 +++---- 8 files changed, 106 insertions(+), 15 deletions(-) diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 5bc7f9ca5a6..5faadcf3f02 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -6,9 +6,11 @@ package git import ( + "bytes" "container/list" "fmt" "io" + "regexp" "strconv" "strings" "time" @@ -84,14 +86,97 @@ func (repo *Repository) GetCompareInfo(basePath, baseBranch, headBranch string) } // Count number of changed files. - stdout, err := NewCommand("diff", "--name-only", remoteBranch+"..."+headBranch).RunInDir(repo.Path) + // This probably should be removed as we need to use shortstat elsewhere + // Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly + compareInfo.NumFiles, err = repo.GetDiffNumChangedFiles(remoteBranch, headBranch) if err != nil { return nil, err } - compareInfo.NumFiles = len(strings.Split(stdout, "\n")) - 1 return compareInfo, nil } +type lineCountWriter struct { + numLines int +} + +// Write counts the number of newlines in the provided bytestream +func (l *lineCountWriter) Write(p []byte) (n int, err error) { + n = len(p) + l.numLines += bytes.Count(p, []byte{'\000'}) + return +} + +// GetDiffNumChangedFiles counts the number of changed files +// This is substantially quicker than shortstat but... +func (repo *Repository) GetDiffNumChangedFiles(base, head string) (int, error) { + // Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly + w := &lineCountWriter{} + stderr := new(bytes.Buffer) + + if err := NewCommand("diff", "-z", "--name-only", base+"..."+head). + RunInDirPipeline(repo.Path, w, stderr); err != nil { + return 0, fmt.Errorf("%v: Stderr: %s", err, stderr) + } + return w.numLines, nil +} + +// GetDiffShortStat counts number of changed files, number of additions and deletions +func (repo *Repository) GetDiffShortStat(base, head string) (numFiles, totalAdditions, totalDeletions int, err error) { + return GetDiffShortStat(repo.Path, base+"..."+head) +} + +// GetDiffShortStat counts number of changed files, number of additions and deletions +func GetDiffShortStat(repoPath string, args ...string) (numFiles, totalAdditions, totalDeletions int, err error) { + // Now if we call: + // $ git diff --shortstat 1ebb35b98889ff77299f24d82da426b434b0cca0...788b8b1440462d477f45b0088875 + // we get: + // " 9902 files changed, 2034198 insertions(+), 298800 deletions(-)\n" + args = append([]string{ + "diff", + "--shortstat", + }, args...) + + stdout, err := NewCommand(args...).RunInDir(repoPath) + if err != nil { + return 0, 0, 0, err + } + + return parseDiffStat(stdout) +} + +var shortStatFormat = regexp.MustCompile( + `\s*(\d+) files? changed(?:, (\d+) insertions?\(\+\))?(?:, (\d+) deletions?\(-\))?`) + +func parseDiffStat(stdout string) (numFiles, totalAdditions, totalDeletions int, err error) { + if len(stdout) == 0 || stdout == "\n" { + return 0, 0, 0, nil + } + groups := shortStatFormat.FindStringSubmatch(stdout) + if len(groups) != 4 { + return 0, 0, 0, fmt.Errorf("unable to parse shortstat: %s groups: %s", stdout, groups) + } + + numFiles, err = strconv.Atoi(groups[1]) + if err != nil { + return 0, 0, 0, fmt.Errorf("unable to parse shortstat: %s. Error parsing NumFiles %v", stdout, err) + } + + if len(groups[2]) != 0 { + totalAdditions, err = strconv.Atoi(groups[2]) + if err != nil { + return 0, 0, 0, fmt.Errorf("unable to parse shortstat: %s. Error parsing NumAdditions %v", stdout, err) + } + } + + if len(groups[3]) != 0 { + totalDeletions, err = strconv.Atoi(groups[3]) + if err != nil { + return 0, 0, 0, fmt.Errorf("unable to parse shortstat: %s. Error parsing NumDeletions %v", stdout, err) + } + } + return +} + // GetDiffOrPatch generates either diff or formatted patch data between given revisions func (repo *Repository) GetDiffOrPatch(base, head string, w io.Writer, formatted bool) error { if formatted { diff --git a/modules/repofiles/diff_test.go b/modules/repofiles/diff_test.go index 4e1d5b13ebd..5c09e180f3f 100644 --- a/modules/repofiles/diff_test.go +++ b/modules/repofiles/diff_test.go @@ -108,6 +108,7 @@ func TestGetDiffPreview(t *testing.T) { }, IsIncomplete: false, } + expectedDiff.NumFiles = len(expectedDiff.Files) t.Run("with given branch", func(t *testing.T) { diff, err := GetDiffPreview(ctx.Repo.Repository, branch, treePath, content) diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index 89f9b0b2089..2b03db8b4a3 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -299,6 +299,11 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { t.repo.FullName(), err, stderr) } + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(t.basePath, "--cached", "HEAD") + if err != nil { + return nil, err + } + return diff, nil } diff --git a/routers/repo/commit.go b/routers/repo/commit.go index d87d220f246..2b6c9cece50 100644 --- a/routers/repo/commit.go +++ b/routers/repo/commit.go @@ -291,7 +291,7 @@ func Diff(ctx *context.Context) { ctx.Data["Author"] = models.ValidateCommitWithEmail(commit) ctx.Data["Diff"] = diff ctx.Data["Parents"] = parents - ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0 + ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0 if err := models.CalculateTrustStatus(verification, ctx.Repo.Repository, nil); err != nil { ctx.ServerError("CalculateTrustStatus", err) diff --git a/routers/repo/compare.go b/routers/repo/compare.go index 97bb5e6b18c..858e4e548e0 100644 --- a/routers/repo/compare.go +++ b/routers/repo/compare.go @@ -437,7 +437,7 @@ func PrepareCompareDiff( return false } ctx.Data["Diff"] = diff - ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0 + ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0 headCommit, err := headGitRepo.GetCommit(headCommitID) if err != nil { diff --git a/routers/repo/editor.go b/routers/repo/editor.go index 2fa7976e001..f91ce1b462f 100644 --- a/routers/repo/editor.go +++ b/routers/repo/editor.go @@ -339,7 +339,7 @@ func DiffPreviewPost(ctx *context.Context, form auth.EditPreviewDiffForm) { return } - if diff.NumFiles() == 0 { + if diff.NumFiles == 0 { ctx.PlainText(200, []byte(ctx.Tr("repo.editor.no_changes_to_show"))) return } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index d23c93d0b65..d4c99e27699 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -611,7 +611,7 @@ func ViewPullFiles(ctx *context.Context) { } ctx.Data["Diff"] = diff - ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0 + ctx.Data["DiffNotAvailable"] = diff.NumFiles == 0 baseCommit, err := ctx.Repo.GitRepo.GetCommit(startCommitID) if err != nil { diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 6867f8e0a4e..af88ed4d40e 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -367,9 +367,9 @@ func getCommitFileLineCount(commit *git.Commit, filePath string) int { // Diff represents a difference between two git trees. type Diff struct { - TotalAddition, TotalDeletion int - Files []*DiffFile - IsIncomplete bool + NumFiles, TotalAddition, TotalDeletion int + Files []*DiffFile + IsIncomplete bool } // LoadComments loads comments into each line @@ -398,11 +398,6 @@ func (diff *Diff) LoadComments(issue *models.Issue, currentUser *models.User) er return nil } -// NumFiles returns number of files changes in a diff. -func (diff *Diff) NumFiles() int { - return len(diff.Files) -} - const cmdDiffHead = "diff --git " // ParsePatch builds a Diff object from a io.Reader and some @@ -639,7 +634,7 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D } } } - + diff.NumFiles = len(diff.Files) return diff, nil } @@ -716,6 +711,11 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID return nil, fmt.Errorf("Wait: %v", err) } + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(repoPath, beforeCommitID+"..."+afterCommitID) + if err != nil { + return nil, err + } + return diff, nil }