repository: Stop calling CreateRepository to create hooks symlink

Back in the days, we used to set up Git hooks by creating a symlink to
our directory containing the actual hook scripts. This has long be
replaced by a much more robust mechanism which injects "core.hooksPath"
into Git processes to point to that directory instead. Since then, we
don't really need to setup hooks on-disk anymore, except if an admin
wants to create per-project server-side hooks.

Back when we used to create the symlink though we had to make sure that
all repos which got newly initialized did in fact have the symlink. To
do so, we used to call `CreateRepository()` after having set up repos,
which created the symlink for us. Given that the call is idempotent, it
was totally fine if the repository was already initialized: git-init(1)
simply re-copies the templates into the repository and creates anything
that's missing.

Nowadays this call is superfluous though: we have just created the repo,
we don't want to set up a symlink and thus it ultimately doesn't serve
any purpose at all anymore. So let's get rid of this useless step and
drop calls to `CreateRepository()` in `CreateRepositoryFromBundle()`,
`CreateRepositoryFromURL()` and `CreateFork()` and start to assert that
the hook directory is a directory, like it would be created by Git.

Note that this also removes the transactional voting step as performed
by CreateRepository. The voting in there is broken anyway given that it
doesn't guarantee any atomicity anyway. This is being remodelled in
another MR in the future which updates creation of repos to become
atomic, and dropping the CreateRepository call now goes into the same
direction.

6 files changed, 3 insertions, 32 deletions

diff --git a/internal/gitaly/service/repository/create_from_bundle.go b/internal/gitaly/service/repository/create_from_bundle.go
index 719a2c0d4..092b77861 100644
--- a/internal/gitaly/service/repository/create_from_bundle.go
+++ b/internal/gitaly/service/repository/create_from_bundle.go
@@ -114,12 +114,6 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr
 		return status.Error(codes.Internal, cleanError)
 	}
 
-	// CreateRepository is harmless on existing repositories with the side effect that it creates the hook symlink.
-	if _, err := s.CreateRepository(ctx, &gitalypb.CreateRepositoryRequest{Repository: repo}); err != nil {
-		cleanError := sanitizedError(repoPath, "CreateRepositoryFromBundle: create hooks failed: %v", err)
-		return status.Error(codes.Internal, cleanError)
-	}
-
 	return stream.SendAndClose(&gitalypb.CreateRepositoryFromBundleResponse{})
 }
 
diff --git a/internal/gitaly/service/repository/create_from_bundle_test.go b/internal/gitaly/service/repository/create_from_bundle_test.go
index 8ab62fedc..680a6a713 100644
--- a/internal/gitaly/service/repository/create_from_bundle_test.go
+++ b/internal/gitaly/service/repository/create_from_bundle_test.go
@@ -7,7 +7,6 @@ import (
 	"io"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -87,7 +86,7 @@ func TestServer_CreateRepositoryFromBundle_successful(t *testing.T) {
 
 	info, err := os.Lstat(filepath.Join(importedRepoPath, "hooks"))
 	require.NoError(t, err)
-	require.NotEqual(t, 0, info.Mode()&os.ModeSymlink)
+	require.True(t, info.IsDir())
 
 	commit, err := importedRepo.ReadCommit(ctx, "refs/custom-refs/ref1")
 	require.NoError(t, err)
@@ -167,18 +166,6 @@ func TestServerCreateRepositoryFromBundleTransactional(t *testing.T) {
 		fmt.Sprintf("%s %s refs/keep-around/2\n%s %s refs/keep-around/1\n", git.ZeroOID, masterOID, git.ZeroOID, masterOID),
 	)
 
-	// And this is the final vote in Create(), which does a git-for-each-ref(1) in the target
-	// repository and then manually invokes the hook. The format is thus different from above
-	// votes.
-	votingInput = append(votingInput,
-		strings.Join([]string{
-			featureOID + " commit\trefs/heads/feature",
-			masterOID + " commit\trefs/heads/master",
-			masterOID + " commit\trefs/keep-around/1",
-			masterOID + " commit\trefs/keep-around/2",
-		}, "\n")+"\n",
-	)
-
 	var expectedVotes []voting.Vote
 	for _, expectedVote := range votingInput {
 		expectedVotes = append(expectedVotes, voting.VoteFromData([]byte(expectedVote)))
diff --git a/internal/gitaly/service/repository/create_from_url.go b/internal/gitaly/service/repository/create_from_url.go
index 80bb761fd..e12c0c875 100644
--- a/internal/gitaly/service/repository/create_from_url.go
+++ b/internal/gitaly/service/repository/create_from_url.go
@@ -89,11 +89,6 @@ func (s *server) CreateRepositoryFromURL(ctx context.Context, req *gitalypb.Crea
 		return nil, status.Errorf(codes.Internal, "CreateRepositoryFromURL: clone cmd wait: %s: %v", stderr.String(), err)
 	}
 
-	// CreateRepository is harmless on existing repositories with the side effect that it creates the hook symlink.
-	if _, err := s.CreateRepository(ctx, &gitalypb.CreateRepositoryRequest{Repository: repository}); err != nil {
-		return nil, status.Errorf(codes.Internal, "CreateRepositoryFromURL: create hooks failed: %v", err)
-	}
-
 	if err := s.removeOriginInRepo(ctx, repository); err != nil {
 		return nil, status.Errorf(codes.Internal, "CreateRepositoryFromURL: %v", err)
 	}
diff --git a/internal/gitaly/service/repository/create_from_url_test.go b/internal/gitaly/service/repository/create_from_url_test.go
index 527ccea07..d610f0133 100644
--- a/internal/gitaly/service/repository/create_from_url_test.go
+++ b/internal/gitaly/service/repository/create_from_url_test.go
@@ -55,7 +55,7 @@ func TestSuccessfulCreateRepositoryFromURLRequest(t *testing.T) {
 
 	info, err := os.Lstat(filepath.Join(importedRepoPath, "hooks"))
 	require.NoError(t, err)
-	require.NotEqual(t, 0, info.Mode()&os.ModeSymlink)
+	require.True(t, info.IsDir())
 }
 
 func TestCloneRepositoryFromUrlCommand(t *testing.T) {
diff --git a/internal/gitaly/service/repository/fork.go b/internal/gitaly/service/repository/fork.go
index a6f7196bb..610fdf55a 100644
--- a/internal/gitaly/service/repository/fork.go
+++ b/internal/gitaly/service/repository/fork.go
@@ -79,10 +79,5 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest
 		return nil, status.Errorf(codes.Internal, "CreateFork: %v", err)
 	}
 
-	// CreateRepository is harmless on existing repositories with the side effect that it creates the hook symlink.
-	if _, err := s.CreateRepository(ctx, &gitalypb.CreateRepositoryRequest{Repository: targetRepository}); err != nil {
-		return nil, status.Errorf(codes.Internal, "CreateFork: create hooks failed: %v", err)
-	}
-
 	return &gitalypb.CreateForkResponse{}, nil
 }
diff --git a/internal/gitaly/service/repository/fork_test.go b/internal/gitaly/service/repository/fork_test.go
index 284325b44..1e874719a 100644
--- a/internal/gitaly/service/repository/fork_test.go
+++ b/internal/gitaly/service/repository/fork_test.go
@@ -113,7 +113,7 @@ func TestSuccessfulCreateForkRequest(t *testing.T) {
 
 			info, err := os.Lstat(filepath.Join(forkedRepoPath, "hooks"))
 			require.NoError(t, err)
-			require.NotEqual(t, 0, info.Mode()&os.ModeSymlink)
+			require.True(t, info.IsDir())
 		})
 	}
 }