Merge branch 'ce-to-ee-2018-01-10' into 'master'

CE upstream - Wednesday

Closes gitaly#873 and gitaly#903

See merge request gitlab-org/gitlab-ee!3994

GITALY_SERVER_VERSION

-0.66.0
+0.67.0

Gemfile

@@ -422,7 +422,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC client
-gem 'gitaly-proto', '~> 0.64.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.69.0', require: 'gitaly'
 
 gem 'toml-rb', '~> 0.3.15', require: false
 

Gemfile.lock

@@ -308,7 +308,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     gherkin-ruby (0.3.2)
-    gitaly-proto (0.64.0)
+    gitaly-proto (0.69.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.0)
     github-linguist (4.7.6)
@@ -1088,7 +1088,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.2.0)
-  gitaly-proto (~> 0.64.0)
+  gitaly-proto (~> 0.69.0)
   github-linguist (~> 4.7.0)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-license (~> 1.0)

app/models/repository.rb

@@ -110,6 +110,10 @@ class Repository
     "#<#{self.class.name}:#{@disk_path}>"
   end
 
+  def create_hooks
+    Gitlab::Git::Repository.create_hooks(path_to_repo, Gitlab.config.gitlab_shell.hooks_path)
+  end
+
   def commit(ref = 'HEAD')
     return nil unless exists?
     return ref if ref.is_a?(::Commit)

changelogs/unreleased/sh-fix-bare-import-hooks.yml

+---
+title: Fix hooks not being set up properly for bare import Rake task
+merge_request:
+author:
+type: fixed

doc/ci/examples/code_climate.md

@@ -16,7 +16,8 @@ codequality:
     - docker:dind
   script:
     - docker pull codeclimate/codeclimate
-    - docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate analyze -f json > codeclimate.json || true
+    - docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate:0.69.0 init
+    - docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate:0.69.0 analyze -f json > codeclimate.json || true
   artifacts:
     paths: [codeclimate.json]
 ```

lib/gitlab/bare_repository_import/importer.rb

@@ -63,6 +63,7 @@ module Gitlab
           log " * Created #{project.name} (#{project_full_path})".color(:green)
 
           project.write_repository_config
+          project.repository.create_hooks
 
           ProjectCacheWorker.perform_async(project.id)
         else

lib/gitlab/git/gitlab_projects.rb

@@ -41,36 +41,6 @@ module Gitlab
         io.read
       end
 
-      def rm_project
-        logger.info "Removing repository <#{repository_absolute_path}>."
-        FileUtils.rm_rf(repository_absolute_path)
-      end
-
-      # Move repository from one directory to another
-      #
-      # Example:  gitlab/gitlab-ci.git -> randx/six.git
-      #
-      # Won't work if target namespace directory does not exist
-      #
-      def mv_project(new_path)
-        new_absolute_path = File.join(shard_path, new_path)
-
-        # verify that the source repo exists
-        unless File.exist?(repository_absolute_path)
-          logger.error "mv-project failed: source path <#{repository_absolute_path}> does not exist."
-          return false
-        end
-
-        # ...and that the target repo does not exist
-        if File.exist?(new_absolute_path)
-          logger.error "mv-project failed: destination path <#{new_absolute_path}> already exists."
-          return false
-        end
-
-        logger.info "Moving repository from <#{repository_absolute_path}> to <#{new_absolute_path}>."
-        FileUtils.mv(repository_absolute_path, new_absolute_path)
-      end
-
       # Import project via git clone --bare
       # URL must be publicly cloneable
       def import_project(source, timeout)

lib/gitlab/gitaly_client/remote_service.rb

@@ -7,10 +7,12 @@ module Gitlab
         @storage = repository.storage
       end
 
-      def add_remote(name, url, mirror_refmap)
+      def add_remote(name, url, mirror_refmaps)
         request = Gitaly::AddRemoteRequest.new(
-          repository: @gitaly_repo, name: name, url: url,
-          mirror_refmap: mirror_refmap.to_s
+          repository: @gitaly_repo,
+          name: name,
+          url: url,
+          mirror_refmaps: Array.wrap(mirror_refmaps).map(&:to_s)
         )
 
         GitalyClient.call(@storage, :remote_service, :add_remote, request)

lib/gitlab/shell.rb

@@ -136,7 +136,10 @@ module Gitlab
       end
     end
 
-    # Move repository
+    # Move repository reroutes to mv_directory which is an alias for
+    # mv_namespace. Given the underlying implementation is a move action,
+    # indescriminate of what the folders might be.
+    #
     # storage - project's storage path
     # path - project disk path
     # new_path - new project disk path
@@ -146,7 +149,9 @@ module Gitlab
     #
     # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/873
     def mv_repository(storage, path, new_path)
-      gitlab_projects(storage, "#{path}.git").mv_project("#{new_path}.git")
+      return false if path.empty? || new_path.empty?
+
+      !!mv_directory(storage, "#{path}.git", "#{new_path}.git")
     end
 
     # Fork repository to new path
@@ -164,7 +169,9 @@ module Gitlab
         .fork_repository(forked_to_storage, "#{forked_to_disk_path}.git")
     end
 
-    # Remove repository from file system
+    # Removes a repository from file system, using rm_diretory which is an alias
+    # for rm_namespace. Given the underlying implementation removes the name
+    # passed as second argument on the passed storage.
     #
     # storage - project's storage path
     # name - project disk path
@@ -174,7 +181,12 @@ module Gitlab
     #
     # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/873
     def remove_repository(storage, name)
-      gitlab_projects(storage, "#{name}.git").rm_project
+      return false if name.empty?
+
+      !!rm_directory(storage, "#{name}.git")
+    rescue ArgumentError => e
+      Rails.logger.warn("Repository does not exist: #{e} at: #{name}.git")
+      false
     end
 
     # Add new key to gitlab-shell
@@ -313,6 +325,7 @@ module Gitlab
     rescue GRPC::InvalidArgument => e
       raise ArgumentError, e.message
     end
+    alias_method :rm_directory, :rm_namespace
 
     # Move namespace directory inside repositories storage
     #
@@ -332,6 +345,7 @@ module Gitlab
     rescue GRPC::InvalidArgument
       false
     end
+    alias_method :mv_directory, :mv_namespace
 
     def url_to_repo(path)
       Gitlab.config.gitlab_shell.ssh_path_prefix + "#{path}.git"

spec/lib/gitlab/bare_repository_import/importer_spec.rb

@@ -74,14 +74,18 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do
         importer.create_project_if_needed
       end
 
-      it 'creates the Git repo in disk' do
+      it 'creates the Git repo on disk with the proper symlink for hooks' do
         create_bare_repository("#{project_path}.git")
 
         importer.create_project_if_needed
 
         project = Project.find_by_full_path(project_path)
+        repo_path = File.join(project.repository_storage_path, project.disk_path + '.git')
+        hook_path = File.join(repo_path, 'hooks')
 
-        expect(File).to exist(File.join(project.repository_storage_path, project.disk_path + '.git'))
+        expect(File).to exist(repo_path)
+        expect(File.symlink?(hook_path)).to be true
+        expect(File.readlink(hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path)
       end
 
       context 'hashed storage enabled' do

spec/lib/gitlab/git/gitlab_projects_spec.rb

@@ -25,51 +25,6 @@ describe Gitlab::Git::GitlabProjects do
     it { expect(gl_projects.logger).to eq(logger) }
   end
 
-  describe '#mv_project' do
-    let(:new_repo_path) { File.join(tmp_repos_path, 'repo.git') }
-
-    it 'moves a repo directory' do
-      expect(File.exist?(tmp_repo_path)).to be_truthy
-
-      message = "Moving repository from <#{tmp_repo_path}> to <#{new_repo_path}>."
-      expect(logger).to receive(:info).with(message)
-
-      expect(gl_projects.mv_project('repo.git')).to be_truthy
-
-      expect(File.exist?(tmp_repo_path)).to be_falsy
-      expect(File.exist?(new_repo_path)).to be_truthy
-    end
-
-    it "fails if the source path doesn't exist" do
-      expected_source_path = File.join(tmp_repos_path, 'bad-src.git')
-      expect(logger).to receive(:error).with("mv-project failed: source path <#{expected_source_path}> does not exist.")
-
-      result = build_gitlab_projects(tmp_repos_path, 'bad-src.git').mv_project('repo.git')
-      expect(result).to be_falsy
-    end
-
-    it 'fails if the destination path already exists' do
-      FileUtils.mkdir_p(File.join(tmp_repos_path, 'already-exists.git'))
-
-      expected_distination_path = File.join(tmp_repos_path, 'already-exists.git')
-      message = "mv-project failed: destination path <#{expected_distination_path}> already exists."
-      expect(logger).to receive(:error).with(message)
-
-      expect(gl_projects.mv_project('already-exists.git')).to be_falsy
-    end
-  end
-
-  describe '#rm_project' do
-    it 'removes a repo directory' do
-      expect(File.exist?(tmp_repo_path)).to be_truthy
-      expect(logger).to receive(:info).with("Removing repository <#{tmp_repo_path}>.")
-
-      expect(gl_projects.rm_project).to be_truthy
-
-      expect(File.exist?(tmp_repo_path)).to be_falsy
-    end
-  end
-
   describe '#push_branches' do
     let(:remote_name) { 'remote-name' }
     let(:branch_name) { 'master' }

spec/lib/gitlab/shell_spec.rb

@@ -443,32 +443,44 @@ describe Gitlab::Shell do
     end
 
     describe '#remove_repository' do
-      subject { gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path) }
+      let!(:project) { create(:project, :repository) }
+      let(:disk_path) { "#{project.disk_path}.git" }
 
       it 'returns true when the command succeeds' do
-        expect(gitlab_projects).to receive(:rm_project) { true }
+        expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(true)
 
-        is_expected.to be_truthy
+        expect(gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path)).to be(true)
+
+        expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(false)
       end
 
-      it 'returns false when the command fails' do
-        expect(gitlab_projects).to receive(:rm_project) { false }
+      it 'keeps the namespace directory' do
+        gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path)
 
-        is_expected.to be_falsy
+        expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(false)
+        expect(gitlab_shell.exists?(project.repository_storage_path, project.disk_path.gsub(project.name, ''))).to be(true)
       end
     end
 
     describe '#mv_repository' do
+      let!(:project2) { create(:project, :repository) }
+
       it 'returns true when the command succeeds' do
-        expect(gitlab_projects).to receive(:mv_project).with('project/newpath.git') { true }
+        old_path = project2.disk_path
+        new_path = "project/new_path"
+
+        expect(gitlab_shell.exists?(project2.repository_storage_path, "#{old_path}.git")).to be(true)
+        expect(gitlab_shell.exists?(project2.repository_storage_path, "#{new_path}.git")).to be(false)
 
-        expect(gitlab_shell.mv_repository(project.repository_storage_path, project.disk_path, 'project/newpath')).to be_truthy
+        expect(gitlab_shell.mv_repository(project2.repository_storage_path, old_path, new_path)).to be_truthy
+
+        expect(gitlab_shell.exists?(project2.repository_storage_path, "#{old_path}.git")).to be(false)
+        expect(gitlab_shell.exists?(project2.repository_storage_path, "#{new_path}.git")).to be(true)
       end
 
       it 'returns false when the command fails' do
-        expect(gitlab_projects).to receive(:mv_project).with('project/newpath.git') { false }
-
-        expect(gitlab_shell.mv_repository(project.repository_storage_path, project.disk_path, 'project/newpath')).to be_falsy
+        expect(gitlab_shell.mv_repository(project2.repository_storage_path, project2.disk_path, '')).to be_falsy
+        expect(gitlab_shell.exists?(project2.repository_storage_path, "#{project2.disk_path}.git")).to be(true)
       end
     end
 

spec/models/repository_spec.rb

@@ -412,6 +412,28 @@ describe Repository do
     end
   end
 
+  describe '#create_hooks' do
+    let(:hook_path) { File.join(repository.path_to_repo, 'hooks') }
+
+    it 'symlinks the global hooks directory' do
+      repository.create_hooks
+
+      expect(File.symlink?(hook_path)).to be true
+      expect(File.readlink(hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path)
+    end
+
+    it 'replaces existing symlink with the right directory' do
+      FileUtils.mkdir_p(hook_path)
+
+      expect(File.symlink?(hook_path)).to be false
+
+      repository.create_hooks
+
+      expect(File.symlink?(hook_path)).to be true
+      expect(File.readlink(hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path)
+    end
+  end
+
   describe "#create_dir" do
     it "commits a change that creates a new directory" do
       expect do