Refactor gitlab:import:repos task to remove direct disk access

lib/gitlab/bare_repository_import/importer.rb

@@ -26,6 +26,12 @@ module Gitlab
         end
       end
 
+      # This is called from within a rake task only used by Admins, so allow writing
+      # to STDOUT
+      def self.log(message)
+        puts message # rubocop:disable Rails/Output
+      end
+
       attr_reader :user, :project_name, :bare_repo
 
       delegate :log, to: :class
@@ -59,11 +65,10 @@ module Gitlab
                                               import_type: 'bare_repository',
                                               namespace_id: group&.id).execute
 
-        if project.persisted? && mv_repo(project)
+        if project.persisted? && mv_repositories(project)
           log " * Created #{project.name} (#{project_full_path})".color(:green)
 
           project.write_repository_config
-          Gitlab::Git::Repository.create_hooks(project.repository.path_to_repo, Gitlab.config.gitlab_shell.hooks_path)
 
           ProjectCacheWorker.perform_async(project.id)
         else
@@ -74,12 +79,11 @@ module Gitlab
         project
       end
 
-      def mv_repo(project)
-        storage_path = storage_path_for_shard(project.repository_storage)
-        FileUtils.mv(repo_path, project.repository.path_to_repo)
+      def mv_repositories(project)
+        mv_repo(bare_repo.repo_path, project.repository)
 
         if bare_repo.wiki_exists?
-          FileUtils.mv(wiki_path, File.join(storage_path, project.disk_path + '.wiki.git'))
+          mv_repo(bare_repo.wiki_path, project.wiki.repository)
         end
 
         true
@@ -89,6 +93,11 @@ module Gitlab
         false
       end
 
+      def mv_repo(path, repository)
+        repository.create_from_bundle(bundle(path))
+        FileUtils.rm_rf(path)
+      end
+
       def storage_path_for_shard(shard)
         Gitlab.config.repositories.storages[shard].legacy_disk_path
       end
@@ -101,10 +110,17 @@ module Gitlab
         Groups::NestedCreateService.new(user, group_path: group_path).execute
       end
 
-      # This is called from within a rake task only used by Admins, so allow writing
-      # to STDOUT
-      def self.log(message)
-        puts message # rubocop:disable Rails/Output
+      def bundle(repo_path)
+        # TODO: we could save some time and disk space by using
+        # `git bundle create - --all` and streaming the bundle directly to
+        # Gitaly, rather than writing it on disk first
+        bundle_path = "#{repo_path}.bundle"
+        cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} bundle create #{bundle_path} --all)
+        output, status = Gitlab::Popen.popen(cmd)
+
+        raise output unless status.zero?
+
+        bundle_path
       end
     end
   end

lib/gitlab/bare_repository_import/repository.rb

@@ -1,5 +1,3 @@
-# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/953
-#
 module Gitlab
   module BareRepositoryImport
     class Repository

lib/gitlab/git/repository.rb

@@ -39,19 +39,6 @@ module Gitlab
       ChecksumError = Class.new(StandardError)
 
       class << self
-        # Unlike `new`, `create` takes the repository path
-        def create(repo_path, bare: true, symlink_hooks_to: nil)
-          FileUtils.mkdir_p(repo_path, mode: 0770)
-
-          # Equivalent to `git --git-path=#{repo_path} init [--bare]`
-          repo = Rugged::Repository.init_at(repo_path, bare)
-          repo.close
-
-          create_hooks(repo_path, symlink_hooks_to) if symlink_hooks_to.present?
-
-          true
-        end
-
         def create_hooks(repo_path, global_hooks_path)
           local_hooks_path = File.join(repo_path, 'hooks')
           real_local_hooks_path = :not_found

spec/lib/gitlab/bare_repository_import/importer_spec.rb

@@ -1,10 +1,11 @@
 require 'spec_helper'
 
-describe Gitlab::BareRepositoryImport::Importer, repository: true do
+describe Gitlab::BareRepositoryImport::Importer, :seed_helper do
   let!(:admin) { create(:admin) }
   let!(:base_dir) { Dir.mktmpdir + '/' }
   let(:bare_repository) { Gitlab::BareRepositoryImport::Repository.new(base_dir, File.join(base_dir, "#{project_path}.git")) }
   let(:gitlab_shell) { Gitlab::Shell.new }
+  let(:source_project) { TEST_REPO_PATH }
 
   subject(:importer) { described_class.new(admin, bare_repository) }
 
@@ -17,16 +18,11 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
 
   after do
     FileUtils.rm_rf(base_dir)
+    TestEnv.clean_test_path
+    ensure_seeds
     Rainbow.enabled = @rainbow
   end
 
-  around do |example|
-    # TODO migrate BareRepositoryImport https://gitlab.com/gitlab-org/gitaly/issues/953
-    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-      example.run
-    end
-  end
-
   shared_examples 'importing a repository' do
     describe '.execute' do
       it 'creates a project for a repository in storage' do
@@ -86,8 +82,8 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
         importer.create_project_if_needed
       end
 
-      it 'creates the Git repo on disk with the proper symlink for hooks' do
-        create_bare_repository("#{project_path}.git")
+      it 'creates the Git repo on disk' do
+        prepare_repository("#{project_path}.git", source_project)
 
         importer.create_project_if_needed
 
@@ -97,9 +93,6 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
 
         expect(gitlab_shell.exists?(project.repository_storage, repo_path)).to be(true)
         expect(gitlab_shell.exists?(project.repository_storage, hook_path)).to be(true)
-
-        full_hook_path = File.join(project.repository.path_to_repo, 'hooks')
-        expect(File.readlink(full_hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path)
       end
 
       context 'hashed storage enabled' do
@@ -148,7 +141,7 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
     end
 
     it 'creates the Git repo in disk' do
-      create_bare_repository("#{project_path}.git")
+      prepare_repository("#{project_path}.git", source_project)
 
       importer.create_project_if_needed
 
@@ -158,23 +151,23 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
       expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.wiki.git')).to be(true)
     end
 
-    it 'moves an existing project to the correct path' do
+    context 'with a repository already on disk' do
+      let!(:base_dir) { TestEnv.repos_path }
       # This is a quick way to get a valid repository instead of copying an
       # existing one. Since it's not persisted, the importer will try to
       # create the project.
-      project = build(:project, :legacy_storage, :repository)
-      original_commit_count = project.repository.commit_count
+      let(:project) { build(:project, :legacy_storage, :repository) }
+      let(:project_path) { project.full_path }
 
-      legacy_path = Gitlab.config.repositories.storages[project.repository_storage].legacy_disk_path
+      it 'moves an existing project to the correct path' do
+        original_commit_count = project.repository.commit_count
 
-      bare_repo = Gitlab::BareRepositoryImport::Repository.new(legacy_path, project.repository.path)
-      gitlab_importer = described_class.new(admin, bare_repo)
+        expect(importer).to receive(:create_project).and_call_original
 
-      expect(gitlab_importer).to receive(:create_project).and_call_original
+        new_project = importer.create_project_if_needed
 
-      new_project = gitlab_importer.create_project_if_needed
-
-      expect(new_project.repository.commit_count).to eq(original_commit_count)
+        expect(new_project.repository.commit_count).to eq(original_commit_count)
+      end
     end
   end
 
@@ -185,8 +178,8 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
     it_behaves_like 'importing a repository'
 
     it 'creates the Wiki git repo in disk' do
-      create_bare_repository("#{project_path}.git")
-      create_bare_repository("#{project_path}.wiki.git")
+      prepare_repository("#{project_path}.git", source_project)
+      prepare_repository("#{project_path}.wiki.git", source_project)
 
       expect(Projects::CreateService).to receive(:new).with(admin, hash_including(skip_wiki: true,
                                                                                   import_type: 'bare_repository')).and_call_original
@@ -213,8 +206,13 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
     end
   end
 
-  def create_bare_repository(project_path)
+  def prepare_repository(project_path, source_project)
     repo_path = File.join(base_dir, project_path)
-    Gitlab::Git::Repository.create(repo_path, bare: true)
+
+    return create_bare_repository(repo_path) unless source_project
+
+    cmd = %W(#{Gitlab.config.git.bin_path} clone --bare #{source_project} #{repo_path})
+
+    system(git_env, *cmd, chdir: SEED_STORAGE_PATH, out: '/dev/null', err: '/dev/null')
   end
 end

spec/lib/gitlab/git/attributes_at_ref_parser_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Git::AttributesAtRefParser, seed_helper: true do
+describe Gitlab::Git::AttributesAtRefParser, :seed_helper do
   let(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
 

spec/lib/gitlab/git/attributes_parser_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Git::AttributesParser, seed_helper: true do
+describe Gitlab::Git::AttributesParser, :seed_helper do
   let(:attributes_path) { File.join(SEED_STORAGE_PATH, 'with-git-attributes.git', 'info', 'attributes') }
   let(:data) { File.read(attributes_path) }
 

spec/lib/gitlab/git/blame_spec.rb

@@ -1,7 +1,7 @@
 # coding: utf-8
 require "spec_helper"
 
-describe Gitlab::Git::Blame, seed_helper: true do
+describe Gitlab::Git::Blame, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
   let(:blame) do
     Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md")

spec/lib/gitlab/git/blob_snippet_spec.rb

@@ -2,7 +2,7 @@
 
 require "spec_helper"
 
-describe Gitlab::Git::BlobSnippet, seed_helper: true do
+describe Gitlab::Git::BlobSnippet, :seed_helper do
   describe '#data' do
     context 'empty lines' do
       let(:snippet) { Gitlab::Git::BlobSnippet.new('master', nil, nil, nil) }

spec/lib/gitlab/git/blob_spec.rb

@@ -2,7 +2,7 @@
 
 require "spec_helper"
 
-describe Gitlab::Git::Blob, seed_helper: true do
+describe Gitlab::Git::Blob, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
 
   describe 'initialize' do

spec/lib/gitlab/git/branch_spec.rb

@@ -1,6 +1,6 @@
 require "spec_helper"
 
-describe Gitlab::Git::Branch, seed_helper: true do
+describe Gitlab::Git::Branch, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
   let(:rugged) do
     Gitlab::GitalyClient::StorageSettings.allow_disk_access do

spec/lib/gitlab/git/commit_spec.rb

@@ -1,6 +1,6 @@
 require "spec_helper"
 
-describe Gitlab::Git::Commit, seed_helper: true do
+describe Gitlab::Git::Commit, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
   let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) }
   let(:rugged_commit) do

spec/lib/gitlab/git/committer_with_hooks_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Git::CommitterWithHooks, seed_helper: true do
+describe Gitlab::Git::CommitterWithHooks, :seed_helper do
   # TODO https://gitlab.com/gitlab-org/gitaly/issues/1234
   skip 'needs to be moved to gitaly-ruby test suite' do
     shared_examples 'calling wiki hooks' do

spec/lib/gitlab/git/compare_spec.rb

@@ -1,6 +1,6 @@
 require "spec_helper"
 
-describe Gitlab::Git::Compare, seed_helper: true do
+describe Gitlab::Git::Compare, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
   let(:compare) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: false) }
   let(:compare_straight) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: true) }

spec/lib/gitlab/git/diff_collection_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Git::DiffCollection, seed_helper: true do
+describe Gitlab::Git::DiffCollection, :seed_helper do
   subject do
     Gitlab::Git::DiffCollection.new(
       iterator,

spec/lib/gitlab/git/diff_spec.rb

@@ -1,6 +1,6 @@
 require "spec_helper"
 
-describe Gitlab::Git::Diff, seed_helper: true do
+describe Gitlab::Git::Diff, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
 
   before do

spec/lib/gitlab/git/hooks_service_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Git::HooksService, seed_helper: true do
+describe Gitlab::Git::HooksService, :seed_helper do
   let(:gl_id) { 'user-456' }
   let(:gl_username) { 'janedoe' }
   let(:user) { Gitlab::Git::User.new(gl_username, 'Jane Doe', 'janedoe@example.com', gl_id) }

spec/lib/gitlab/git/index_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Git::Index, seed_helper: true do
+describe Gitlab::Git::Index, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
   let(:index) { described_class.new(repository) }
 

spec/lib/gitlab/git/remote_repository_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::Git::RemoteRepository, seed_helper: true do
+describe Gitlab::Git::RemoteRepository, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
   subject { described_class.new(repository) }
 

spec/lib/gitlab/git/repository_spec.rb

@@ -1,7 +1,7 @@
 # coding: utf-8
 require "spec_helper"
 
-describe Gitlab::Git::Repository, seed_helper: true do
+describe Gitlab::Git::Repository, :seed_helper do
   include Gitlab::EncodingHelper
   using RSpec::Parameterized::TableSyntax
 

spec/lib/gitlab/git/tag_spec.rb

@@ -1,6 +1,6 @@
 require "spec_helper"
 
-describe Gitlab::Git::Tag, seed_helper: true do
+describe Gitlab::Git::Tag, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
 
   shared_examples 'Gitlab::Git::Repository#tags' do

spec/lib/gitlab/git/tree_spec.rb

@@ -1,6 +1,6 @@
 require "spec_helper"
 
-describe Gitlab::Git::Tree, seed_helper: true do
+describe Gitlab::Git::Tree, :seed_helper do
   let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
 
   context :repo do

spec/support/helpers/seed_helper.rb

@@ -101,10 +101,4 @@ bla/bla.txt
       handle.write('# hello'.encode(enc))
     end
   end
-
-  # Prevent developer git configurations from being persisted to test
-  # repositories
-  def git_env
-    { 'GIT_TEMPLATE_DIR' => '' }
-  end
 end

spec/support/helpers/test_env.rb

@@ -243,6 +243,14 @@ module TestEnv
     set_repo_refs(target_repo_path, refs)
   end
 
+  def create_bare_repository(path)
+    FileUtils.mkdir_p(path)
+
+    system(git_env, *%W(#{Gitlab.config.git.bin_path} -C #{path} init --bare),
+           out: '/dev/null',
+           err: '/dev/null')
+  end
+
   def repos_path
     @repos_path ||= Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path
   end

spec/support/stored_repositories.rb

@@ -1,8 +1,4 @@
 RSpec.configure do |config|
-  config.before(:each, :repository) do
-    TestEnv.clean_test_path
-  end
-
   config.before(:all, :broken_storage) do
     FileUtils.rm_rf Gitlab.config.repositories.storages.broken.legacy_disk_path
   end