Stop symlinking hooks on repository creation

In an earlier MR[1] hooks are executed not through the symlink, but
leveraging the `-c` flag on the `git` binary. This works well, but when
`#run_git` was executed in Gitaly-Ruby the hooks weren't executed the
new way.

Luckily this was covered by tests, and the symlinking strategy remained
employed. This commit fixes the bug where the hooks weren't executed and
now allows the removal of the code that set the hooks.

Part of: #1226

[1]: !886

5 files changed, 6 insertions, 93 deletions

diff --git a/changelogs/unreleased/zj-stop-symlinking-hooks.yml b/changelogs/unreleased/zj-stop-symlinking-hooks.yml
new file mode 100644
index 000000000..b366b813e
--- /dev/null
+++ b/changelogs/unreleased/zj-stop-symlinking-hooks.yml
@@ -0,0 +1,6 @@
+---
+title: Stop symlinking hooks on repository creation
+merge_request: 1052
+author:
+type: added
+
diff --git a/internal/service/repository/create_from_snapshot_test.go b/internal/service/repository/create_from_snapshot_test.go
index e252ce9e2..b092479d0 100644
--- a/internal/service/repository/create_from_snapshot_test.go
+++ b/internal/service/repository/create_from_snapshot_test.go
@@ -106,10 +106,6 @@ func TestCreateRepositoryFromSnapshotSuccess(t *testing.T) {
 
 	// hooks/ and config were excluded, but the RPC should create them
 	require.FileExists(t, filepath.Join(repoPath, "config"), "Config file not created")
-
-	fi, err := os.Lstat(filepath.Join(repoPath, "hooks"))
-	require.NoError(t, err)
-	require.Equal(t, os.ModeSymlink, fi.Mode()&os.ModeSymlink, "Symlink to global hooks not created")
 }
 
 func TestCreateRepositoryFromSnapshotFailsIfRepositoryExists(t *testing.T) {
diff --git a/internal/service/repository/create_test.go b/internal/service/repository/create_test.go
index 7a36c40f7..e238f8e18 100644
--- a/internal/service/repository/create_test.go
+++ b/internal/service/repository/create_test.go
@@ -8,7 +8,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
-	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"google.golang.org/grpc/codes"
@@ -40,16 +39,6 @@ func TestCreateRepositorySuccess(t *testing.T) {
 		require.NoError(t, err)
 		require.True(t, fi.IsDir(), "%q must be a directory", fi.Name())
 	}
-
-	hooksDir := path.Join(repoDir, "hooks")
-
-	fi, err := os.Lstat(hooksDir)
-	require.NoError(t, err)
-	require.True(t, fi.Mode()&os.ModeSymlink > 0, "expected %q to be a symlink, got mode %v", hooksDir, fi.Mode())
-
-	hooksTarget, err := os.Readlink(hooksDir)
-	require.NoError(t, err)
-	require.Equal(t, path.Join(config.Config.GitlabShell.Dir, "hooks"), hooksTarget)
 }
 
 func TestCreateRepositoryFailure(t *testing.T) {
diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb
index 73373a0a9..f511b3ef6 100644
--- a/ruby/lib/gitlab/git/repository.rb
+++ b/ruby/lib/gitlab/git/repository.rb
@@ -61,37 +61,6 @@ module Gitlab
           # Equivalent to `git --git-path=#{repo_path} init [--bare]`
           repo = Rugged::Repository.init_at(repo_path, true)
           repo.close
-
-          # TODO: stop symlinking to the old hooks location in or after GitLab 12.0.
-          # https://gitlab.com/gitlab-org/gitaly/issues/1392
-          create_hooks(repo_path, Gitlab::Git::Hook.legacy_hooks_directory)
-        end
-
-        def create_hooks(repo_path, global_hooks_path)
-          local_hooks_path = File.join(repo_path, 'hooks')
-          real_local_hooks_path = :not_found
-
-          begin
-            real_local_hooks_path = File.realpath(local_hooks_path)
-          rescue Errno::ENOENT
-            # real_local_hooks_path == :not_found
-          end
-
-          # Do nothing if hooks already exist
-          unless real_local_hooks_path == File.realpath(global_hooks_path)
-            if File.exist?(local_hooks_path)
-              # Move the existing hooks somewhere safe
-              FileUtils.mv(
-                local_hooks_path,
-                "#{local_hooks_path}.old.#{Time.now.to_i}"
-              )
-            end
-
-            # Create the hooks symlink
-            FileUtils.ln_sf(global_hooks_path, local_hooks_path)
-          end
-
-          true
         end
       end
 
diff --git a/ruby/spec/lib/gitlab/git/repository_spec.rb b/ruby/spec/lib/gitlab/git/repository_spec.rb
index 962be8fc6..57b88ef59 100644
--- a/ruby/spec/lib/gitlab/git/repository_spec.rb
+++ b/ruby/spec/lib/gitlab/git/repository_spec.rb
@@ -37,53 +37,6 @@ describe Gitlab::Git::Repository do # rubocop:disable Metrics/BlockLength
     end
   end
 
-  describe '.create_hooks' do
-    let(:repo_path) { File.join(storage_path, 'hook-test.git') }
-    let(:hooks_dir) { File.join(repo_path, 'hooks') }
-    let(:target_hooks_dir) { Gitlab::Git::Hook.legacy_hooks_directory }
-    let(:existing_target) { File.join(repo_path, 'foobar') }
-
-    before do
-      GitlabShellHelper.setup_gitlab_shell
-
-      FileUtils.rm_rf(repo_path)
-      FileUtils.mkdir_p(repo_path)
-    end
-
-    context 'hooks is a directory' do
-      let(:existing_file) { File.join(hooks_dir, 'my-file') }
-
-      before do
-        FileUtils.mkdir_p(hooks_dir)
-        FileUtils.touch(existing_file)
-        described_class.create_hooks(repo_path, target_hooks_dir)
-      end
-
-      it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) }
-      it { expect(Dir[File.join(repo_path, "hooks.old.*/my-file")].count).to eq(1) }
-    end
-
-    context 'hooks is a valid symlink' do
-      before do
-        FileUtils.mkdir_p existing_target
-        File.symlink(existing_target, hooks_dir)
-        described_class.create_hooks(repo_path, target_hooks_dir)
-      end
-
-      it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) }
-    end
-
-    context 'hooks is a broken symlink' do
-      before do
-        FileUtils.rm_f(existing_target)
-        File.symlink(existing_target, hooks_dir)
-        described_class.create_hooks(repo_path, target_hooks_dir)
-      end
-
-      it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) }
-    end
-  end
-
   describe "Respond to" do
     subject { repository }