Merge branch '1175-rewrite-repositoryservice-fsck-in-go' into 'master'

Rewrite Repository::Fsck in Go

Closes #1175

See merge request gitlab-org/gitaly!738

7 files changed, 29 insertions, 9 deletions

diff --git a/.ruby-version b/.ruby-version
new file mode 100644
index 000000000..00355e29d
--- /dev/null
+++ b/.ruby-version
@@ -0,0 +1 @@
+2.3.7
diff --git a/changelogs/unreleased/1175-rewrite-repositoryservice-fsck-in-go.yml b/changelogs/unreleased/1175-rewrite-repositoryservice-fsck-in-go.yml
new file mode 100644
index 000000000..149d680a9
--- /dev/null
+++ b/changelogs/unreleased/1175-rewrite-repositoryservice-fsck-in-go.yml
@@ -0,0 +1,5 @@
+---
+title: Rewrite Repository::Fsck in Go
+merge_request: 738
+author:
+type: changed
diff --git a/internal/service/repository/fsck.go b/internal/service/repository/fsck.go
index 6286160b2..a7764661e 100644
--- a/internal/service/repository/fsck.go
+++ b/internal/service/repository/fsck.go
@@ -1,23 +1,34 @@
 package repository
 
 import (
+	"bytes"
+	"os/exec"
+
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/command"
+	"gitlab.com/gitlab-org/gitaly/internal/git/alternates"
 
 	"golang.org/x/net/context"
-
-	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
 )
 
 func (s *server) Fsck(ctx context.Context, req *pb.FsckRequest) (*pb.FsckResponse, error) {
-	client, err := s.RepositoryServiceClient(ctx)
+	var stdout, stderr bytes.Buffer
+
+	repoPath, env, err := alternates.PathAndEnv(req.GetRepository())
 	if err != nil {
 		return nil, err
 	}
 
-	clientCtx, err := rubyserver.SetHeaders(ctx, req.GetRepository())
+	args := []string{"--git-dir", repoPath, "fsck"}
+
+	cmd, err := command.New(ctx, exec.Command(command.GitPath(), args...), nil, &stdout, &stderr, env...)
 	if err != nil {
 		return nil, err
 	}
 
-	return client.Fsck(clientCtx, req)
+	if err = cmd.Wait(); err != nil {
+		return &pb.FsckResponse{Error: append(stdout.Bytes(), stderr.Bytes()...)}, nil
+	}
+
+	return &pb.FsckResponse{}, nil
 }
diff --git a/internal/service/repository/fsck_test.go b/internal/service/repository/fsck_test.go
index 6e457f924..e8777df67 100644
--- a/internal/service/repository/fsck_test.go
+++ b/internal/service/repository/fsck_test.go
@@ -78,5 +78,5 @@ func TestFsckFailureSlightlyBrokenRepo(t *testing.T) {
 	assert.NoError(t, err)
 	assert.NotNil(t, c)
 	assert.NotEmpty(t, string(c.GetError()))
-	assert.Contains(t, string(c.GetError()), "Could not fsck repository")
+	assert.Contains(t, string(c.GetError()), "error: HEAD: invalid sha1 pointer")
 }
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index d856fada2..31b90e8c6 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -133,11 +133,11 @@ func AssertGrpcError(t *testing.T, err error, expectedCode codes.Code, _deprecat
 	// Check that the code matches
 	status, _ := status.FromError(err)
 	if code := status.Code(); code != expectedCode {
-		t.Fatalf("Expected an error with code %v, got %v. The error was %v", expectedCode, code, err)
+		t.Fatalf("Expected an error with code '%v', got '%v'. The error was '%v'", expectedCode, code, err)
 	}
 
 	if _deprecated != "" && !strings.Contains(err.Error(), _deprecated) {
-		t.Fatalf("Expected an error message containing %v, got %v", _deprecated, err.Error())
+		t.Fatalf("Expected an error message containing '%v', got '%v'", _deprecated, err.Error())
 	}
 }
 
diff --git a/ruby/lib/gitaly_server/repository_service.rb b/ruby/lib/gitaly_server/repository_service.rb
index 48ee81a1a..ec913c363 100644
--- a/ruby/lib/gitaly_server/repository_service.rb
+++ b/ruby/lib/gitaly_server/repository_service.rb
@@ -24,6 +24,8 @@ module GitalyServer
       end
     end
 
+    # TODO: Can be removed once https://gitlab.com/gitlab-org/gitaly/merge_requests/738
+    #       is well and truly out in the wild.
     def fsck(request, call)
       repo = Gitlab::Git::Repository.from_gitaly(request.repository, call)
 
diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb
index b61c03b97..97d956309 100644
--- a/ruby/lib/gitlab/git/repository.rb
+++ b/ruby/lib/gitlab/git/repository.rb
@@ -94,7 +94,8 @@ module Gitlab
       end
 
       # This method is mandatory and no longer exists in gitlab-ce.
-      # TODO: implement it in Go because it is slow, and gitaly-ruby gets restarted a lot.
+      # TODO: Can be removed once https://gitlab.com/gitlab-org/gitaly/merge_requests/738
+      #       is well and truly out in the wild.
       def fsck
         msg, status = run_git(%W[--git-dir=#{path} fsck], nice: true)
         raise GitError.new("Could not fsck repository: #{msg}") unless status.zero?