Merge branch 'ee-gitaly-user-squash' into 'master'

[EE] Incorporate OperationService.UserSquash Gitaly RPC

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

GITALY_SERVER_VERSION

-0.77.0
+0.78.0

lib/gitlab/git/repository.rb

@@ -1222,33 +1222,13 @@ module Gitlab
       end
 
       def squash(user, squash_id, branch:, start_sha:, end_sha:, author:, message:)
-        squash_path = worktree_path(SQUASH_WORKTREE_PREFIX, squash_id)
-        env = git_env_for_user(user).merge(
-          'GIT_AUTHOR_NAME' => author.name,
-          'GIT_AUTHOR_EMAIL' => author.email
-        )
-        diff_range = "#{start_sha}...#{end_sha}"
-        diff_files = run_git!(
-          %W(diff --name-only --diff-filter=a --binary #{diff_range})
-        ).chomp
-
-        with_worktree(squash_path, branch, sparse_checkout_files: diff_files, env: env) do
-          # Apply diff of the `diff_range` to the worktree
-          diff = run_git!(%W(diff --binary #{diff_range}))
-          run_git!(%w(apply --index), chdir: squash_path, env: env) do |stdin|
-            stdin.write(diff)
+        gitaly_migrate(:squash) do |is_enabled|
+          if is_enabled
+            gitaly_operation_client.user_squash(user, squash_id, branch,
+              start_sha, end_sha, author, message)
+          else
+            git_squash(user, squash_id, branch, start_sha, end_sha, author, message)
           end
-
-          # Commit the `diff_range` diff
-          run_git!(%W(commit --no-verify --message #{message}), chdir: squash_path, env: env)
-
-          # Return the squash sha. May print a warning for ambiguous refs, but
-          # we can ignore that with `--quiet` and just take the SHA, if present.
-          # HEAD here always refers to the current HEAD commit, even if there is
-          # another ref called HEAD.
-          run_git!(
-            %w(rev-parse --quiet --verify HEAD), chdir: squash_path, env: env
-          ).chomp
         end
       end
 
@@ -2164,6 +2144,37 @@ module Gitlab
         end
       end
 
+      def git_squash(user, squash_id, branch, start_sha, end_sha, author, message)
+        squash_path = worktree_path(SQUASH_WORKTREE_PREFIX, squash_id)
+        env = git_env_for_user(user).merge(
+          'GIT_AUTHOR_NAME' => author.name,
+          'GIT_AUTHOR_EMAIL' => author.email
+        )
+        diff_range = "#{start_sha}...#{end_sha}"
+        diff_files = run_git!(
+          %W(diff --name-only --diff-filter=a --binary #{diff_range})
+        ).chomp
+
+        with_worktree(squash_path, branch, sparse_checkout_files: diff_files, env: env) do
+          # Apply diff of the `diff_range` to the worktree
+          diff = run_git!(%W(diff --binary #{diff_range}))
+          run_git!(%w(apply --index), chdir: squash_path, env: env) do |stdin|
+            stdin.write(diff)
+          end
+
+          # Commit the `diff_range` diff
+          run_git!(%W(commit --no-verify --message #{message}), chdir: squash_path, env: env)
+
+          # Return the squash sha. May print a warning for ambiguous refs, but
+          # we can ignore that with `--quiet` and just take the SHA, if present.
+          # HEAD here always refers to the current HEAD commit, even if there is
+          # another ref called HEAD.
+          run_git!(
+            %w(rev-parse --quiet --verify HEAD), chdir: squash_path, env: env
+          ).chomp
+        end
+      end
+
       def local_fetch_ref(source_path, source_ref:, target_ref:)
         args = %W(fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref})
         run_git(args)

lib/gitlab/gitaly_client/operation_service.rb

@@ -183,6 +183,32 @@ module Gitlab
         end
       end
 
+      def user_squash(user, squash_id, branch, start_sha, end_sha, author, message)
+        request = Gitaly::UserSquashRequest.new(
+          repository: @gitaly_repo,
+          user: Gitlab::Git::User.from_gitlab(user).to_gitaly,
+          squash_id: squash_id.to_s,
+          branch: encode_binary(branch),
+          start_sha: start_sha,
+          end_sha: end_sha,
+          author: Gitlab::Git::User.from_gitlab(author).to_gitaly,
+          commit_message: encode_binary(message)
+        )
+
+        response = GitalyClient.call(
+          @repository.storage,
+          :operation_service,
+          :user_squash,
+          request
+        )
+
+        if response.git_error.presence
+          raise Gitlab::Git::Repository::GitError, response.git_error
+        end
+
+        response.squash_sha
+      end
+
       def user_commit_files(
         user, branch_name, commit_message, actions, author_email, author_name,
         start_branch_name, start_repository)

spec/lib/gitlab/git/repository_spec.rb

@@ -2183,7 +2183,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
         repository.squash(user, squash_id, opts)
       end
 
-      context 'sparse checkout' do
+      context 'sparse checkout', :skip_gitaly_mock do
         let(:expected_files) { %w(files files/js files/js/application.js) }
 
         before do

spec/lib/gitlab/gitaly_client/operation_service_spec.rb

@@ -123,4 +123,53 @@ describe Gitlab::GitalyClient::OperationService do
       expect(subject.branch_created).to be(false)
     end
   end
+
+  describe '#user_squash' do
+    let(:branch_name) { 'my-branch' }
+    let(:squash_id) { '1' }
+    let(:start_sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' }
+    let(:end_sha) { '54cec5282aa9f21856362fe321c800c236a61615' }
+    let(:commit_message) { 'Squash message' }
+    let(:request) do
+      Gitaly::UserSquashRequest.new(
+        repository: repository.gitaly_repository,
+        user: gitaly_user,
+        squash_id: squash_id.to_s,
+        branch: branch_name,
+        start_sha: start_sha,
+        end_sha: end_sha,
+        author: gitaly_user,
+        commit_message: commit_message
+      )
+    end
+    let(:squash_sha) { 'f00' }
+    let(:response) { Gitaly::UserSquashResponse.new(squash_sha: squash_sha) }
+
+    subject do
+      client.user_squash(user, squash_id, branch_name, start_sha, end_sha, user, commit_message)
+    end
+
+    it 'sends a user_squash message and returns the squash sha' do
+      expect_any_instance_of(Gitaly::OperationService::Stub)
+        .to receive(:user_squash).with(request, kind_of(Hash))
+        .and_return(response)
+
+      expect(subject).to eq(squash_sha)
+    end
+
+    context "when git_error is present" do
+      let(:response) do
+        Gitaly::UserSquashResponse.new(git_error: "something failed")
+      end
+
+      it "throws a PreReceive exception" do
+        expect_any_instance_of(Gitaly::OperationService::Stub)
+          .to receive(:user_squash).with(request, kind_of(Hash))
+          .and_return(response)
+
+        expect { subject }.to raise_error(
+          Gitlab::Git::Repository::GitError, "something failed")
+      end
+    end
+  end
 end

spec/services/merge_requests/squash_service_spec.rb

@@ -110,30 +110,14 @@ describe MergeRequests::SquashService do
       it { is_expected.to match(status: :error, message: a_string_including('License')) }
     end
 
-    stages = {
-      'add worktree for squash' => 'worktree',
-      'configure sparse checkout' => 'config',
-      'get files in diff' => 'diff --name-only',
-      'check out target branch' => 'checkout',
-      'apply patch' => 'diff --binary',
-      'commit squashed changes' => 'commit',
-      'get SHA of squashed commit' => 'rev-parse'
-    }
-
-    stages.each do |stage, command|
-      context "when the #{stage} stage fails" do
-        let(:merge_request) { merge_request_with_only_new_files }
-        let(:error) { 'A test error' }
+    context 'git errors' do
+      let(:merge_request) { merge_request_with_only_new_files }
+      let(:error) { 'A test error' }
 
+      context 'with gitaly enabled' do
         before do
-          git_command = a_collection_containing_exactly(
-            a_string_starting_with("#{Gitlab.config.git.bin_path} #{command}")
-          ).or(
-            a_collection_starting_with([Gitlab.config.git.bin_path] + command.split)
-          )
-
-          allow(repository).to receive(:popen).and_return(['', 0])
-          allow(repository).to receive(:popen).with(git_command, anything, anything).and_return([error, 1])
+          allow(repository.gitaly_operation_client).to receive(:user_squash)
+            .and_raise(Gitlab::Git::Repository::GitError, error)
         end
 
         it 'logs the stage and output' do
@@ -147,11 +131,50 @@ describe MergeRequests::SquashService do
           expect(service.execute(merge_request)).to match(status: :error,
                                                           message: a_string_including('squash'))
         end
+      end
 
-        it 'cleans up the temporary directory' do
-          expect(File.exist?(squash_dir_path)).to be(false)
-
-          service.execute(merge_request)
+      context 'with Gitaly disabled', :skip_gitaly_mock do
+        stages = {
+          'add worktree for squash' => 'worktree',
+          'configure sparse checkout' => 'config',
+          'get files in diff' => 'diff --name-only',
+          'check out target branch' => 'checkout',
+          'apply patch' => 'diff --binary',
+          'commit squashed changes' => 'commit',
+          'get SHA of squashed commit' => 'rev-parse'
+        }
+
+        stages.each do |stage, command|
+          context "when the #{stage} stage fails" do
+            before do
+              git_command = a_collection_containing_exactly(
+                a_string_starting_with("#{Gitlab.config.git.bin_path} #{command}")
+              ).or(
+                a_collection_starting_with([Gitlab.config.git.bin_path] + command.split)
+              )
+
+              allow(repository).to receive(:popen).and_return(['', 0])
+              allow(repository).to receive(:popen).with(git_command, anything, anything).and_return([error, 1])
+            end
+
+            it 'logs the stage and output' do
+              expect(service).to receive(:log_error).with(log_error)
+              expect(service).to receive(:log_error).with(error)
+
+              service.execute(merge_request)
+            end
+
+            it 'returns an error' do
+              expect(service.execute(merge_request)).to match(status: :error,
+                                                              message: a_string_including('squash'))
+            end
+
+            it 'cleans up the temporary directory' do
+              expect(File.exist?(squash_dir_path)).to be(false)
+
+              service.execute(merge_request)
+            end
+          end
         end
       end
     end