From f5b7feea6012cb9505b919042f3e2c9ae58bf6d8 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Fri, 15 Sep 2017 20:47:50 +0200 Subject: [PATCH 1/2] Refactor git command creation --- internal/command/command.go | 5 --- internal/git/command.go | 22 +++++++++++ internal/git/log/commit.go | 11 +----- internal/helper/repo.go | 16 -------- internal/ref/helper.go | 23 ++++++++++++ internal/service/commit/commits_by_message.go | 13 +++---- internal/service/commit/count_commits.go | 16 ++++---- internal/service/commit/find_all_commits.go | 8 +--- .../service/commit/find_all_commits_test.go | 2 +- internal/service/commit/find_commits.go | 9 +---- internal/service/commit/isancestor.go | 17 ++++----- internal/service/commit/languages.go | 19 +++++----- internal/service/commit/list_files.go | 22 ++++++++--- internal/service/commit/list_files_test.go | 2 +- internal/service/commit/raw_blame.go | 14 +++---- internal/service/diff/commit.go | 25 +++++-------- internal/service/ref/refexists.go | 17 ++++----- internal/service/ref/refname.go | 19 +++++----- internal/service/ref/refs.go | 37 ++++++++----------- internal/service/ref/refs_test.go | 30 +++++++-------- internal/service/repository/gc.go | 16 ++++---- internal/service/repository/repack.go | 18 ++++----- internal/service/smarthttp/inforefs.go | 8 +++- 23 files changed, 181 insertions(+), 188 deletions(-) create mode 100644 internal/git/command.go create mode 100644 internal/ref/helper.go diff --git a/internal/command/command.go b/internal/command/command.go index b6d563aa58..6ad336d0bd 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -83,11 +83,6 @@ func GitPath() string { return config.Config.Git.BinPath } -// Git creates a git Command with the given args -func Git(ctx context.Context, args ...string) (*Command, error) { - return New(ctx, exec.Command(GitPath(), args...), nil, nil, nil) -} - // GitlabShell creates a gitlab-shell Command with the given args func GitlabShell(ctx context.Context, envs []string, executable string, args ...string) (*Command, error) { shellPath, ok := config.GitlabShellPath() diff --git a/internal/git/command.go b/internal/git/command.go new file mode 100644 index 0000000000..b219bf41b3 --- /dev/null +++ b/internal/git/command.go @@ -0,0 +1,22 @@ +package git + +import ( + "context" + "os/exec" + + "gitlab.com/gitlab-org/gitaly/internal/command" + "gitlab.com/gitlab-org/gitaly/internal/helper" + + pb "gitlab.com/gitlab-org/gitaly-proto/go" +) + +// Command creates a git.Command with the given args +func Command(ctx context.Context, repo *pb.Repository, args ...string) (*command.Command, error) { + repoPath, err := helper.GetRepoPath(repo) + if err != nil { + return nil, err + } + args = append([]string{"--git-dir", repoPath}, args...) + + return command.New(ctx, exec.Command(command.GitPath(), args...), nil, nil, nil) +} diff --git a/internal/git/log/commit.go b/internal/git/log/commit.go index a6d52ee8f7..95bcfa6fb7 100644 --- a/internal/git/log/commit.go +++ b/internal/git/log/commit.go @@ -5,7 +5,7 @@ import ( "strings" "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/git" pb "gitlab.com/gitlab-org/gitaly-proto/go" @@ -54,16 +54,9 @@ func GitLogCommand(ctx context.Context, repo *pb.Repository, revisions []string, "Revisions": revisions, }).Debug("GitLog") - repoPath, err := helper.GetRepoPath(repo) - if err != nil { - return nil, err - } - formatFlag := "--pretty=format:" + strings.Join(commitLogFormatFields, fieldDelimiterGitFormatString) args := []string{ - "--git-dir", - repoPath, "log", "-z", // use 0x00 as the entry terminator (instead of \n) formatFlag, @@ -73,7 +66,7 @@ func GitLogCommand(ctx context.Context, repo *pb.Repository, revisions []string, args = append(args, "--") args = append(args, paths...) - cmd, err := command.Git(ctx, args...) + cmd, err := git.Command(ctx, repo, args...) if err != nil { return nil, err } diff --git a/internal/helper/repo.go b/internal/helper/repo.go index d0badd3be5..326088814e 100644 --- a/internal/helper/repo.go +++ b/internal/helper/repo.go @@ -1,12 +1,10 @@ package helper import ( - "context" "os" "path" "strings" - "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/config" pb "gitlab.com/gitlab-org/gitaly-proto/go" @@ -92,17 +90,3 @@ func IsGitDirectory(dir string) bool { return true } - -// IsValidRef checks if a ref in a repo is valid -func IsValidRef(ctx context.Context, path, ref string) bool { - if path == "" || ref == "" { - return false - } - - cmd, err := command.Git(ctx, "--git-dir", path, "log", "--max-count=1", ref) - if err != nil { - return false - } - - return cmd.Wait() == nil -} diff --git a/internal/ref/helper.go b/internal/ref/helper.go new file mode 100644 index 0000000000..ee4baabc9f --- /dev/null +++ b/internal/ref/helper.go @@ -0,0 +1,23 @@ +package ref + +import ( + "context" + + "gitlab.com/gitlab-org/gitaly/internal/git" + + pb "gitlab.com/gitlab-org/gitaly-proto/go" +) + +// IsValidRef checks if a ref in a repo is valid +func IsValidRef(ctx context.Context, repo *pb.Repository, ref string) bool { + if ref == "" { + return false + } + + cmd, err := git.Command(ctx, repo, "log", "--max-count=1", ref) + if err != nil { + return false + } + + return cmd.Wait() == nil +} diff --git a/internal/service/commit/commits_by_message.go b/internal/service/commit/commits_by_message.go index 560e5119f5..0178b768b9 100644 --- a/internal/service/commit/commits_by_message.go +++ b/internal/service/commit/commits_by_message.go @@ -3,12 +3,11 @@ package commit import ( "fmt" - "gitlab.com/gitlab-org/gitaly/internal/helper" - pb "gitlab.com/gitlab-org/gitaly-proto/go" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) type commitsByMessageSender struct { @@ -38,13 +37,11 @@ func (s *server) CommitsByMessage(in *pb.CommitsByMessageRequest, stream pb.Comm if len(revision) == 0 { var err error - repoPath, err := helper.GetRepoPath(in.GetRepository()) - if err != nil { - return err - } - - revision, err = defaultBranchName(ctx, repoPath) + revision, err = defaultBranchName(ctx, in.Repository) if err != nil { + if _, ok := status.FromError(err); ok { + return err + } return grpc.Errorf(codes.Internal, "CommitsByMessage: defaultBranchName: %v", err) } } diff --git a/internal/service/commit/count_commits.go b/internal/service/commit/count_commits.go index a0c519a4aa..96545cc253 100644 --- a/internal/service/commit/count_commits.go +++ b/internal/service/commit/count_commits.go @@ -7,8 +7,7 @@ import ( "strconv" "time" - "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/git" pb "gitlab.com/gitlab-org/gitaly-proto/go" @@ -16,6 +15,7 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func (s *server) CountCommits(ctx context.Context, in *pb.CountCommitsRequest) (*pb.CountCommitsResponse, error) { @@ -23,12 +23,7 @@ func (s *server) CountCommits(ctx context.Context, in *pb.CountCommitsRequest) ( return nil, grpc.Errorf(codes.InvalidArgument, "CountCommits: %v", err) } - repoPath, err := helper.GetRepoPath(in.Repository) - if err != nil { - return nil, err - } - - cmdArgs := []string{"--git-dir", repoPath, "rev-list", "--count", string(in.GetRevision())} + cmdArgs := []string{"rev-list", "--count", string(in.GetRevision())} if before := in.GetBefore(); before != nil { cmdArgs = append(cmdArgs, "--before="+timestampToRFC3339(before.Seconds)) @@ -40,8 +35,11 @@ func (s *server) CountCommits(ctx context.Context, in *pb.CountCommitsRequest) ( cmdArgs = append(cmdArgs, "--", string(path)) } - cmd, err := command.Git(ctx, cmdArgs...) + cmd, err := git.Command(ctx, in.Repository, cmdArgs...) if err != nil { + if _, ok := status.FromError(err); ok { + return nil, err + } return nil, grpc.Errorf(codes.Internal, "CountCommits: cmd: %v", err) } diff --git a/internal/service/commit/find_all_commits.go b/internal/service/commit/find_all_commits.go index a46bf8465d..a3c44471b5 100644 --- a/internal/service/commit/find_all_commits.go +++ b/internal/service/commit/find_all_commits.go @@ -3,7 +3,6 @@ package commit import ( "fmt" - "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/service/ref" pb "gitlab.com/gitlab-org/gitaly-proto/go" @@ -40,12 +39,7 @@ func (s *server) FindAllCommits(in *pb.FindAllCommitsRequest, stream pb.CommitSe var revisions []string if len(in.GetRevision()) == 0 { - repoPath, err := helper.GetRepoPath(in.GetRepository()) - if err != nil { - return grpc.Errorf(codes.InvalidArgument, "FindAllCommits: %v", err) - } - - branchNames, err := _findBranchNamesFunc(stream.Context(), repoPath) + branchNames, err := _findBranchNamesFunc(stream.Context(), in.Repository) if err != nil { return grpc.Errorf(codes.InvalidArgument, "FindAllCommits: %v", err) } diff --git a/internal/service/commit/find_all_commits_test.go b/internal/service/commit/find_all_commits_test.go index 6785ebfa78..9994061f1a 100644 --- a/internal/service/commit/find_all_commits_test.go +++ b/internal/service/commit/find_all_commits_test.go @@ -24,7 +24,7 @@ func TestSuccessfulFindAllCommitsRequest(t *testing.T) { _findBranchNamesFunc = ref.FindBranchNames }() - _findBranchNamesFunc = func(ctx context.Context, repoPath string) ([][]byte, error) { + _findBranchNamesFunc = func(ctx context.Context, repo *pb.Repository) ([][]byte, error) { return [][]byte{ []byte("few-commits"), []byte("two-commits"), diff --git a/internal/service/commit/find_commits.go b/internal/service/commit/find_commits.go index 18053b9f7c..87bb256375 100644 --- a/internal/service/commit/find_commits.go +++ b/internal/service/commit/find_commits.go @@ -1,7 +1,6 @@ package commit import ( - "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/rubyserver" pb "gitlab.com/gitlab-org/gitaly-proto/go" @@ -16,12 +15,8 @@ func (s *server) FindCommits(req *pb.FindCommitsRequest, stream pb.CommitService // Use Gitaly's default branch lookup function because that is already // migrated. if revision := req.Revision; len(revision) == 0 { - repoPath, err := helper.GetRepoPath(req.Repository) - if err != nil { - return err - } - - req.Revision, err = defaultBranchName(ctx, repoPath) + var err error + req.Revision, err = defaultBranchName(ctx, req.Repository) if err != nil { return grpc.Errorf(codes.Internal, "defaultBranchName: %v", err) } diff --git a/internal/service/commit/isancestor.go b/internal/service/commit/isancestor.go index 3937cf43f6..4292b5c08f 100644 --- a/internal/service/commit/isancestor.go +++ b/internal/service/commit/isancestor.go @@ -4,21 +4,17 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "golang.org/x/net/context" log "github.com/sirupsen/logrus" pb "gitlab.com/gitlab-org/gitaly-proto/go" - "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/git" ) func (s *server) CommitIsAncestor(ctx context.Context, in *pb.CommitIsAncestorRequest) (*pb.CommitIsAncestorResponse, error) { - repoPath, err := helper.GetRepoPath(in.GetRepository()) - if err != nil { - return nil, err - } if in.AncestorId == "" { return nil, grpc.Errorf(codes.InvalidArgument, "Bad Request (empty ancestor sha)") } @@ -26,19 +22,22 @@ func (s *server) CommitIsAncestor(ctx context.Context, in *pb.CommitIsAncestorRe return nil, grpc.Errorf(codes.InvalidArgument, "Bad Request (empty child sha)") } - ret, err := commitIsAncestorName(ctx, repoPath, in.AncestorId, in.ChildId) + ret, err := commitIsAncestorName(ctx, in.Repository, in.AncestorId, in.ChildId) return &pb.CommitIsAncestorResponse{Value: ret}, err } // Assumes that `path`, `ancestorID` and `childID` are populated :trollface: -func commitIsAncestorName(ctx context.Context, path, ancestorID, childID string) (bool, error) { +func commitIsAncestorName(ctx context.Context, repo *pb.Repository, ancestorID, childID string) (bool, error) { grpc_logrus.Extract(ctx).WithFields(log.Fields{ "ancestorSha": ancestorID, "childSha": childID, }).Debug("commitIsAncestor") - cmd, err := command.Git(ctx, "--git-dir", path, "merge-base", "--is-ancestor", ancestorID, childID) + cmd, err := git.Command(ctx, repo, "merge-base", "--is-ancestor", ancestorID, childID) if err != nil { + if _, ok := status.FromError(err); ok { + return false, err + } return false, grpc.Errorf(codes.Internal, err.Error()) } diff --git a/internal/service/commit/languages.go b/internal/service/commit/languages.go index dddc842357..adcbcb3fd2 100644 --- a/internal/service/commit/languages.go +++ b/internal/service/commit/languages.go @@ -8,7 +8,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" - "gitlab.com/gitlab-org/gitaly/internal/command" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/linguist" "gitlab.com/gitlab-org/gitaly/internal/service/ref" @@ -19,25 +19,26 @@ import ( ) func (*server) CommitLanguages(ctx context.Context, req *pb.CommitLanguagesRequest) (*pb.CommitLanguagesResponse, error) { - repoPath, err := helper.GetRepoPath(req.Repository) - if err != nil { - return nil, err - } + repo := req.Repository revision := string(req.Revision) if revision == "" { - defaultBranch, err := ref.DefaultBranchName(ctx, repoPath) + defaultBranch, err := ref.DefaultBranchName(ctx, req.Repository) if err != nil { return nil, err } revision = string(defaultBranch) } - commitID, err := lookupRevision(ctx, repoPath, revision) + commitID, err := lookupRevision(ctx, repo, revision) if err != nil { return nil, err } + repoPath, err := helper.GetRepoPath(repo) + if err != nil { + return nil, err + } stats, err := linguist.Stats(ctx, repoPath, commitID) if err != nil { return nil, err @@ -77,8 +78,8 @@ func (ls languageSorter) Len() int { return len(ls) } func (ls languageSorter) Swap(i, j int) { ls[i], ls[j] = ls[j], ls[i] } func (ls languageSorter) Less(i, j int) bool { return ls[i].Share > ls[j].Share } -func lookupRevision(ctx context.Context, repoPath string, revision string) (string, error) { - revParse, err := command.Git(ctx, "--git-dir", repoPath, "rev-parse", revision) +func lookupRevision(ctx context.Context, repo *pb.Repository, revision string) (string, error) { + revParse, err := git.Command(ctx, repo, "rev-parse", revision) if err != nil { return "", err } diff --git a/internal/service/commit/list_files.go b/internal/service/commit/list_files.go index 4f1205e082..5c102ecd99 100644 --- a/internal/service/commit/list_files.go +++ b/internal/service/commit/list_files.go @@ -7,11 +7,13 @@ import ( log "github.com/sirupsen/logrus" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" pb "gitlab.com/gitlab-org/gitaly-proto/go" - "gitlab.com/gitlab-org/gitaly/internal/command" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/helper/lines" + "gitlab.com/gitlab-org/gitaly/internal/ref" ) func (s *server) ListFiles(in *pb.ListFilesRequest, stream pb.CommitService_ListFilesServer) error { @@ -19,24 +21,32 @@ func (s *server) ListFiles(in *pb.ListFilesRequest, stream pb.CommitService_List "Revision": in.GetRevision(), }).Debug("ListFiles") - repoPath, err := helper.GetRepoPath(in.Repository) - if err != nil { + repo := in.Repository + if _, err := helper.GetRepoPath(repo); err != nil { return err } revision := in.GetRevision() if len(revision) == 0 { - revision, err = defaultBranchName(stream.Context(), repoPath) + var err error + + revision, err = defaultBranchName(stream.Context(), repo) if err != nil { + if _, ok := status.FromError(err); ok { + return err + } return grpc.Errorf(codes.NotFound, "Revision not found %q", in.GetRevision()) } } - if !helper.IsValidRef(stream.Context(), repoPath, string(revision)) { + if !ref.IsValidRef(stream.Context(), repo, string(revision)) { return stream.Send(&pb.ListFilesResponse{}) } - cmd, err := command.Git(stream.Context(), "--git-dir", repoPath, "ls-tree", "-z", "-r", "--full-tree", "--full-name", "--", string(revision)) + cmd, err := git.Command(stream.Context(), repo, "ls-tree", "-z", "-r", "--full-tree", "--full-name", "--", string(revision)) if err != nil { + if _, ok := status.FromError(err); ok { + return err + } return grpc.Errorf(codes.Internal, err.Error()) } diff --git a/internal/service/commit/list_files_test.go b/internal/service/commit/list_files_test.go index 2185a25221..a5c488fddc 100644 --- a/internal/service/commit/list_files_test.go +++ b/internal/service/commit/list_files_test.go @@ -43,7 +43,7 @@ var ( ) func TestListFilesSuccess(t *testing.T) { - defaultBranchName = func(ctx context.Context, _ string) ([]byte, error) { + defaultBranchName = func(ctx context.Context, _ *pb.Repository) ([]byte, error) { return []byte("test-do-not-touch"), nil } defer func() { diff --git a/internal/service/commit/raw_blame.go b/internal/service/commit/raw_blame.go index 1bfca26470..9489533964 100644 --- a/internal/service/commit/raw_blame.go +++ b/internal/service/commit/raw_blame.go @@ -4,8 +4,7 @@ import ( "fmt" "io" - "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/streamio" pb "gitlab.com/gitlab-org/gitaly-proto/go" @@ -13,6 +12,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func (s *server) RawBlame(in *pb.RawBlameRequest, stream pb.CommitService_RawBlameServer) error { @@ -20,17 +20,15 @@ func (s *server) RawBlame(in *pb.RawBlameRequest, stream pb.CommitService_RawBla return grpc.Errorf(codes.InvalidArgument, "RawBlame: %v", err) } - repoPath, err := helper.GetRepoPath(in.Repository) - if err != nil { - return err - } - ctx := stream.Context() revision := string(in.GetRevision()) path := string(in.GetPath()) - cmd, err := command.Git(ctx, "--git-dir", repoPath, "blame", "-p", revision, "--", path) + cmd, err := git.Command(ctx, in.Repository, "blame", "-p", revision, "--", path) if err != nil { + if _, ok := status.FromError(err); ok { + return err + } return grpc.Errorf(codes.Internal, "RawBlame: cmd: %v", err) } diff --git a/internal/service/diff/commit.go b/internal/service/diff/commit.go index 11e03f2bb5..e323ff8c11 100644 --- a/internal/service/diff/commit.go +++ b/internal/service/diff/commit.go @@ -7,11 +7,11 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" log "github.com/sirupsen/logrus" pb "gitlab.com/gitlab-org/gitaly-proto/go" - "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/diff" - "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/git" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) type requestWithLeftRightCommitIds interface { @@ -31,17 +31,12 @@ func (s *server) CommitDiff(in *pb.CommitDiffRequest, stream pb.DiffService_Comm return grpc.Errorf(codes.InvalidArgument, "CommitDiff: %v", err) } - repoPath, err := helper.GetRepoPath(in.Repository) - if err != nil { - return err - } leftSha := in.LeftCommitId rightSha := in.RightCommitId ignoreWhitespaceChange := in.GetIgnoreWhitespaceChange() paths := in.GetPaths() cmdArgs := []string{ - "--git-dir", repoPath, "diff", "--patch", "--raw", @@ -72,7 +67,7 @@ func (s *server) CommitDiff(in *pb.CommitDiffRequest, stream pb.DiffService_Comm limits.SafeMaxLines = int(in.SafeMaxLines) limits.SafeMaxBytes = int(in.SafeMaxBytes) - err = eachDiff(stream.Context(), "CommitDiff", cmdArgs, limits, func(diff *diff.Diff) error { + err := eachDiff(stream.Context(), "CommitDiff", in.Repository, cmdArgs, limits, func(diff *diff.Diff) error { response := &pb.CommitDiffResponse{ FromPath: diff.FromPath, ToPath: diff.ToPath, @@ -131,16 +126,11 @@ func (s *server) CommitDelta(in *pb.CommitDeltaRequest, stream pb.DiffService_Co return grpc.Errorf(codes.InvalidArgument, "CommitDelta: %v", err) } - repoPath, err := helper.GetRepoPath(in.Repository) - if err != nil { - return err - } leftSha := in.LeftCommitId rightSha := in.RightCommitId paths := in.GetPaths() cmdArgs := []string{ - "--git-dir", repoPath, "diff", "--raw", "--abbrev=40", @@ -171,7 +161,7 @@ func (s *server) CommitDelta(in *pb.CommitDeltaRequest, stream pb.DiffService_Co return nil } - err = eachDiff(stream.Context(), "CommitDelta", cmdArgs, diff.Limits{}, func(diff *diff.Diff) error { + err := eachDiff(stream.Context(), "CommitDelta", in.Repository, cmdArgs, diff.Limits{}, func(diff *diff.Diff) error { delta := &pb.CommitDelta{ FromPath: diff.FromPath, ToPath: diff.ToPath, @@ -214,9 +204,12 @@ func validateRequest(in requestWithLeftRightCommitIds) error { return nil } -func eachDiff(ctx context.Context, rpc string, cmdArgs []string, limits diff.Limits, callback func(*diff.Diff) error) error { - cmd, err := command.Git(ctx, cmdArgs...) +func eachDiff(ctx context.Context, rpc string, repo *pb.Repository, cmdArgs []string, limits diff.Limits, callback func(*diff.Diff) error) error { + cmd, err := git.Command(ctx, repo, cmdArgs...) if err != nil { + if _, ok := status.FromError(err); ok { + return err + } return grpc.Errorf(codes.Internal, "%s: cmd: %v", rpc, err) } diff --git a/internal/service/ref/refexists.go b/internal/service/ref/refexists.go index 750fea9a65..817d627ab0 100644 --- a/internal/service/ref/refexists.go +++ b/internal/service/ref/refexists.go @@ -7,22 +7,18 @@ import ( log "github.com/sirupsen/logrus" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" pb "gitlab.com/gitlab-org/gitaly-proto/go" "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/git" "golang.org/x/net/context" ) // RefExists returns true if the given reference exists. The ref must start with the string `ref/` func (server) RefExists(ctx context.Context, in *pb.RefExistsRequest) (*pb.RefExistsResponse, error) { - repoPath, err := helper.GetRepoPath(in.Repository) - if err != nil { - return nil, err - } - ref := string(in.Ref) - exists, err := refExists(ctx, repoPath, ref) + exists, err := refExists(ctx, in.Repository, ref) if err != nil { return nil, err } @@ -30,7 +26,7 @@ func (server) RefExists(ctx context.Context, in *pb.RefExistsRequest) (*pb.RefEx return &pb.RefExistsResponse{Value: exists}, nil } -func refExists(ctx context.Context, repoPath string, ref string) (bool, error) { +func refExists(ctx context.Context, repo *pb.Repository, ref string) (bool, error) { grpc_logrus.Extract(ctx).WithFields(log.Fields{ "ref": ref, }).Debug("refExists") @@ -39,8 +35,11 @@ func refExists(ctx context.Context, repoPath string, ref string) (bool, error) { return false, grpc.Errorf(codes.InvalidArgument, "invalid refname") } - cmd, err := command.Git(ctx, "--git-dir", repoPath, "show-ref", "--verify", "--quiet", ref) + cmd, err := git.Command(ctx, repo, "show-ref", "--verify", "--quiet", ref) if err != nil { + if _, ok := status.FromError(err); ok { + return false, err + } return false, grpc.Errorf(codes.Internal, err.Error()) } diff --git a/internal/service/ref/refname.go b/internal/service/ref/refname.go index 5a007de75a..96145a56ec 100644 --- a/internal/service/ref/refname.go +++ b/internal/service/ref/refname.go @@ -6,12 +6,12 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" log "github.com/sirupsen/logrus" pb "gitlab.com/gitlab-org/gitaly-proto/go" - "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/git" "golang.org/x/net/context" ) @@ -19,30 +19,29 @@ import ( // If there is more than one such ref there is no guarantee which one is // returned or that the same one is returned on each call. func (s *server) FindRefName(ctx context.Context, in *pb.FindRefNameRequest) (*pb.FindRefNameResponse, error) { - repoPath, err := helper.GetRepoPath(in.Repository) - if err != nil { - return nil, err - } if in.CommitId == "" { return nil, grpc.Errorf(codes.InvalidArgument, "Bad Request (empty commit sha)") } - ref, err := findRefName(ctx, repoPath, in.CommitId, string(in.Prefix)) + ref, err := findRefName(ctx, in.Repository, in.CommitId, string(in.Prefix)) if err != nil { + if _, ok := status.FromError(err); ok { + return nil, err + } return nil, grpc.Errorf(codes.Internal, err.Error()) } return &pb.FindRefNameResponse{Name: []byte(ref)}, nil } -// We assume `path` and `commitID` and `prefix` are non-empty -func findRefName(ctx context.Context, path, commitID, prefix string) (string, error) { +// We assume `repo` and `commitID` and `prefix` are non-empty +func findRefName(ctx context.Context, repo *pb.Repository, commitID, prefix string) (string, error) { grpc_logrus.Extract(ctx).WithFields(log.Fields{ "commitSha": commitID, "prefix": prefix, }).Debug("findRefName") - cmd, err := command.Git(ctx, "--git-dir", path, "for-each-ref", "--format=%(refname)", "--count=1", prefix, "--contains", commitID) + cmd, err := git.Command(ctx, repo, "for-each-ref", "--format=%(refname)", "--count=1", prefix, "--contains", commitID) if err != nil { return "", err } diff --git a/internal/service/ref/refs.go b/internal/service/ref/refs.go index 18b17112fd..ab1de15385 100644 --- a/internal/service/ref/refs.go +++ b/internal/service/ref/refs.go @@ -10,10 +10,10 @@ import ( log "github.com/sirupsen/logrus" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" pb "gitlab.com/gitlab-org/gitaly-proto/go" - "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper/lines" "gitlab.com/gitlab-org/gitaly/internal/rubyserver" "golang.org/x/net/context" @@ -38,12 +38,7 @@ func findRefs(ctx context.Context, writer lines.Sender, repo *pb.Repository, pat "Patterns": patterns, }).Debug("FindRefs") - repoPath, err := helper.GetRepoPath(repo) - if err != nil { - return err - } - - baseArgs := []string{"--git-dir", repoPath, "for-each-ref"} + baseArgs := []string{"for-each-ref"} var args []string if len(opts.cmdArgs) == 0 { @@ -53,7 +48,7 @@ func findRefs(ctx context.Context, writer lines.Sender, repo *pb.Repository, pat } args = append(args, patterns...) - cmd, err := command.Git(ctx, args...) + cmd, err := git.Command(ctx, repo, args...) if err != nil { return err } @@ -102,10 +97,10 @@ func (s *server) FindAllTags(in *pb.FindAllTagsRequest, stream pb.RefService_Fin }) } -func _findBranchNames(ctx context.Context, repoPath string) ([][]byte, error) { +func _findBranchNames(ctx context.Context, repo *pb.Repository) ([][]byte, error) { var names [][]byte - cmd, err := command.Git(ctx, "--git-dir", repoPath, "for-each-ref", "refs/heads", "--format=%(refname)") + cmd, err := git.Command(ctx, repo, "for-each-ref", "refs/heads", "--format=%(refname)") if err != nil { return nil, err } @@ -125,10 +120,10 @@ func _findBranchNames(ctx context.Context, repoPath string) ([][]byte, error) { return names, nil } -func _headReference(ctx context.Context, repoPath string) ([]byte, error) { +func _headReference(ctx context.Context, repo *pb.Repository) ([]byte, error) { var headRef []byte - cmd, err := command.Git(ctx, "--git-dir", repoPath, "rev-parse", "--symbolic-full-name", "HEAD") + cmd, err := git.Command(ctx, repo, "rev-parse", "--symbolic-full-name", "HEAD") if err != nil { return nil, err } @@ -154,8 +149,8 @@ func _headReference(ctx context.Context, repoPath string) ([]byte, error) { } // DefaultBranchName looks up the name of the default branch given a repoPath -func DefaultBranchName(ctx context.Context, repoPath string) ([]byte, error) { - branches, err := FindBranchNames(ctx, repoPath) +func DefaultBranchName(ctx context.Context, repo *pb.Repository) ([]byte, error) { + branches, err := FindBranchNames(ctx, repo) if err != nil { return nil, err @@ -172,7 +167,7 @@ func DefaultBranchName(ctx context.Context, repoPath string) ([]byte, error) { } hasMaster := false - headRef, err := headReference(ctx, repoPath) + headRef, err := headReference(ctx, repo) if err != nil { return nil, err } @@ -198,13 +193,11 @@ func DefaultBranchName(ctx context.Context, repoPath string) ([]byte, error) { func (s *server) FindDefaultBranchName(ctx context.Context, in *pb.FindDefaultBranchNameRequest) (*pb.FindDefaultBranchNameResponse, error) { grpc_logrus.Extract(ctx).Debug("FindDefaultBranchName") - repoPath, err := helper.GetRepoPath(in.GetRepository()) - if err != nil { - return nil, err - } - - defaultBranchName, err := DefaultBranchName(ctx, repoPath) + defaultBranchName, err := DefaultBranchName(ctx, in.Repository) if err != nil { + if _, ok := status.FromError(err); ok { + return nil, err + } return nil, grpc.Errorf(codes.Internal, err.Error()) } diff --git a/internal/service/ref/refs_test.go b/internal/service/ref/refs_test.go index bc60bbf920..ccfbaf1ae3 100644 --- a/internal/service/ref/refs_test.go +++ b/internal/service/ref/refs_test.go @@ -199,7 +199,7 @@ func TestInvalidRepoFindAllTagNamesRequest(t *testing.T) { func TestHeadReference(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - headRef, err := headReference(ctx, testRepoPath) + headRef, err := headReference(ctx, testRepo) if err != nil { t.Fatal(err) } @@ -218,7 +218,7 @@ func TestHeadReferenceWithNonExistingHead(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - headRef, err := headReference(ctx, testRepoPath) + headRef, err := headReference(ctx, testRepo) if err != nil { t.Fatal(err) } @@ -236,47 +236,47 @@ func TestDefaultBranchName(t *testing.T) { testCases := []struct { desc string - findBranchNames func(context.Context, string) ([][]byte, error) - headReference func(context.Context, string) ([]byte, error) + findBranchNames func(context.Context, *pb.Repository) ([][]byte, error) + headReference func(context.Context, *pb.Repository) ([]byte, error) expected []byte }{ { desc: "Get first branch when only one branch exists", expected: []byte("refs/heads/foo"), - findBranchNames: func(context.Context, string) ([][]byte, error) { + findBranchNames: func(context.Context, *pb.Repository) ([][]byte, error) { return [][]byte{[]byte("refs/heads/foo")}, nil }, - headReference: func(context.Context, string) ([]byte, error) { return nil, nil }, + headReference: func(context.Context, *pb.Repository) ([]byte, error) { return nil, nil }, }, { desc: "Get empy ref if no branches exists", expected: nil, - findBranchNames: func(context.Context, string) ([][]byte, error) { return [][]byte{}, nil }, - headReference: func(context.Context, string) ([]byte, error) { return nil, nil }, + findBranchNames: func(context.Context, *pb.Repository) ([][]byte, error) { return [][]byte{}, nil }, + headReference: func(context.Context, *pb.Repository) ([]byte, error) { return nil, nil }, }, { desc: "Get the name of the head reference when more than one branch exists", expected: []byte("refs/heads/bar"), - findBranchNames: func(context.Context, string) ([][]byte, error) { + findBranchNames: func(context.Context, *pb.Repository) ([][]byte, error) { return [][]byte{[]byte("refs/heads/foo"), []byte("refs/heads/bar")}, nil }, - headReference: func(context.Context, string) ([]byte, error) { return []byte("refs/heads/bar"), nil }, + headReference: func(context.Context, *pb.Repository) ([]byte, error) { return []byte("refs/heads/bar"), nil }, }, { desc: "Get `ref/heads/master` when several branches exist", expected: []byte("refs/heads/master"), - findBranchNames: func(context.Context, string) ([][]byte, error) { + findBranchNames: func(context.Context, *pb.Repository) ([][]byte, error) { return [][]byte{[]byte("refs/heads/foo"), []byte("refs/heads/master"), []byte("refs/heads/bar")}, nil }, - headReference: func(context.Context, string) ([]byte, error) { return nil, nil }, + headReference: func(context.Context, *pb.Repository) ([]byte, error) { return nil, nil }, }, { desc: "Get the name of the first branch when several branches exists and no other conditions are met", expected: []byte("refs/heads/foo"), - findBranchNames: func(context.Context, string) ([][]byte, error) { + findBranchNames: func(context.Context, *pb.Repository) ([][]byte, error) { return [][]byte{[]byte("refs/heads/foo"), []byte("refs/heads/bar"), []byte("refs/heads/baz")}, nil }, - headReference: func(context.Context, string) ([]byte, error) { return nil, nil }, + headReference: func(context.Context, *pb.Repository) ([]byte, error) { return nil, nil }, }, } @@ -286,7 +286,7 @@ func TestDefaultBranchName(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - defaultBranch, err := DefaultBranchName(ctx, "") + defaultBranch, err := DefaultBranchName(ctx, testRepo) if err != nil { t.Fatal(err) } diff --git a/internal/service/repository/gc.go b/internal/service/repository/gc.go index ae5c86b1bd..105708d01d 100644 --- a/internal/service/repository/gc.go +++ b/internal/service/repository/gc.go @@ -6,10 +6,10 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" pb "gitlab.com/gitlab-org/gitaly-proto/go" - "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/git" ) func (server) GarbageCollect(ctx context.Context, in *pb.GarbageCollectRequest) (*pb.GarbageCollectResponse, error) { @@ -17,20 +17,18 @@ func (server) GarbageCollect(ctx context.Context, in *pb.GarbageCollectRequest) "WriteBitmaps": in.GetCreateBitmap(), }).Debug("GarbageCollect") - repoPath, err := helper.GetRepoPath(in.GetRepository()) - if err != nil { - return nil, err - } - - args := []string{"-C", repoPath, "-c"} + args := []string{"-c"} if in.GetCreateBitmap() { args = append(args, "repack.writeBitmaps=true") } else { args = append(args, "repack.writeBitmaps=false") } args = append(args, "gc") - cmd, err := command.Git(ctx, args...) + cmd, err := git.Command(ctx, in.GetRepository(), args...) if err != nil { + if _, ok := status.FromError(err); ok { + return nil, err + } return nil, grpc.Errorf(codes.Internal, err.Error()) } diff --git a/internal/service/repository/repack.go b/internal/service/repository/repack.go index 67dac910f7..e6478f527d 100644 --- a/internal/service/repository/repack.go +++ b/internal/service/repository/repack.go @@ -6,10 +6,10 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" pb "gitlab.com/gitlab-org/gitaly-proto/go" - "gitlab.com/gitlab-org/gitaly/internal/command" - "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/git" ) func (server) RepackFull(ctx context.Context, in *pb.RepackFullRequest) (*pb.RepackFullResponse, error) { @@ -31,21 +31,19 @@ func repackCommand(ctx context.Context, rpcName string, repo *pb.Repository, bit "WriteBitmaps": bitmap, }).Debug(rpcName) - repoPath, err := helper.GetRepoPath(repo) - if err != nil { - return err - } - var cmdArgs []string if bitmap { - cmdArgs = []string{"-C", repoPath, "-c", "repack.writeBitmaps=true", "repack", "-d"} + cmdArgs = []string{"-c", "repack.writeBitmaps=true", "repack", "-d"} } else { - cmdArgs = []string{"-C", repoPath, "-c", "repack.writeBitmaps=false", "repack", "-d"} + cmdArgs = []string{"-c", "repack.writeBitmaps=false", "repack", "-d"} } cmdArgs = append(cmdArgs, args...) - cmd, err := command.Git(ctx, cmdArgs...) + cmd, err := git.Command(ctx, repo, cmdArgs...) if err != nil { + if _, ok := status.FromError(err); ok { + return err + } return grpc.Errorf(codes.Internal, err.Error()) } diff --git a/internal/service/smarthttp/inforefs.go b/internal/service/smarthttp/inforefs.go index a4a245b1c7..21f1dd897d 100644 --- a/internal/service/smarthttp/inforefs.go +++ b/internal/service/smarthttp/inforefs.go @@ -8,12 +8,13 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" log "github.com/sirupsen/logrus" pb "gitlab.com/gitlab-org/gitaly-proto/go" - "gitlab.com/gitlab-org/gitaly/internal/command" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/streamio" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func (s *server) InfoRefsUploadPack(in *pb.InfoRefsRequest, stream pb.SmartHTTPService_InfoRefsUploadPackServer) error { @@ -40,8 +41,11 @@ func handleInfoRefs(ctx context.Context, service string, repo *pb.Repository, w return err } - cmd, err := command.Git(ctx, service, "--stateless-rpc", "--advertise-refs", repoPath) + cmd, err := git.Command(ctx, repo, service, "--stateless-rpc", "--advertise-refs", repoPath) if err != nil { + if _, ok := status.FromError(err); ok { + return err + } return grpc.Errorf(codes.Internal, "GetInfoRefs: cmd: %v", err) } -- GitLab From 8c119dcb8d36e248c45fa3df0e2bd485da90980b Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Fri, 15 Sep 2017 20:48:40 +0200 Subject: [PATCH 2/2] Change how we set git object dir attributes Fixes #564 --- internal/command/command.go | 10 --- internal/git/command.go | 14 ++- internal/{ref/helper.go => git/ref.go} | 6 +- .../objectdirhandler/objectdirhandler.go | 90 ------------------- internal/server/server.go | 3 - internal/service/commit/list_files.go | 3 +- internal/service/commit/testhelper_test.go | 7 +- internal/service/diff/commit.go | 4 +- 8 files changed, 18 insertions(+), 119 deletions(-) rename internal/{ref/helper.go => git/ref.go} (70%) delete mode 100644 internal/middleware/objectdirhandler/objectdirhandler.go diff --git a/internal/command/command.go b/internal/command/command.go index 6ad336d0bd..a2d7adabf2 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -8,7 +8,6 @@ import ( "os" "os/exec" "path" - "strings" "sync" "syscall" "time" @@ -16,7 +15,6 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" "gitlab.com/gitlab-org/gitaly/internal/config" - "gitlab.com/gitlab-org/gitaly/internal/middleware/objectdirhandler" log "github.com/sirupsen/logrus" ) @@ -127,14 +125,6 @@ func New(ctx context.Context, cmd *exec.Cmd, stdin io.Reader, stdout, stderr io. // Export env vars cmd.Env = exportEnvironment(env) - if dir, ok := objectdirhandler.ObjectDir(ctx); ok { - cmd.Env = append(cmd.Env, fmt.Sprintf("GIT_OBJECT_DIRECTORY=%s", dir)) - } - if dirs, ok := objectdirhandler.AltObjectDirs(ctx); ok { - dirsList := strings.Join(dirs, ":") - cmd.Env = append(cmd.Env, fmt.Sprintf("GIT_ALTERNATE_OBJECT_DIRECTORIES=%s", dirsList)) - } - // Start the command in its own process group (nice for signalling) cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} diff --git a/internal/git/command.go b/internal/git/command.go index b219bf41b3..043a0c5b53 100644 --- a/internal/git/command.go +++ b/internal/git/command.go @@ -2,7 +2,9 @@ package git import ( "context" + "fmt" "os/exec" + "strings" "gitlab.com/gitlab-org/gitaly/internal/command" "gitlab.com/gitlab-org/gitaly/internal/helper" @@ -18,5 +20,15 @@ func Command(ctx context.Context, repo *pb.Repository, args ...string) (*command } args = append([]string{"--git-dir", repoPath}, args...) - return command.New(ctx, exec.Command(command.GitPath(), args...), nil, nil, nil) + var env []string + if dir := repo.GetGitObjectDirectory(); dir != "" { + env = append(env, fmt.Sprintf("GIT_OBJECT_DIRECTORY=%s", dir)) + } + + if dirs := repo.GetGitAlternateObjectDirectories(); len(dirs) > 0 { + dirsList := strings.Join(dirs, ":") + env = append(env, fmt.Sprintf("GIT_ALTERNATE_OBJECT_DIRECTORIES=%s", dirsList)) + } + + return command.New(ctx, exec.Command(command.GitPath(), args...), nil, nil, nil, env...) } diff --git a/internal/ref/helper.go b/internal/git/ref.go similarity index 70% rename from internal/ref/helper.go rename to internal/git/ref.go index ee4baabc9f..2f5ec6f08b 100644 --- a/internal/ref/helper.go +++ b/internal/git/ref.go @@ -1,10 +1,8 @@ -package ref +package git import ( "context" - "gitlab.com/gitlab-org/gitaly/internal/git" - pb "gitlab.com/gitlab-org/gitaly-proto/go" ) @@ -14,7 +12,7 @@ func IsValidRef(ctx context.Context, repo *pb.Repository, ref string) bool { return false } - cmd, err := git.Command(ctx, repo, "log", "--max-count=1", ref) + cmd, err := Command(ctx, repo, "log", "--max-count=1", ref) if err != nil { return false } diff --git a/internal/middleware/objectdirhandler/objectdirhandler.go b/internal/middleware/objectdirhandler/objectdirhandler.go deleted file mode 100644 index 0eb40933e5..0000000000 --- a/internal/middleware/objectdirhandler/objectdirhandler.go +++ /dev/null @@ -1,90 +0,0 @@ -package objectdirhandler - -import ( - "sync" - - pb "gitlab.com/gitlab-org/gitaly-proto/go" - - "golang.org/x/net/context" - "google.golang.org/grpc" -) - -type requestWithRepository interface { - GetRepository() *pb.Repository -} - -type ctxObjectDirMarker struct{} -type ctxAltObjectDirsMarker struct{} - -var ( - ctxObjectDirMarkerKey = &ctxObjectDirMarker{} - ctxAltObjectDirsMarkerKey = &ctxAltObjectDirsMarker{} -) - -type recvWrapper struct { - grpc.ServerStream - wrappedContext context.Context - wrapOnce sync.Once -} - -// Unary sets Git object dir attributes for unary RPCs -func Unary(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { - return handler(newContextWithDirValues(ctx, req), req) -} - -// Stream sets Git object dir attributes for streaming RPCs -func Stream(srv interface{}, stream grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error { - return handler(srv, &recvWrapper{ServerStream: stream, wrappedContext: stream.Context()}) -} - -// ObjectDir returns the value for Repository.GitObjectDirectory -func ObjectDir(ctx context.Context) (string, bool) { - dir1 := ctx.Value(ctxObjectDirMarkerKey) - if dir1 == nil { - return "", false - } - - dir2, ok := dir1.(string) - return dir2, ok -} - -// AltObjectDirs returns the value for Repository.GitAlternateObjectDirectories -func AltObjectDirs(ctx context.Context) ([]string, bool) { - dirs1 := ctx.Value(ctxAltObjectDirsMarkerKey) - if dirs1 == nil { - return nil, false - } - - dirs2, ok := dirs1.([]string) - return dirs2, ok -} - -func newContextWithDirValues(ctx context.Context, req interface{}) context.Context { - if repo, ok := req.(requestWithRepository); ok { - if dir := repo.GetRepository().GetGitObjectDirectory(); dir != "" { - ctx = context.WithValue(ctx, ctxObjectDirMarkerKey, dir) - } - - if dirs := repo.GetRepository().GetGitAlternateObjectDirectories(); len(dirs) > 0 { - ctx = context.WithValue(ctx, ctxAltObjectDirsMarkerKey, dirs) - } - } - - return ctx -} - -func (s *recvWrapper) RecvMsg(m interface{}) error { - if err := s.ServerStream.RecvMsg(m); err != nil { - return err - } - - s.wrapOnce.Do(func() { - s.wrappedContext = newContextWithDirValues(s.wrappedContext, m) - }) - - return nil -} - -func (s *recvWrapper) Context() context.Context { - return s.wrappedContext -} diff --git a/internal/server/server.go b/internal/server/server.go index 51e7fd5470..4690bb5c32 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -5,7 +5,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/helper/fieldextractors" "gitlab.com/gitlab-org/gitaly/internal/middleware/cancelhandler" - "gitlab.com/gitlab-org/gitaly/internal/middleware/objectdirhandler" "gitlab.com/gitlab-org/gitaly/internal/middleware/panichandler" "gitlab.com/gitlab-org/gitaly/internal/middleware/sentryhandler" "gitlab.com/gitlab-org/gitaly/internal/rubyserver" @@ -31,7 +30,6 @@ func New(rubyServer *rubyserver.Server) *grpc.Server { server := grpc.NewServer( grpc.StreamInterceptor(grpc_middleware.ChainStreamServer( - objectdirhandler.Stream, grpc_ctxtags.StreamServerInterceptor(ctxTagOpts...), grpc_prometheus.StreamServerInterceptor, grpc_logrus.StreamServerInterceptor(logrusEntry), @@ -43,7 +41,6 @@ func New(rubyServer *rubyserver.Server) *grpc.Server { panichandler.StreamPanicHandler, )), grpc.UnaryInterceptor(grpc_middleware.ChainUnaryServer( - objectdirhandler.Unary, grpc_ctxtags.UnaryServerInterceptor(ctxTagOpts...), grpc_prometheus.UnaryServerInterceptor, grpc_logrus.UnaryServerInterceptor(logrusEntry), diff --git a/internal/service/commit/list_files.go b/internal/service/commit/list_files.go index 5c102ecd99..fdb7b5dcd3 100644 --- a/internal/service/commit/list_files.go +++ b/internal/service/commit/list_files.go @@ -13,7 +13,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/helper/lines" - "gitlab.com/gitlab-org/gitaly/internal/ref" ) func (s *server) ListFiles(in *pb.ListFilesRequest, stream pb.CommitService_ListFilesServer) error { @@ -38,7 +37,7 @@ func (s *server) ListFiles(in *pb.ListFilesRequest, stream pb.CommitService_List return grpc.Errorf(codes.NotFound, "Revision not found %q", in.GetRevision()) } } - if !ref.IsValidRef(stream.Context(), repo, string(revision)) { + if !git.IsValidRef(stream.Context(), repo, string(revision)) { return stream.Send(&pb.ListFilesResponse{}) } diff --git a/internal/service/commit/testhelper_test.go b/internal/service/commit/testhelper_test.go index b7df17ac8c..f5509a639b 100644 --- a/internal/service/commit/testhelper_test.go +++ b/internal/service/commit/testhelper_test.go @@ -8,7 +8,6 @@ import ( "time" "gitlab.com/gitlab-org/gitaly/internal/linguist" - "gitlab.com/gitlab-org/gitaly/internal/middleware/objectdirhandler" "gitlab.com/gitlab-org/gitaly/internal/rubyserver" "gitlab.com/gitlab-org/gitaly/internal/testhelper" @@ -49,11 +48,7 @@ func testMain(m *testing.M) int { } func startTestServices(t *testing.T) *grpc.Server { - server := testhelper.NewTestGrpcServer( - t, - []grpc.StreamServerInterceptor{objectdirhandler.Stream}, - []grpc.UnaryServerInterceptor{objectdirhandler.Unary}, - ) + server := testhelper.NewTestGrpcServer(t, nil, nil) if err := os.RemoveAll(serverSocketPath); err != nil { t.Fatal(err) diff --git a/internal/service/diff/commit.go b/internal/service/diff/commit.go index e323ff8c11..59b4788097 100644 --- a/internal/service/diff/commit.go +++ b/internal/service/diff/commit.go @@ -67,7 +67,7 @@ func (s *server) CommitDiff(in *pb.CommitDiffRequest, stream pb.DiffService_Comm limits.SafeMaxLines = int(in.SafeMaxLines) limits.SafeMaxBytes = int(in.SafeMaxBytes) - err := eachDiff(stream.Context(), "CommitDiff", in.Repository, cmdArgs, limits, func(diff *diff.Diff) error { + return eachDiff(stream.Context(), "CommitDiff", in.Repository, cmdArgs, limits, func(diff *diff.Diff) error { response := &pb.CommitDiffResponse{ FromPath: diff.FromPath, ToPath: diff.ToPath, @@ -111,8 +111,6 @@ func (s *server) CommitDiff(in *pb.CommitDiffRequest, stream pb.DiffService_Comm return nil }) - - return err } func (s *server) CommitDelta(in *pb.CommitDeltaRequest, stream pb.DiffService_CommitDeltaServer) error { -- GitLab