Move git operations for rebase into Gitlab::Git

ee/app/models/ee/merge_request.rb

@@ -16,10 +16,6 @@ module EE
       delegate :sha, to: :base_pipeline, prefix: :base_pipeline, allow_nil: true
     end
 
-    def rebase_dir_path
-      File.join(::Gitlab.config.shared.path, 'tmp/rebase', source_project.id.to_s, id.to_s).to_s
-    end
-
     def squash_dir_path
       File.join(::Gitlab.config.shared.path, 'tmp/squash', source_project.id.to_s, id.to_s).to_s
     end
@@ -28,7 +24,7 @@ module EE
       # The source project can be deleted
       return false unless source_project
 
-      File.exist?(rebase_dir_path) && !clean_stuck_rebase
+      source_project.repository.rebase_in_progress?(id)
     end
 
     def squash_in_progress?
@@ -38,13 +34,6 @@ module EE
       File.exist?(squash_dir_path) && !clean_stuck_squash
     end
 
-    def clean_stuck_rebase
-      if File.mtime(rebase_dir_path) < 15.minutes.ago
-        FileUtils.rm_rf(rebase_dir_path)
-        true
-      end
-    end
-
     def clean_stuck_squash
       if File.mtime(squash_dir_path) < 15.minutes.ago
         FileUtils.rm_rf(squash_dir_path)

ee/app/services/merge_requests/rebase_service.rb

@@ -16,48 +16,19 @@ module MergeRequests
         return false
       end
 
-      run_git_command(
-        %W(worktree add --detach #{tree_path} #{merge_request.source_branch}),
-        repository.path_to_repo,
-        git_env,
-        'add worktree for rebase'
-      )
-
-      run_git_command(
-        %W(pull --rebase #{target_project.repository.path_to_repo} #{merge_request.target_branch}),
-        tree_path,
-        git_env.merge('GIT_COMMITTER_NAME' => current_user.name,
-                      'GIT_COMMITTER_EMAIL' => current_user.email),
-        'rebase branch'
-      )
-
-      rebase_sha = run_git_command(
-        %w(rev-parse HEAD),
-        tree_path,
-        git_env,
-        'get SHA of rebased branch'
-      )
-
-      Gitlab::Git::OperationService.new(current_user, project.repository.raw_repository)
-        .update_branch(merge_request.source_branch, rebase_sha, merge_request.source_branch_sha)
+      rebase_sha = repository.rebase(current_user, merge_request.id,
+        source_branch: merge_request.source_branch,
+        source_branch_sha: merge_request.source_branch_sha,
+        target_repository: target_project.repository.raw,
+        target_branch: merge_request.target_branch)
 
       merge_request.update_attributes(rebase_commit_sha: rebase_sha)
 
       true
-    rescue GitCommandError
-      false
     rescue => e
       log_error('Failed to rebase branch:')
       log_error(e.message, save_message_on_model: true)
       false
-    ensure
-      clean_dir
-    end
-
-    private
-
-    def tree_path
-      @tree_path ||= merge_request.rebase_dir_path
     end
   end
 end

lib/gitlab/git/repository.rb

@@ -7,6 +7,7 @@ module Gitlab
   module Git
     class Repository
       include Gitlab::Git::RepositoryMirroring
+      include Gitlab::Git::RepositoryWorktree
       include Gitlab::Git::Popen
 
       ALLOWED_OBJECT_DIRECTORIES_VARIABLES = %w[
@@ -1212,6 +1213,29 @@ module Gitlab
         raise GitError.new("Could not fsck repository:\n#{output}") unless status.zero?
       end
 
+      def rebase(user, rebase_id, source_branch:, source_branch_sha:, target_repository:, target_branch:)
+        rebase_path = rebase_dir_path(rebase_id)
+        env = git_env_for_user(user)
+
+        with_worktree(rebase_path, source_branch, env: env) do
+          run_git!(
+            %W(pull --rebase #{target_repository.path} #{target_branch}),
+            chdir: rebase_path, env: env
+          )
+
+          rebase_sha = run_git!(%w(rev-parse HEAD), chdir: rebase_path, env: env).strip
+
+          Gitlab::Git::OperationService.new(user, self)
+            .update_branch(source_branch, rebase_sha, source_branch_sha)
+
+          rebase_sha
+        end
+      end
+
+      def rebase_in_progress?(rebase_id)
+        fresh_worktree?(rebase_dir_path(rebase_id))
+      end
+
       def gitaly_repository
         Gitlab::GitalyClient::Util.repository(@storage, @relative_path, @gl_repository)
       end
@@ -1259,6 +1283,20 @@ module Gitlab
         end
       end
 
+      def rebase_dir_path(id)
+        File.join(::Gitlab.config.shared.path, 'tmp/rebase', gl_repository, id.to_s).to_s
+      end
+
+      def git_env_for_user(user)
+        {
+          'GIT_COMMITTER_NAME' => user.name,
+          'GIT_COMMITTER_EMAIL' => user.email,
+          'GL_ID' => Gitlab::GlId.gl_id(user),
+          'GL_PROTOCOL' => Gitlab::Git::Hook::GL_PROTOCOL,
+          'GL_REPOSITORY' => gl_repository
+        }
+      end
+
       # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'.
       def branches_filter(filter: nil, sort_by: nil)
         # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37464

lib/gitlab/git/repository_worktree.rb

+module Gitlab
+  module Git
+    module RepositoryWorktree
+      def fresh_worktree?(path)
+        File.exist?(path) && !clean_stuck_worktree(path)
+      end
+
+      def with_worktree(path, target, env:)
+        run_git!(%W(worktree add --detach #{path} #{target}), env: env)
+
+        yield
+      ensure
+        FileUtils.rm_rf(path) if File.exist?(path)
+      end
+
+      def clean_stuck_worktree(path)
+        return false unless File.mtime(path) < 15.minutes.ago
+
+        FileUtils.rm_rf(path)
+        true
+      end
+    end
+  end
+end

spec/ee/spec/models/ee/merge_request_spec.rb

@@ -51,7 +51,7 @@ describe MergeRequest do
     end
 
     it 'returns false when there is no rebase directory' do
-      allow(File).to receive(:exist?).with(subject.rebase_dir_path).and_return(false)
+      allow(File).to receive(:exist?).and_return(false)
 
       expect(subject.rebase_in_progress?).to be_falsey
     end

spec/services/merge_requests/rebase_service_spec.rb

@@ -10,6 +10,7 @@ describe MergeRequests::RebaseService do
            target_branch: 'master')
   end
   let(:project) { merge_request.project }
+  let(:repository) { project.repository.raw }
 
   subject(:service) { described_class.new(project, user, {}) }
 
@@ -37,7 +38,7 @@ describe MergeRequests::RebaseService do
 
     context 'when unexpected error occurs' do
       before do
-        allow(service).to receive(:run_git_command).and_raise('Something went wrong')
+        allow(repository).to receive(:run_git!).and_raise('Something went wrong')
       end
 
       it 'saves the error message' do
@@ -54,7 +55,7 @@ describe MergeRequests::RebaseService do
 
     context 'with git command failure' do
       before do
-        allow(service).to receive(:popen).and_return(['Something went wrong', 1])
+        allow(repository).to receive(:run_git!).and_raise(Gitlab::Git::Repository::GitError, 'Something went wrong')
       end
 
       it 'saves the error message' do
@@ -96,12 +97,9 @@ describe MergeRequests::RebaseService do
 
       context 'git commands' do
         it 'sets GL_REPOSITORY env variable when calling git commands' do
-          expect_any_instance_of(described_class)
-            .to receive(:run_git_command).exactly(3).with(
-              anything,
-              anything,
-              hash_including('GL_REPOSITORY'),
-              anything)
+          expect(repository).to receive(:popen).exactly(3)
+            .with(anything, anything, hash_including('GL_REPOSITORY'))
+            .and_return(['', 0])
 
           service.execute(merge_request)
         end