diff options
| -rw-r--r-- | internal/gitaly/service/repository/objects_size.go | 41 | ||||
| -rw-r--r-- | internal/gitaly/service/repository/objects_size_test.go | 21 | ||||
| -rw-r--r-- | proto/go/gitalypb/repository.pb.go | 4 | ||||
| -rw-r--r-- | proto/repository.proto | 4 |
4 files changed, 45 insertions, 25 deletions
diff --git a/internal/gitaly/service/repository/objects_size.go b/internal/gitaly/service/repository/objects_size.go index 665cb87ef..8bbccb407 100644 --- a/internal/gitaly/service/repository/objects_size.go +++ b/internal/gitaly/service/repository/objects_size.go @@ -15,9 +15,8 @@ import ( ) // revisionNotFoundRegexp is used to parse the standard error of git-rev-list(1) in order to figure out whether the -// actual error condition was that the revision could not be found. Checking for ambiguous arguments is kind of awkward, -// but we can't really get around that until we have migrated to `--stdin` with Git v2.42 and later. -var revisionNotFoundRegexp = regexp.MustCompile("^fatal: ambiguous argument '([^']*)': unknown revision or path not in the working tree.") +// actual error condition was that the revision could not be found. +var revisionNotFoundRegexp = regexp.MustCompile("^fatal: bad revision '([^']*)'") // ObjectsSize calculates the on-disk object size via a graph walk. The intent of this RPC is to // calculate the on-disk size as accurately as possible. @@ -35,11 +34,23 @@ func (s *server) ObjectsSize(server gitalypb.RepositoryService_ObjectsSizeServer repo := s.localrepo(request.GetRepository()) - revlistArgs := make([]string, 0, len(request.GetRevisions())) + var stderr, stdout strings.Builder + cmd, err := repo.Exec(ctx, + git.Command{ + Name: "rev-list", + Flags: []git.Option{ + git.Flag{Name: "--disk-usage"}, + git.Flag{Name: "--objects"}, + git.Flag{Name: "--stdin"}, + }, + }, + git.WithStderr(&stderr), + git.WithStdout(&stdout), + git.WithSetupStdin()) + if err != nil { + return fmt.Errorf("start rev-list command: %w", err) + } - // TODO: Git v2.41 and older do not support passing pseudo-options via `--stdin`. We have upstreamed patches to - // fix this in Git v2.42, so once we have upgraded our Git version we should convert this loop to instead stream - // to git-rev-list(1)'s standard input. for i := 0; ; i++ { if i != 0 && request.GetRepository() != nil { return structerr.NewInvalidArgument("subsequent requests must not contain repository") @@ -54,7 +65,11 @@ func (s *server) ObjectsSize(server gitalypb.RepositoryService_ObjectsSizeServer return structerr.NewInvalidArgument("validating revision: %w", err).WithMetadata("revision", revision) } - revlistArgs = append(revlistArgs, string(revision)) + // Each revision must be separated by a newline when the `--stdin` option is used, as Git + // parses these differently to command-line arguments. + if _, err := cmd.Write([]byte(fmt.Sprintf("%s\n", revision))); err != nil { + return structerr.NewInvalidArgument("process revision: %w", err).WithMetadata("revision", revision) + } } request, err = server.Recv() @@ -67,15 +82,7 @@ func (s *server) ObjectsSize(server gitalypb.RepositoryService_ObjectsSizeServer } } - var stderr, stdout strings.Builder - if err := repo.ExecAndWait(ctx, git.Command{ - Name: "rev-list", - Flags: []git.Option{ - git.Flag{Name: "--disk-usage"}, - git.Flag{Name: "--objects"}, - }, - Args: revlistArgs, - }, git.WithStderr(&stderr), git.WithStdout(&stdout)); err != nil { + if err := cmd.Wait(); err != nil { var exitErr *exec.ExitError if errors.As(err, &exitErr) { if matches := revisionNotFoundRegexp.FindStringSubmatch(stderr.String()); len(matches) == 2 { diff --git a/internal/gitaly/service/repository/objects_size_test.go b/internal/gitaly/service/repository/objects_size_test.go index 90f798245..11723ddfe 100644 --- a/internal/gitaly/service/repository/objects_size_test.go +++ b/internal/gitaly/service/repository/objects_size_test.go @@ -130,6 +130,27 @@ func TestObjectsSize(t *testing.T) { }, }, { + desc: "revision with newline character", + setup: func(t *testing.T) setupData { + repo, _ := gittest.CreateRepository(t, ctx, cfg) + + return setupData{ + requests: []*gitalypb.ObjectsSizeRequest{ + { + Repository: repo, + Revisions: [][]byte{ + []byte("HEAD\n"), + }, + }, + }, + expectedErr: testhelper.WithInterceptedMetadata( + structerr.NewInvalidArgument("validating revision: revision can't contain whitespace"), + "revision", []byte("HEAD\n"), + ), + } + }, + }, + { desc: "missing revision", setup: func(t *testing.T) setupData { repo, _ := gittest.CreateRepository(t, ctx, cfg) diff --git a/proto/go/gitalypb/repository.pb.go b/proto/go/gitalypb/repository.pb.go index 171bd012b..ca005e2b5 100644 --- a/proto/go/gitalypb/repository.pb.go +++ b/proto/go/gitalypb/repository.pb.go @@ -531,10 +531,6 @@ type ObjectsSizeRequest struct { // revisions is the set of revisions that shall be used to compute the object size for. Supports normal revisions as // well as pseudo-revisions like `--not`, `--all`, `--branches[=pattern]`, `--tags[=pattern]` and `--glob=pattern`. // Please refer to the man pages gitrevisions(7) as well as git-rev-list(1) for more information. - // - // Note: due to a restriction in Git v2.41 and older all arguments need to be passed via command line parameters. So - // even though you can send multiple requests, the actual number of revisions that can be passed is limited by the - // maximum command line length. This limitation will be lifted once Gitaly has upgraded to Git v2.42 and later. Revisions [][]byte `protobuf:"bytes,2,rep,name=revisions,proto3" json:"revisions,omitempty"` } diff --git a/proto/repository.proto b/proto/repository.proto index 0445ce3a4..25d446f61 100644 --- a/proto/repository.proto +++ b/proto/repository.proto @@ -511,10 +511,6 @@ message ObjectsSizeRequest { // revisions is the set of revisions that shall be used to compute the object size for. Supports normal revisions as // well as pseudo-revisions like `--not`, `--all`, `--branches[=pattern]`, `--tags[=pattern]` and `--glob=pattern`. // Please refer to the man pages gitrevisions(7) as well as git-rev-list(1) for more information. - // - // Note: due to a restriction in Git v2.41 and older all arguments need to be passed via command line parameters. So - // even though you can send multiple requests, the actual number of revisions that can be passed is limited by the - // maximum command line length. This limitation will be lifted once Gitaly has upgraded to Git v2.42 and later. repeated bytes revisions = 2; } |