Merge branch 'more_descriptive_git_shell_messages' into 'master'

Handle custom Git hook result in GitLab UI

## What does this MR do?

This MR fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19190
It allows to show messages from git custom hooks

## Screenshots (if relevant)

![joxi_screenshot_1467651998090](/uploads/7f814523c96712c07b4f4b52c843aab3/joxi_screenshot_1467651998090.png)

See merge request !5073

CHANGELOG

@@ -34,6 +34,7 @@ v 8.10.0 (unreleased)
   - Add basic system information like memory and disk usage to the admin panel
   - Don't garbage collect commits that have related DB records like comments
   - More descriptive message for git hooks and file locks
+  - Handle custom Git hook result in GitLab UI
 v 8.9.5 (unreleased)
   - Improve the request / withdraw access button. !4860

app/services/merge_requests/merge_service.rb

@@ -36,10 +36,13 @@ module MergeRequests
       commit_id = repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options)
       merge_request.update(merge_commit_sha: commit_id)
+    rescue GitHooksService::PreReceiveError => e
+      merge_request.update(merge_error: e.message)
+      false
     rescue StandardError => e
       merge_request.update(merge_error: "Something went wrong during merge")
       Rails.logger.error(e.message)
-      return false
+      false
     end
     def after_merge

lib/gitlab/git/hook.rb

@@ -41,7 +41,7 @@ module Gitlab
           chdir: repo_path
         }
-        Open3.popen3(vars, path, options) do |stdin, _, stderr, wait_thr|
+        Open3.popen3(vars, path, options) do |stdin, stdout, stderr, wait_thr|
           exit_status = true
           stdin.sync = true
@@ -60,7 +60,7 @@ module Gitlab
           unless wait_thr.value == 0
             exit_status = false
-            exit_message = stderr.gets
+            exit_message = retrieve_error_message(stderr, stdout)
           end
         end
@@ -76,6 +76,11 @@ module Gitlab
         [status, nil]
       end
+
+      def retrieve_error_message(stderr, stdout)
+        err_message = stderr.gets
+        err_message.blank? ? stdout.gets : err_message
+      end
     end
   end
 end

spec/services/merge_requests/merge_service_spec.rb

@@ -65,6 +65,16 @@ describe MergeRequests::MergeService, services: true do
         expect(merge_request.merge_error).to eq("Something went wrong during merge")
       end
+
+      it 'saves error if there is an PreReceiveError exception' do
+        allow(service).to receive(:repository).and_raise(GitHooksService::PreReceiveError, "error")
+
+        allow(service).to receive(:execute_hooks)
+
+        service.execute(merge_request)
+
+        expect(merge_request.merge_error).to eq("error")
+      end
     end
   end
 end