Pass 'not' to commit count (#24473)

Due to #24409 , we can now specify '--not' when getting all commits from
a repo to exclude commits from a different branch.

When I wrote that PR, I forgot to also update the code that counts the
number of commits in the repo. So now, if the --not option is used, it
may return too many commits, which can indicate that another page of
data is available when it is not.

This PR passes --not to the commands that count the number of commits in
a repo

modules/context/repo.go

@@ -208,7 +208,12 @@ func (r *Repository) GetCommitGraphsCount(ctx context.Context, hidePRRefs bool,
 		if len(branches) == 0 {
 			return git.AllCommitsCount(ctx, r.Repository.RepoPath(), hidePRRefs, files...)
 		}
-		return git.CommitsCountFiles(ctx, r.Repository.RepoPath(), branches, files)
+		return git.CommitsCount(ctx,
+			git.CommitsCountOptions{
+				RepoPath: r.Repository.RepoPath(),
+				Revision: branches,
+				RelPath:  files,
+			})
 	})
 }
 

modules/git/commit.go

@@ -160,15 +160,29 @@ func AllCommitsCount(ctx context.Context, repoPath string, hidePRRefs bool, file
 	return strconv.ParseInt(strings.TrimSpace(stdout), 10, 64)
 }
 
-// CommitsCountFiles returns number of total commits of until given revision.
+// CommitsCountOptions the options when counting commits
-func CommitsCountFiles(ctx context.Context, repoPath string, revision, relpath []string) (int64, error) {
+type CommitsCountOptions struct {
-	cmd := NewCommand(ctx, "rev-list", "--count")
+	RepoPath string
-	cmd.AddDynamicArguments(revision...)
+	Not      string
-	if len(relpath) > 0 {
+	Revision []string
-		cmd.AddDashesAndList(relpath...)
+	RelPath  []string
 }
 
-	stdout, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath})
+// CommitsCount returns number of total commits of until given revision.
+func CommitsCount(ctx context.Context, opts CommitsCountOptions) (int64, error) {
+	cmd := NewCommand(ctx, "rev-list", "--count")
+
+	cmd.AddDynamicArguments(opts.Revision...)
+
+	if opts.Not != "" {
+		cmd.AddOptionValues("--not", opts.Not)
+	}
+
+	if len(opts.RelPath) > 0 {
+		cmd.AddDashesAndList(opts.RelPath...)
+	}
+
+	stdout, _, err := cmd.RunStdString(&RunOpts{Dir: opts.RepoPath})
 	if err != nil {
 		return 0, err
 	}
@@ -176,14 +190,12 @@ func CommitsCountFiles(ctx context.Context, repoPath string, revision, relpath [
 	return strconv.ParseInt(strings.TrimSpace(stdout), 10, 64)
 }
 
-// CommitsCount returns number of total commits of until given revision.
-func CommitsCount(ctx context.Context, repoPath string, revision ...string) (int64, error) {
-	return CommitsCountFiles(ctx, repoPath, revision, []string{})
-}
-
 // CommitsCount returns number of total commits of until current revision.
 func (c *Commit) CommitsCount() (int64, error) {
-	return CommitsCount(c.repo.Ctx, c.repo.Path, c.ID.String())
+	return CommitsCount(c.repo.Ctx, CommitsCountOptions{
+		RepoPath: c.repo.Path,
+		Revision: []string{c.ID.String()},
+	})
 }
 
 // CommitsByRange returns the specific page commits before current revision, every page's number default by CommitsRangeSize

modules/git/commit_test.go

@@ -14,11 +14,30 @@ import (
 func TestCommitsCount(t *testing.T) {
 	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
 
-	commitsCount, err := CommitsCount(DefaultContext, bareRepo1Path, "8006ff9adbf0cb94da7dad9e537e53817f9fa5c0")
+	commitsCount, err := CommitsCount(DefaultContext,
+		CommitsCountOptions{
+			RepoPath: bareRepo1Path,
+			Revision: []string{"8006ff9adbf0cb94da7dad9e537e53817f9fa5c0"},
+		})
+
 	assert.NoError(t, err)
 	assert.Equal(t, int64(3), commitsCount)
 }
 
+func TestCommitsCountWithoutBase(t *testing.T) {
+	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
+
+	commitsCount, err := CommitsCount(DefaultContext,
+		CommitsCountOptions{
+			RepoPath: bareRepo1Path,
+			Not:      "master",
+			Revision: []string{"branch1"},
+		})
+
+	assert.NoError(t, err)
+	assert.Equal(t, int64(2), commitsCount)
+}
+
 func TestGetFullCommitID(t *testing.T) {
 	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
 

modules/git/repo_commit.go

@@ -205,12 +205,24 @@ func (repo *Repository) FileChangedBetweenCommits(filename, id1, id2 string) (bo
 
 // FileCommitsCount return the number of files at a revision
 func (repo *Repository) FileCommitsCount(revision, file string) (int64, error) {
-	return CommitsCountFiles(repo.Ctx, repo.Path, []string{revision}, []string{file})
+	return CommitsCount(repo.Ctx,
+		CommitsCountOptions{
+			RepoPath: repo.Path,
+			Revision: []string{revision},
+			RelPath:  []string{file},
+		})
+}
+
+type CommitsByFileAndRangeOptions struct {
+	Revision string
+	File     string
+	Not      string
+	Page     int
 }
 
 // CommitsByFileAndRange return the commits according revision file and the page
-func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ([]*Commit, error) {
+func (repo *Repository) CommitsByFileAndRange(opts CommitsByFileAndRangeOptions) ([]*Commit, error) {
-	skip := (page - 1) * setting.Git.CommitsRangeSize
+	skip := (opts.Page - 1) * setting.Git.CommitsRangeSize
 
 	stdoutReader, stdoutWriter := io.Pipe()
 	defer func() {
@@ -220,10 +232,15 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) (
 	go func() {
 		stderr := strings.Builder{}
 		gitCmd := NewCommand(repo.Ctx, "rev-list").
-			AddOptionFormat("--max-count=%d", setting.Git.CommitsRangeSize*page).
+			AddOptionFormat("--max-count=%d", setting.Git.CommitsRangeSize*opts.Page).
 			AddOptionFormat("--skip=%d", skip)
-		gitCmd.AddDynamicArguments(revision)
+		gitCmd.AddDynamicArguments(opts.Revision)
-		gitCmd.AddDashesAndList(file)
+
+		if opts.Not != "" {
+			gitCmd.AddOptionValues("--not", opts.Not)
+		}
+
+		gitCmd.AddDashesAndList(opts.File)
 		err := gitCmd.Run(&RunOpts{
 			Dir:    repo.Path,
 			Stdout: stdoutWriter,
@@ -365,11 +382,18 @@ func (repo *Repository) CommitsBetweenIDs(last, before string) ([]*Commit, error
 
 // CommitsCountBetween return numbers of commits between two commits
 func (repo *Repository) CommitsCountBetween(start, end string) (int64, error) {
-	count, err := CommitsCountFiles(repo.Ctx, repo.Path, []string{start + ".." + end}, []string{})
+	count, err := CommitsCount(repo.Ctx, CommitsCountOptions{
+		RepoPath: repo.Path,
+		Revision: []string{start + ".." + end},
+	})
+
 	if err != nil && strings.Contains(err.Error(), "no merge base") {
 		// future versions of git >= 2.28 are likely to return an error if before and last have become unrelated.
 		// previously it would return the results of git rev-list before last so let's try that...
-		return CommitsCountFiles(repo.Ctx, repo.Path, []string{start, end}, []string{})
+		return CommitsCount(repo.Ctx, CommitsCountOptions{
+			RepoPath: repo.Path,
+			Revision: []string{start, end},
+		})
 	}
 
 	return count, err

routers/api/v1/repo/commits.go

@@ -146,6 +146,7 @@ func GetAllCommits(ctx *context.APIContext) {
 
 	sha := ctx.FormString("sha")
 	path := ctx.FormString("path")
+	not := ctx.FormString("not")
 
 	var (
 		commitsCountTotal int64
@@ -178,14 +179,18 @@ func GetAllCommits(ctx *context.APIContext) {
 		}
 
 		// Total commit count
-		commitsCountTotal, err = baseCommit.CommitsCount()
+		commitsCountTotal, err = git.CommitsCount(ctx.Repo.GitRepo.Ctx, git.CommitsCountOptions{
+			RepoPath: ctx.Repo.GitRepo.Path,
+			Not:      not,
+			Revision: []string{baseCommit.ID.String()},
+		})
+
 		if err != nil {
 			ctx.Error(http.StatusInternalServerError, "GetCommitsCount", err)
 			return
 		}
 
 		// Query commits
-		not := ctx.FormString("not")
 		commits, err = baseCommit.CommitsByRange(listOptions.Page, listOptions.PageSize, not)
 		if err != nil {
 			ctx.Error(http.StatusInternalServerError, "CommitsByRange", err)
@@ -196,7 +201,14 @@ func GetAllCommits(ctx *context.APIContext) {
 			sha = ctx.Repo.Repository.DefaultBranch
 		}
 
-		commitsCountTotal, err = ctx.Repo.GitRepo.FileCommitsCount(sha, path)
+		commitsCountTotal, err = git.CommitsCount(ctx,
+			git.CommitsCountOptions{
+				RepoPath: ctx.Repo.GitRepo.Path,
+				Not:      not,
+				Revision: []string{sha},
+				RelPath:  []string{path},
+			})
+
 		if err != nil {
 			ctx.Error(http.StatusInternalServerError, "FileCommitsCount", err)
 			return
@@ -205,7 +217,14 @@ func GetAllCommits(ctx *context.APIContext) {
 			return
 		}
 
-		commits, err = ctx.Repo.GitRepo.CommitsByFileAndRange(sha, path, listOptions.Page)
+		commits, err = ctx.Repo.GitRepo.CommitsByFileAndRange(
+			git.CommitsByFileAndRangeOptions{
+				Revision: sha,
+				File:     path,
+				Not:      not,
+				Page:     listOptions.Page,
+			})
+
 		if err != nil {
 			ctx.Error(http.StatusInternalServerError, "CommitsByFileAndRange", err)
 			return

routers/api/v1/repo/wiki.go

@@ -429,7 +429,12 @@ func ListPageRevisions(ctx *context.APIContext) {
 	}
 
 	// get Commit Count
-	commitsHistory, err := wikiRepo.CommitsByFileAndRange("master", pageFilename, page)
+	commitsHistory, err := wikiRepo.CommitsByFileAndRange(
+		git.CommitsByFileAndRangeOptions{
+			Revision: "master",
+			File:     pageFilename,
+			Page:     page,
+		})
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "CommitsByFileAndRange", err)
 		return

routers/web/feed/file.go

@@ -10,6 +10,7 @@ import (
 
 	"code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/modules/context"
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/util"
 
 	"github.com/gorilla/feeds"
@@ -21,7 +22,12 @@ func ShowFileFeed(ctx *context.Context, repo *repo.Repository, formatType string
 	if len(fileName) == 0 {
 		return
 	}
-	commits, err := ctx.Repo.GitRepo.CommitsByFileAndRange(ctx.Repo.RefName, fileName, 1)
+	commits, err := ctx.Repo.GitRepo.CommitsByFileAndRange(
+		git.CommitsByFileAndRangeOptions{
+			Revision: ctx.Repo.RefName,
+			File:     fileName,
+			Page:     1,
+		})
 	if err != nil {
 		ctx.ServerError("ShowBranchFeed", err)
 		return

routers/web/repo/commit.go

@@ -230,7 +230,12 @@ func FileHistory(ctx *context.Context) {
 		page = 1
 	}
 
-	commits, err := ctx.Repo.GitRepo.CommitsByFileAndRange(ctx.Repo.RefName, fileName, page)
+	commits, err := ctx.Repo.GitRepo.CommitsByFileAndRange(
+		git.CommitsByFileAndRangeOptions{
+			Revision: ctx.Repo.RefName,
+			File:     fileName,
+			Page:     page,
+		})
 	if err != nil {
 		ctx.ServerError("CommitsByFileAndRange", err)
 		return

routers/web/repo/wiki.go

@@ -376,7 +376,12 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry)
 	}
 
 	// get Commit Count
-	commitsHistory, err := wikiRepo.CommitsByFileAndRange(wiki_service.DefaultBranch, pageFilename, page)
+	commitsHistory, err := wikiRepo.CommitsByFileAndRange(
+		git.CommitsByFileAndRangeOptions{
+			Revision: wiki_service.DefaultBranch,
+			File:     pageFilename,
+			Page:     page,
+		})
 	if err != nil {
 		if wikiRepo != nil {
 			wikiRepo.Close()

tests/integration/api_repo_git_commits_test.go

@@ -58,6 +58,29 @@ func TestAPIReposGitCommitList(t *testing.T) {
 	session := loginUser(t, user.Name)
 	token := getTokenForLoggedInUser(t, session)
 
+	// Test getting commits (Page 1)
+	req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo20/commits?token="+token+"&not=master&sha=remove-files-a", user.Name)
+	resp := MakeRequest(t, req, http.StatusOK)
+
+	var apiData []api.Commit
+	DecodeJSON(t, resp, &apiData)
+
+	assert.Len(t, apiData, 2)
+	assert.EqualValues(t, "cfe3b3c1fd36fba04f9183287b106497e1afe986", apiData[0].CommitMeta.SHA)
+	compareCommitFiles(t, []string{"link_hi", "test.csv"}, apiData[0].Files)
+	assert.EqualValues(t, "c8e31bc7688741a5287fcde4fbb8fc129ca07027", apiData[1].CommitMeta.SHA)
+	compareCommitFiles(t, []string{"test.csv"}, apiData[1].Files)
+
+	assert.EqualValues(t, resp.Header().Get("X-Total"), "2")
+}
+
+func TestAPIReposGitCommitListNotMaster(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	// Login as User2.
+	session := loginUser(t, user.Name)
+	token := getTokenForLoggedInUser(t, session)
+
 	// Test getting commits (Page 1)
 	req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo16/commits?token="+token, user.Name)
 	resp := MakeRequest(t, req, http.StatusOK)
@@ -72,6 +95,8 @@ func TestAPIReposGitCommitList(t *testing.T) {
 	compareCommitFiles(t, []string{"readme.md"}, apiData[1].Files)
 	assert.EqualValues(t, "5099b81332712fe655e34e8dd63574f503f61811", apiData[2].CommitMeta.SHA)
 	compareCommitFiles(t, []string{"readme.md"}, apiData[2].Files)
+
+	assert.EqualValues(t, resp.Header().Get("X-Total"), "3")
 }
 
 func TestAPIReposGitCommitListPage2Empty(t *testing.T) {
@@ -148,4 +173,26 @@ func TestGetFileHistory(t *testing.T) {
 	assert.Len(t, apiData, 1)
 	assert.Equal(t, "f27c2b2b03dcab38beaf89b0ab4ff61f6de63441", apiData[0].CommitMeta.SHA)
 	compareCommitFiles(t, []string{"readme.md"}, apiData[0].Files)
+
+	assert.EqualValues(t, resp.Header().Get("X-Total"), "1")
+}
+
+func TestGetFileHistoryNotOnMaster(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	// Login as User2.
+	session := loginUser(t, user.Name)
+	token := getTokenForLoggedInUser(t, session)
+
+	req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo20/commits?path=test.csv&token="+token+"&sha=add-csv&not=master", user.Name)
+	resp := MakeRequest(t, req, http.StatusOK)
+
+	var apiData []api.Commit
+	DecodeJSON(t, resp, &apiData)
+
+	assert.Len(t, apiData, 1)
+	assert.Equal(t, "c8e31bc7688741a5287fcde4fbb8fc129ca07027", apiData[0].CommitMeta.SHA)
+	compareCommitFiles(t, []string{"test.csv"}, apiData[0].Files)
+
+	assert.EqualValues(t, resp.Header().Get("X-Total"), "1")
 }