Decouple diff stats query from actual diffing (#33810)

The diff stats are no longer part of the diff generation.
Use `GetDiffShortStat` instead to get the total number of changed files,
added lines, and deleted lines.
As such, `gitdiff.GetDiff` can be simplified:
It should not do more than expected.

And do not run "git diff --shortstat" for pull list. Fix #31492

2 files changed, 17 insertions, 23 deletions

diff --git a/services/convert/git_commit.go b/services/convert/git_commit.go
index e0efcddbcb..3ec81b52ee 100644
--- a/services/convert/git_commit.go
+++ b/services/convert/git_commit.go
@@ -210,17 +210,15 @@ func ToCommit(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Rep
 
 	// Get diff stats for commit
 	if opts.Stat {
-		diff, err := gitdiff.GetDiff(ctx, gitRepo, &gitdiff.DiffOptions{
-			AfterCommitID: commit.ID.String(),
-		})
+		diffShortStat, err := gitdiff.GetDiffShortStat(gitRepo, "", commit.ID.String())
 		if err != nil {
 			return nil, err
 		}
 
 		res.Stats = &api.CommitStats{
-			Total:     diff.TotalAddition + diff.TotalDeletion,
-			Additions: diff.TotalAddition,
-			Deletions: diff.TotalDeletion,
+			Total:     diffShortStat.TotalAddition + diffShortStat.TotalDeletion,
+			Additions: diffShortStat.TotalAddition,
+			Deletions: diffShortStat.TotalDeletion,
 		}
 	}
 
diff --git a/services/convert/pull.go b/services/convert/pull.go
index ad4f08fa91..928534ce5e 100644
--- a/services/convert/pull.go
+++ b/services/convert/pull.go
@@ -17,8 +17,10 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/util"
+	"code.gitea.io/gitea/services/gitdiff"
 )
 
 // ToAPIPullRequest assumes following fields have been assigned with valid values:
@@ -239,9 +241,13 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u
 		// Calculate diff
 		startCommitID = pr.MergeBase
 
-		apiPullRequest.ChangedFiles, apiPullRequest.Additions, apiPullRequest.Deletions, err = gitRepo.GetDiffShortStat(startCommitID, endCommitID)
+		diffShortStats, err := gitdiff.GetDiffShortStat(gitRepo, startCommitID, endCommitID)
 		if err != nil {
 			log.Error("GetDiffShortStat: %v", err)
+		} else {
+			apiPullRequest.ChangedFiles = &diffShortStats.NumFiles
+			apiPullRequest.Additions = &diffShortStats.TotalAddition
+			apiPullRequest.Deletions = &diffShortStats.TotalDeletion
 		}
 	}
 
@@ -462,12 +468,6 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs
 				return nil, err
 			}
 
-			// Outer scope variables to be used in diff calculation
-			var (
-				startCommitID string
-				endCommitID   string
-			)
-
 			if git.IsErrBranchNotExist(err) {
 				headCommitID, err := headGitRepo.GetRefCommitID(apiPullRequest.Head.Ref)
 				if err != nil && !git.IsErrNotExist(err) {
@@ -476,7 +476,6 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs
 				}
 				if err == nil {
 					apiPullRequest.Head.Sha = headCommitID
-					endCommitID = headCommitID
 				}
 			} else {
 				commit, err := headBranch.GetCommit()
@@ -487,17 +486,8 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs
 				if err == nil {
 					apiPullRequest.Head.Ref = pr.HeadBranch
 					apiPullRequest.Head.Sha = commit.ID.String()
-					endCommitID = commit.ID.String()
 				}
 			}
-
-			// Calculate diff
-			startCommitID = pr.MergeBase
-
-			apiPullRequest.ChangedFiles, apiPullRequest.Additions, apiPullRequest.Deletions, err = gitRepo.GetDiffShortStat(startCommitID, endCommitID)
-			if err != nil {
-				log.Error("GetDiffShortStat: %v", err)
-			}
 		}
 
 		if len(apiPullRequest.Head.Sha) == 0 && len(apiPullRequest.Head.Ref) != 0 {
@@ -518,6 +508,12 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs
 			apiPullRequest.MergedBy = ToUser(ctx, pr.Merger, nil)
 		}
 
+		// Do not provide "ChangeFiles/Additions/Deletions" for the PR list, because the "diff" is quite slow
+		// If callers are interested in these values, they should do a separate request to get the PR details
+		if apiPullRequest.ChangedFiles != nil || apiPullRequest.Additions != nil || apiPullRequest.Deletions != nil {
+			setting.PanicInDevOrTesting("ChangedFiles/Additions/Deletions should not be set in PR list")
+		}
+
 		apiPullRequests = append(apiPullRequests, apiPullRequest)
 	}