Merge branch 'pre-receive-wo-satellites' into 'remove-satellites'

Add support of pre-receive hooks without satellites

Implement commit transaction with pre-receive and post-receive hooks for web editor and merge

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

See merge request !1154

9 files changed, 174 insertions, 129 deletions

diff --git a/Gemfile b/Gemfile
index 2483a7d24a3..77bb02a50aa 100644
--- a/Gemfile
+++ b/Gemfile
@@ -38,7 +38,7 @@ gem "browser", '~> 0.8.0'
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem "gitlab_git", '~> 7.2.12'
+gem "gitlab_git", '~> 7.2.13'
 
 # Ruby/Rack Git Smart-HTTP Server Handler
 # GitLab fork with a lot of changes (improved thread-safety, better memory usage etc)
diff --git a/Gemfile.lock b/Gemfile.lock
index 335f6777c9f..21410671839 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -271,7 +271,7 @@ GEM
       mime-types (~> 1.19)
     gitlab_emoji (0.1.0)
       gemojione (~> 2.0)
-    gitlab_git (7.2.12)
+    gitlab_git (7.2.13)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.6)
       gitlab-linguist (~> 3.0)
@@ -783,7 +783,7 @@ DEPENDENCIES
   gitlab-grack (~> 2.0.2)
   gitlab-linguist (~> 3.0.1)
   gitlab_emoji (~> 0.1)
-  gitlab_git (~> 7.2.12)
+  gitlab_git (~> 7.2.13)
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (= 1.2.1)
   gollum-lib (~> 4.0.2)
@@ -875,4 +875,4 @@ DEPENDENCIES
   wikicloth (= 0.8.1)
 
 BUNDLED WITH
-   1.10.4
+   1.10.6
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 46efbede2a2..ea298d88f27 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1,4 +1,9 @@
+require 'securerandom'
+
 class Repository
+  class PreReceiveError < StandardError; end
+  class CommitError < StandardError; end
+
   include Gitlab::ShellAdapter
 
   attr_accessor :raw_repository, :path_with_namespace, :project
@@ -364,43 +369,47 @@ class Repository
     @root_ref ||= raw_repository.root_ref
   end
 
-  def commit_file(user, path, content, message, ref)
-    path[0] = '' if path[0] == '/'
+  def commit_file(user, path, content, message, branch)
+    commit_with_hooks(user, branch) do |ref|
+      path[0] = '' if path[0] == '/'
 
-    committer = user_to_comitter(user)
-    options = {}
-    options[:committer] = committer
-    options[:author] = committer
-    options[:commit] = {
-      message: message,
-      branch: ref
-    }
+      committer = user_to_comitter(user)
+      options = {}
+      options[:committer] = committer
+      options[:author] = committer
+      options[:commit] = {
+        message: message,
+        branch: ref,
+      }
 
-    options[:file] = {
-      content: content,
-      path: path
-    }
+      options[:file] = {
+        content: content,
+        path: path
+      }
 
-    Gitlab::Git::Blob.commit(raw_repository, options)
+      Gitlab::Git::Blob.commit(raw_repository, options)
+    end
   end
 
-  def remove_file(user, path, message, ref)
-    path[0] = '' if path[0] == '/'
+  def remove_file(user, path, message, branch)
+    commit_with_hooks(user, branch) do |ref|
+      path[0] = '' if path[0] == '/'
 
-    committer = user_to_comitter(user)
-    options = {}
-    options[:committer] = committer
-    options[:author] = committer
-    options[:commit] = {
-      message: message,
-      branch: ref
-    }
+      committer = user_to_comitter(user)
+      options = {}
+      options[:committer] = committer
+      options[:author] = committer
+      options[:commit] = {
+        message: message,
+        branch: ref
+      }
 
-    options[:file] = {
-      path: path
-    }
+      options[:file] = {
+        path: path
+      }
 
-    Gitlab::Git::Blob.remove(raw_repository, options)
+      Gitlab::Git::Blob.remove(raw_repository, options)
+    end
   end
 
   def user_to_comitter(user)
@@ -422,7 +431,7 @@ class Repository
     end
   end
 
-  def merge(source_sha, target_branch, options = {})
+  def merge(user, source_sha, target_branch, options = {})
     our_commit = rugged.branches[target_branch].target
     their_commit = rugged.lookup(source_sha)
 
@@ -432,13 +441,15 @@ class Repository
     merge_index = rugged.merge_commits(our_commit, their_commit)
     return false if merge_index.conflicts?
 
-    actual_options = options.merge(
-      parents: [our_commit, their_commit],
-      tree: merge_index.write_tree(rugged),
-      update_ref: "refs/heads/#{target_branch}"
-    )
+    commit_with_hooks(user, target_branch) do |ref|
+      actual_options = options.merge(
+        parents: [our_commit, their_commit],
+        tree: merge_index.write_tree(rugged),
+        update_ref: ref
+      )
 
-    Rugged::Commit.create(rugged, actual_options)
+      Rugged::Commit.create(rugged, actual_options)
+    end
   end
 
   def search_files(query, ref)
@@ -479,6 +490,59 @@ class Repository
     Gitlab::Popen.popen(args, path_to_repo)
   end
 
+  def commit_with_hooks(current_user, branch)
+    oldrev = Gitlab::Git::BLANK_SHA
+    ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
+    gl_id = Gitlab::ShellEnv.gl_id(current_user)
+    was_empty = empty?
+
+    # Create temporary ref
+    random_string = SecureRandom.hex
+    tmp_ref = "refs/tmp/#{random_string}/head"
+
+    unless was_empty
+      oldrev = find_branch(branch).target
+      rugged.references.create(tmp_ref, oldrev)
+    end
+
+    # Make commit in tmp ref
+    newrev = yield(tmp_ref)
+
+    unless newrev
+      raise CommitError.new('Failed to create commit')
+    end
+
+    # Run GitLab pre-receive hook
+    pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', path_to_repo)
+    status = pre_receive_hook.trigger(gl_id, oldrev, newrev, ref)
+
+    if status
+      if was_empty
+        # Create branch
+        rugged.references.create(ref, newrev)
+      else
+        # Update head
+        current_head = find_branch(branch).target
+
+        # Make sure target branch was not changed during pre-receive hook
+        if current_head == oldrev
+          rugged.references.update(ref, newrev)
+        else
+          raise CommitError.new('Commit was rejected because branch received new push')
+        end
+      end
+
+      # Run GitLab post receive hook
+      post_receive_hook = Gitlab::Git::Hook.new('post-receive', path_to_repo)
+      status = post_receive_hook.trigger(gl_id, oldrev, newrev, ref)
+    else
+      # Remove tmp ref and return error to user
+      rugged.references.delete(tmp_ref)
+
+      raise PreReceiveError.new('Commit was rejected by pre-receive hook')
+    end
+  end
+
   private
 
   def cache
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index d7b40ee8906..7aecee217d8 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -22,21 +22,16 @@ module Files
       end
 
       if sha = commit
-        after_commit(sha, @target_branch)
         success
       else
         error("Something went wrong. Your changes were not committed")
       end
-    rescue ValidationError => ex
+    rescue Repository::CommitError, Repository::PreReceiveError, ValidationError => ex
       error(ex.message)
     end
 
     private
 
-    def after_commit(sha, branch)
-      PostCommitService.new(project, current_user).execute(sha, branch)
-    end
-
     def current_branch
       @current_branch ||= params[:current_branch]
     end
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 2107529a21a..98a67c0bc99 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -17,7 +17,7 @@ module MergeRequests
       end
 
       merge_request.in_locked_state do
-        if merge_changes
+        if commit
           after_merge
           success
         else
@@ -28,12 +28,6 @@ module MergeRequests
 
     private
 
-    def merge_changes
-      if sha = commit
-        after_commit(sha, merge_request.target_branch)
-      end
-    end
-
     def commit
       committer = repository.user_to_comitter(current_user)
 
@@ -43,11 +37,7 @@ module MergeRequests
         committer: committer
       }
 
-      repository.merge(merge_request.source_sha, merge_request.target_branch, options)
-    end
-
-    def after_commit(sha, branch)
-      PostCommitService.new(project, current_user).execute(sha, branch)
+      repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options)
     end
 
     def after_merge
diff --git a/app/services/post_commit_service.rb b/app/services/post_commit_service.rb
deleted file mode 100644
index 8592c8d238b..00000000000
--- a/app/services/post_commit_service.rb
+++ /dev/null
@@ -1,71 +0,0 @@
-class PostCommitService < BaseService
-  include Gitlab::Popen
-
-  attr_reader :changes, :repo_path
-
-  def execute(sha, branch)
-    commit = repository.commit(sha)
-    full_ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
-    old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA
-    @changes = "#{old_sha} #{sha} #{full_ref}"
-    @repo_path = repository.path_to_repo
-
-    post_receive
-  end
-
-  private
-
-  def post_receive
-    hook = hook_file('post-receive', repo_path)
-    return true if hook.nil?
-    call_receive_hook(hook)
-  end
-
-  def call_receive_hook(hook)
-    # function  will return true if succesful
-    exit_status = false
-
-    vars = {
-      'GL_ID' => Gitlab::ShellEnv.gl_id(current_user),
-      'PWD' => repo_path
-    }
-
-    options = {
-      chdir: repo_path
-    }
-
-    # we combine both stdout and stderr as we don't know what stream
-    # will be used by the custom hook
-    Open3.popen2e(vars, hook, options) do |stdin, stdout_stderr, wait_thr|
-      exit_status = true
-      stdin.sync = true
-
-      # in git, pre- and post- receive hooks may just exit without
-      # reading stdin. We catch the exception to avoid a broken pipe
-      # warning
-      begin
-        # inject all the changes as stdin to the hook
-        changes.lines do |line|
-          stdin.puts line
-        end
-      rescue Errno::EPIPE
-      end
-
-      # need to close stdin before reading stdout
-      stdin.close
-
-      # only output stdut_stderr if scripts doesn't return 0
-      unless wait_thr.value == 0
-        exit_status = false
-      end
-    end
-
-    exit_status
-  end
-
-  def hook_file(hook_type, repo_path)
-    hook_path = File.join(repo_path.strip, 'hooks')
-    hook_file = "#{hook_path}/#{hook_type}"
-    hook_file if File.exist?(hook_file)
-  end
-end
diff --git a/features/support/env.rb b/features/support/env.rb
index 672251af084..62c80b9c948 100644
--- a/features/support/env.rb
+++ b/features/support/env.rb
@@ -28,5 +28,9 @@ Spinach.hooks.before_run do
   RSpec::Mocks.setup
   TestEnv.init(mailer: false)
 
+  # skip pre-receive hook check so we can use
+  # web editor and merge
+  TestEnv.disable_pre_receive
+
   include FactoryGirl::Syntax::Methods
 end
diff --git a/lib/gitlab/git/hook.rb b/lib/gitlab/git/hook.rb
new file mode 100644
index 00000000000..dd393fe09d2
--- /dev/null
+++ b/lib/gitlab/git/hook.rb
@@ -0,0 +1,59 @@
+module Gitlab
+  module Git
+    class Hook
+      attr_reader :name, :repo_path, :path
+
+      def initialize(name, repo_path)
+        @name = name
+        @repo_path = repo_path
+        @path = File.join(repo_path.strip, 'hooks', name)
+      end
+
+      def exists?
+        File.exist?(path)
+      end
+
+      def trigger(gl_id, oldrev, newrev, ref)
+        return true unless exists?
+
+        changes = [oldrev, newrev, ref].join(" ")
+
+        # function  will return true if succesful
+        exit_status = false
+
+        vars = {
+          'GL_ID' => gl_id,
+          'PWD' => repo_path
+        }
+
+        options = {
+          chdir: repo_path
+        }
+
+        Open3.popen2(vars, path, options) do |stdin, _, wait_thr|
+          exit_status = true
+          stdin.sync = true
+
+          # in git, pre- and post- receive hooks may just exit without
+          # reading stdin. We catch the exception to avoid a broken pipe
+          # warning
+          begin
+            # inject all the changes as stdin to the hook
+            changes.lines do |line|
+              stdin.puts line
+            end
+          rescue Errno::EPIPE
+          end
+
+          stdin.close
+
+          unless wait_thr.value == 0
+            exit_status = false
+          end
+        end
+
+        exit_status
+      end
+    end
+  end
+end
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 8dc687c3580..3eab74ba986 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -57,6 +57,10 @@ module TestEnv
       and_call_original
   end
 
+  def disable_pre_receive
+    allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true)
+  end
+
   # Clean /tmp/tests
   #
   # Keeps gitlab-shell and gitlab-test