Fix squashing for binary files

Rugged's `#to_patch` method doesn't generate a patch suitable for binary
files. Instead, piping `git diff --binary` to `git apply` will do this
and avoid allocating strings in Ruby, as an added bonus.

app/services/merge_requests/squash_service.rb

@@ -29,9 +29,15 @@ module MergeRequests
         'add worktree for squash'
       )
 
-      run_git_command(%w(apply --cached), tree_path, git_env, 'apply patch') do |stdin|
-        stdin.puts(merge_request_to_patch)
-      end
+      diff = git_command(%W(diff --binary #{merge_request.diff_start_sha}...#{merge_request.diff_head_sha}))
+      apply = git_command(%w(apply --index))
+
+      run_command(
+        ["#{diff.join(' ')} | #{apply.join(' ')}"],
+        tree_path,
+        git_env,
+        'apply patch'
+      )
 
       run_git_command(
         %W(commit -C #{merge_request.diff_head_sha}),
@@ -61,9 +67,5 @@ module MergeRequests
     def tree_path
       @tree_path ||= merge_request.squash_dir_path
     end
-
-    def merge_request_to_patch
-      @merge_request_to_patch ||= rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha).patch
-    end
   end
 end

app/services/merge_requests/working_copy_base_service.rb

@@ -6,15 +6,22 @@ module MergeRequests
 
     attr_reader :merge_request
 
+    def git_command(command)
+      [Gitlab.config.git.bin_path] + command
+    end
+
     def run_git_command(command, path, env, message = nil, &block)
-      git_command = [Gitlab.config.git.bin_path] + command
-      output, status = popen(git_command, path, env, &block)
+      run_command(git_command(command), path, env, message, &block)
+    end
+
+    def run_command(command, path, env, message = nil, &block)
+      output, status = popen(command, path, env, &block)
 
       unless status.zero?
         if message
-          log_error("Failed to #{message} with `#{git_command.join(' ')}`:")
+          log_error("Failed to #{message} with `#{command.join(' ')}`:")
         else
-          log_error("`#{git_command.join(' ')}` failed:")
+          log_error("`#{command.join(' ')}` failed:")
         end
 
         log_error(output)

spec/services/merge_requests/squash_service_spec.rb

@@ -7,7 +7,7 @@ describe MergeRequests::SquashService do
 
   let(:merge_request) do
     create(:merge_request,
-           source_branch: 'fix', source_project: project,
+           source_branch: 'video', source_project: project,
            target_branch: 'master', target_project: project)
   end
 
@@ -17,13 +17,6 @@ describe MergeRequests::SquashService do
            target_branch: 'master', target_project: project)
   end
 
-  def git_command(command)
-    a_collection_starting_with([Gitlab.config.git.bin_path, command])
-  end
-
-  shared_examples 'the squashed commit' do
-  end
-
   describe '#execute' do
     context 'when there is only one commit in the merge request' do
       it 'returns that commit SHA' do
@@ -66,7 +59,9 @@ describe MergeRequests::SquashService do
 
           expect(squash_commit.author_name).to eq(diff_head_commit.author_name)
           expect(squash_commit.author_email).to eq(diff_head_commit.author_email)
-          expect(squash_commit.message).to eq(diff_head_commit.message)
+
+          # The commit message on the 'real' commit doesn't have a trailing newline
+          expect(squash_commit.message.chomp).to eq(diff_head_commit.message)
         end
 
         it 'sets the current user as the committer' do
@@ -87,7 +82,7 @@ describe MergeRequests::SquashService do
 
     stages = {
       'add worktree for squash' => 'worktree',
-      'apply patch' => 'apply',
+      'apply patch' => 'diff --binary',
       'commit squashed changes' => 'commit',
       'get SHA of squashed commit' => 'rev-parse'
     }
@@ -97,8 +92,14 @@ describe MergeRequests::SquashService do
         let(:error) { 'A test error' }
 
         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])
+          )
+
           allow(service).to receive(:popen).and_return(['', 0])
-          allow(service).to receive(:popen).with(git_command(command), anything, anything).and_return([error, 1])
+          allow(service).to receive(:popen).with(git_command, anything, anything).and_return([error, 1])
         end
 
         it 'logs the stage and output' do