Skip to content

Commit

Permalink
gitserver: Add CommitLog API to replace client-side Commits
Browse files Browse the repository at this point in the history
This PR moves the Commits API to the server side and is the last gRPC API to migrate.

Test plan:

Existing tests are still passing, moved the ~extensive unit test coverage this had on the client side to the server side.
  • Loading branch information
eseliger committed May 17, 2024
1 parent 2a5df05 commit b3a07ae
Show file tree
Hide file tree
Showing 34 changed files with 3,493 additions and 2,404 deletions.
21 changes: 19 additions & 2 deletions cmd/frontend/graphqlbackend/git_commits.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"strconv"
"sync"
"time"

"github.com/sourcegraph/sourcegraph/cmd/frontend/graphqlbackend/graphqlutil"
"github.com/sourcegraph/sourcegraph/internal/database"
Expand Down Expand Up @@ -89,14 +90,30 @@ func (r *gitCommitConnectionResolver) compute(ctx context.Context) ([]*gitdomain
return nil, errors.Wrap(err, "failed to resolve revision range")
}

var before, after time.Time

if r.after != nil {
after, err = gitdomain.ParseGitDate(*r.after, time.Now)
if err != nil {
return nil, errors.Wrap(err, "failed to parse after")
}
}

if r.before != nil {
before, err = gitdomain.ParseGitDate(*r.before, time.Now)
if err != nil {
return nil, errors.Wrap(err, "failed to parse before")
}
}

return r.gitserverClient.Commits(ctx, r.repo.RepoName(), gitserver.CommitsOptions{
Ranges: []string{cmp.Or(r.revisionRange, "HEAD")},
N: uint(n),
MessageQuery: pointers.DerefZero(r.query),
Author: pointers.DerefZero(r.author),
After: pointers.DerefZero(r.after),
After: after,
Skip: uint(afterCursor),
Before: pointers.DerefZero(r.before),
Before: before,
Path: pointers.DerefZero(r.path),
Follow: r.follow,
})
Expand Down
3 changes: 1 addition & 2 deletions cmd/frontend/graphqlbackend/repository_contributors.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/gitserver"
"github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain"
"github.com/sourcegraph/sourcegraph/internal/search/query"
"github.com/sourcegraph/sourcegraph/lib/errors"
)

Expand All @@ -27,7 +26,7 @@ func (r *RepositoryResolver) Contributors(args *struct {
var after time.Time
if args.AfterDate != nil && *args.AfterDate != "" {
var err error
after, err = query.ParseGitDate(*args.AfterDate, time.Now)
after, err = gitdomain.ParseGitDate(*args.AfterDate, time.Now)
if err != nil {
return nil, errors.Wrap(err, "failed to parse after date")
}
Expand Down
2 changes: 2 additions & 0 deletions cmd/gitserver/internal/git/gitcli/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ go_library(
"blame.go",
"clibackend.go",
"command.go",
"commitlog.go",
"commits.go",
"config.go",
"contributors.go",
Expand Down Expand Up @@ -54,6 +55,7 @@ go_test(
srcs = [
"archivereader_test.go",
"blame_test.go",
"commitlog_test.go",
"commits_test.go",
"config_test.go",
"contributors_test.go",
Expand Down
193 changes: 193 additions & 0 deletions cmd/gitserver/internal/git/gitcli/commitlog.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
package gitcli

import (
"bufio"
"bytes"
"context"
"io"
"strconv"
"time"

"github.com/sourcegraph/sourcegraph/cmd/gitserver/internal/git"
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain"
"github.com/sourcegraph/sourcegraph/lib/errors"
)

func (g *gitCLIBackend) CommitLog(ctx context.Context, opt git.CommitLogOpts) (git.CommitLogIterator, error) {
for _, r := range opt.Ranges {
if err := checkSpecArgSafety(r); err != nil {
return nil, err
}
}

if len(opt.Ranges) > 0 && opt.AllRefs {
return nil, errors.New("cannot specify both a Range and AllRefs")
}
if len(opt.Ranges) == 0 && !opt.AllRefs {
return nil, errors.New("must specify a Range or AllRefs")
}

args, err := buildCommitLogArgs(opt)
if err != nil {
return nil, err
}

r, err := g.NewCommand(ctx, WithArguments(args...))
if err != nil {
return nil, err
}

return newCommitLogIterator(g.repoName, r), nil
}

func buildCommitLogArgs(opt git.CommitLogOpts) ([]string, error) {
args := []string{"log", logFormatWithoutRefs}

if opt.MaxCommits != 0 {
args = append(args, "-n", strconv.FormatUint(uint64(opt.MaxCommits), 10))
}
if opt.Skip != 0 {
args = append(args, "--skip="+strconv.FormatUint(uint64(opt.Skip), 10))
}

if opt.AuthorQuery != "" {
args = append(args, "--fixed-strings", "--author="+opt.AuthorQuery)
}

if !opt.After.IsZero() {
args = append(args, "--after="+opt.After.Format(time.RFC3339))
}
if !opt.Before.IsZero() {
args = append(args, "--before="+opt.Before.Format(time.RFC3339))
}
switch opt.Order {
case git.CommitLogOrderCommitDate:
args = append(args, "--date-order")
case git.CommitLogOrderTopoDate:
args = append(args, "--topo-order")
case git.CommitLogOrderDefault:
// nothing to do
default:
return nil, errors.Newf("invalid ordering %d", opt.Order)
}

if opt.MessageQuery != "" {
args = append(args, "--fixed-strings", "--regexp-ignore-case", "--grep="+opt.MessageQuery)
}

if opt.FollowOnlyFirstParent {
args = append(args, "--first-parent")
}

args = append(args, opt.Ranges...)
if opt.AllRefs {
args = append(args, "--all")
}

if opt.IncludeModifiedFiles {
args = append(args, "--name-only")
}
if opt.FollowPathRenames {
args = append(args, "--follow")
}
if opt.Path != "" {
args = append(args, "--", opt.Path)
}

return args, nil
}

func newCommitLogIterator(repoName api.RepoName, r io.ReadCloser) *commitLogIterator {
commitScanner := bufio.NewScanner(r)
// We use an increased buffer size since sub-repo permissions
// can result in very lengthy output.
commitScanner.Buffer(make([]byte, 0, 65536), 4294967296)
commitScanner.Split(commitSplitFunc)

return &commitLogIterator{
Closer: r,
repoName: repoName,
sc: commitScanner,
}
}

type commitLogIterator struct {
io.Closer
repoName api.RepoName
sc *bufio.Scanner
}

func (it *commitLogIterator) Next() (*git.GitCommitWithFiles, error) {
if !it.sc.Scan() {
if err := it.sc.Err(); err != nil {
// If exit code is 128 and `fatal: bad object` is part of stderr, most likely we
// are referencing a commit that does not exist.
// We want to return a gitdomain.RevisionNotFoundError in that case.
var e *CommandFailedError
if errors.As(err, &e) && e.ExitStatus == 128 {
if bytes.Contains(e.Stderr, []byte("not a tree object")) || bytes.Contains(e.Stderr, []byte("fatal: bad object")) {
return nil, &gitdomain.RevisionNotFoundError{Repo: it.repoName, Spec: "TODO"} // string(opt.Range)}
}
if bytes.Contains(e.Stderr, []byte("fatal: ambiguous argument")) && bytes.Contains(e.Stderr, []byte("unknown revision or path not in the working tree.")) {
return nil, &gitdomain.RevisionNotFoundError{Repo: it.repoName, Spec: "TODO"} // string(opt.Range)}
}
if bytes.Contains(e.Stderr, []byte("fatal: your current branch")) && bytes.Contains(e.Stderr, []byte("does not have any commits yet")) {
return nil, io.EOF
}
}
return nil, err
}
return nil, io.EOF
}

rawCommit := it.sc.Bytes()
commit, err := parseCommitFromLog(rawCommit)
if err != nil {
return nil, err
}
return commit, nil
}

func (it *commitLogIterator) Close() error {
err := it.Closer.Close()
if err != nil {
var e *CommandFailedError
if errors.As(err, &e) && e.ExitStatus == 128 {
if bytes.Contains(e.Stderr, []byte("not a tree object")) || bytes.Contains(e.Stderr, []byte("fatal: bad object")) {
return &gitdomain.RevisionNotFoundError{Repo: it.repoName, Spec: "TODO"} // string(opt.Range)}
}
if bytes.Contains(e.Stderr, []byte("fatal: your current branch")) && bytes.Contains(e.Stderr, []byte("does not have any commits yet")) {
return nil
}
}
}
return err
}

func commitSplitFunc(data []byte, atEOF bool) (advance int, token []byte, err error) {
if len(data) == 0 {
// Request more data
return 0, nil, nil
}

// Safety check: ensure we are always starting with a record separator
if data[0] != '\x1e' {
return 0, nil, errors.New("internal error: data should always start with an ASCII record separator")
}

loc := bytes.IndexByte(data[1:], '\x1e')
if loc < 0 {
// We can't find the start of the next record
if atEOF {
// If we're at the end of the stream, just return the rest as the last record
return len(data), data[1:], bufio.ErrFinalToken
} else {
// If we're not at the end of the stream, request more data
return 0, nil, nil
}
}
nextStart := loc + 1 // correct for searching at an offset

return nextStart, data[1:nextStart], nil
}

0 comments on commit b3a07ae

Please sign in to comment.