gitserver: Implement CommitLog using Commits

This PR reimplements what CommitLog does using Commits. This reduces the surface area, but also removes an API that currently wasn't respecting sub repo permissions properly.
Another side-effect of this is that we have to make more gRPC requests for indexing, but in return we accumulate less data in memory as we don't need to store all paths in all diffs here.

Test plan:

Existing tests still pass, adjusted test suite as well.

internal/gitserver/BUILD.bazel

@@ -80,7 +80,6 @@ go_test(
         "@com_github_google_go_cmp//cmp",
         "@com_github_google_go_cmp//cmp/cmpopts",
         "@com_github_sourcegraph_go_diff//diff",
-        "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
         "@org_golang_google_grpc//:go_default_library",
         "@org_golang_google_grpc//codes",

internal/gitserver/client.go

@@ -253,14 +253,6 @@ func (o *BlameRange) Attrs() []attribute.KeyValue {
 	}
 }
 
-type CommitLog struct {
-	AuthorEmail  string
-	AuthorName   string
-	Timestamp    time.Time
-	SHA          string
-	ChangedFiles []string
-}
-
 // ListRefsOpts are additional options passed to ListRefs.
 type ListRefsOpts struct {
 	// If true, only heads are returned. Can be combined with TagsOnly.
@@ -459,11 +451,6 @@ type Client interface {
 	// longer required.
 	Diff(ctx context.Context, repo api.RepoName, opts DiffOptions) (*DiffFileIterator, error)
 
-	// CommitLog returns the repository commit log, including the file paths that were changed. The general approach to parsing
-	// is to separate the first line (the metadata line) from the remaining lines (the files), and then parse the metadata line
-	// into component parts separately.
-	CommitLog(ctx context.Context, repo api.RepoName, after time.Time) ([]CommitLog, error)
-
 	// GetCommit returns the commit with the given commit ID, or RevisionNotFoundError if no such commit
 	// exists.
 	GetCommit(ctx context.Context, repo api.RepoName, id api.CommitID) (*gitdomain.Commit, error)

internal/gitserver/commands.go

@@ -221,75 +221,6 @@ func checkSpecArgSafety(spec string) error {
 	return nil
 }
 
-// CommitLog returns the repository commit log, including the file paths that were changed. The general approach to parsing
-// is to separate the first line (the metadata line) from the remaining lines (the files), and then parse the metadata line
-// into component parts separately.
-func (c *clientImplementor) CommitLog(ctx context.Context, repo api.RepoName, after time.Time) (_ []CommitLog, err error) {
-	ctx, _, endObservation := c.operations.commitLog.With(ctx, &err, observation.Args{
-		MetricLabelValues: []string{c.scope},
-		Attrs: []attribute.KeyValue{
-			repo.Attr(),
-			attribute.Stringer("after", after),
-		},
-	})
-	defer endObservation(1, observation.Args{})
-
-	args := []string{"log", "--pretty=format:%H<!>%ae<!>%an<!>%ad", "--name-only", "--topo-order", "--no-merges"}
-	if !after.IsZero() {
-		args = append(args, fmt.Sprintf("--after=%s", after.Format(time.RFC3339)))
-	}
-
-	cmd := c.gitCommand(repo, args...)
-	out, err := cmd.CombinedOutput(ctx)
-	if err != nil {
-		return nil, errors.Wrapf(err, "gitCommand %s", string(out))
-	}
-
-	var ls []CommitLog
-	lines := strings.Split(string(out), "\n\n")
-
-	for _, logOutput := range lines {
-		partitions := strings.Split(logOutput, "\n")
-		if len(partitions) < 2 {
-			continue
-		}
-		metaLine := partitions[0]
-		var changedFiles []string
-		for _, pt := range partitions[1:] {
-			if pt != "" {
-				changedFiles = append(changedFiles, pt)
-			}
-		}
-
-		parts := strings.Split(metaLine, "<!>")
-		if len(parts) != 4 {
-			continue
-		}
-		sha, authorEmail, authorName, timestamp := parts[0], parts[1], parts[2], parts[3]
-		t, err := parseTimestamp(timestamp)
-		if err != nil {
-			return nil, errors.Wrapf(err, "parseTimestamp %s", timestamp)
-		}
-		ls = append(ls, CommitLog{
-			SHA:          sha,
-			AuthorEmail:  authorEmail,
-			AuthorName:   authorName,
-			Timestamp:    t,
-			ChangedFiles: changedFiles,
-		})
-	}
-	return ls, nil
-}
-
-func parseTimestamp(timestamp string) (time.Time, error) {
-	layout := "Mon Jan 2 15:04:05 2006 -0700"
-	t, err := time.Parse(layout, timestamp)
-	if err != nil {
-		return time.Time{}, err
-	}
-	return t, nil
-}
-
 // DevNullSHA 4b825dc642cb6eb9a060e54bf8d69288fbee4904 is `git hash-object -t
 // tree /dev/null`, which is used as the base when computing the `git diff` of
 // the root commit.

internal/gitserver/commands_test.go

@@ -12,7 +12,6 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp/cmpopts"
-	"github.com/stretchr/testify/assert"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -1253,57 +1252,6 @@ func TestClient_GetCommit(t *testing.T) {
 	})
 }
 
-func Test_CommitLog(t *testing.T) {
-	ClientMocks.LocalGitserver = true
-	defer ResetClientMocks()
-
-	tests := map[string]struct {
-		extraGitCommands []string
-		wantFiles        [][]string // put these in log reverse order
-		wantCommits      int
-		wantErr          string
-	}{
-		"commit changes files": {
-			extraGitCommands: getGitCommandsWithFileLists([]string{"file1.txt", "file2.txt"}, []string{"file3.txt"}),
-			wantFiles:        [][]string{{"file3.txt"}, {"file1.txt", "file2.txt"}},
-			wantCommits:      2,
-		},
-		"no commits": {
-			wantErr: "gitCommand fatal: your current branch 'master' does not have any commits yet: exit status 128",
-		},
-		"one file two commits": {
-			extraGitCommands: getGitCommandsWithFileLists([]string{"file1.txt"}, []string{"file1.txt"}),
-			wantFiles:        [][]string{{"file1.txt"}, {"file1.txt"}},
-			wantCommits:      2,
-		},
-		"one commit": {
-			extraGitCommands: getGitCommandsWithFileLists([]string{"file1.txt"}),
-			wantFiles:        [][]string{{"file1.txt"}},
-			wantCommits:      1,
-		},
-	}
-
-	for label, test := range tests {
-		t.Run(label, func(t *testing.T) {
-			repo := MakeGitRepository(t, test.extraGitCommands...)
-			logResults, err := NewClient("test").CommitLog(context.Background(), repo, time.Time{})
-			if err != nil {
-				require.ErrorContains(t, err, test.wantErr)
-			}
-
-			t.Log(test)
-			for i, result := range logResults {
-				t.Log(result)
-				assert.Equal(t, "a@a.com", result.AuthorEmail)
-				assert.Equal(t, "a", result.AuthorName)
-				assert.Equal(t, 40, len(result.SHA))
-				assert.ElementsMatch(t, test.wantFiles[i], result.ChangedFiles)
-			}
-			assert.Equal(t, test.wantCommits, len(logResults))
-		})
-	}
-}
-
 func TestClient_ArchiveReader(t *testing.T) {
 	t.Run("firstChunk memoization", func(t *testing.T) {
 		source := NewTestClientSource(t, []string{"gitserver"}, func(o *TestClientSourceOptions) {