Fix of 'Commits being passed to custom hooks are already reachable when using the UI'backport_from_ee_easy_commit_creation

5 files changed, 46 insertions, 62 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 600b554a118..466e7907721 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,6 @@
 Please view this file on the master branch, on stable branches it's out of date.
+v 8.11.0 (unreleased)
+  - Fix of 'Commits being passed to custom hooks are already reachable when using the UI'
 
 v 8.10.0 (unreleased)
   - Expose {should,force}_remove_source_branch (Ben Boeckel)
diff --git a/Gemfile b/Gemfile
index 81e8ff60ad5..08d3dbb53ea 100644
--- a/Gemfile
+++ b/Gemfile
@@ -52,7 +52,7 @@ gem 'browser', '~> 2.2'
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem 'gitlab_git', '~> 10.2'
+gem 'gitlab_git', '~> 10.3'
 
 # LDAP Auth
 # GitLab fork with several improvements to original library. For full list of changes
diff --git a/Gemfile.lock b/Gemfile.lock
index 0987fd5665a..3bed835d54d 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -274,7 +274,7 @@ GEM
       diff-lcs (~> 1.1)
       mime-types (>= 1.16, < 3)
       posix-spawn (~> 0.3)
-    gitlab_git (10.2.3)
+    gitlab_git (10.3.1)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.7.3)
       github-linguist (~> 4.7.0)
@@ -861,7 +861,7 @@ DEPENDENCIES
   github-linguist (~> 4.7.0)
   github-markup (~> 1.4)
   gitlab-flowdock-git-hook (~> 1.0.1)
-  gitlab_git (~> 10.2)
+  gitlab_git (~> 10.3)
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (~> 1.2.1)
   gollum-lib (~> 4.2)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 5b670cb4b8f..33f2e6b028c 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -704,6 +704,7 @@ class Repository
       options[:commit] = {
         message: message,
         branch: ref,
+        update_ref: false,
       }
 
       raw_repository.mkdir(path, options)
@@ -719,6 +720,7 @@ class Repository
       options[:commit] = {
         message: message,
         branch: ref,
+        update_ref: false,
       }
 
       options[:file] = {
@@ -739,7 +741,8 @@ class Repository
       options[:author] = committer
       options[:commit] = {
         message: message,
-        branch: ref
+        branch: ref,
+        update_ref: false,
       }
 
       options[:file] = {
@@ -779,11 +782,10 @@ class Repository
     merge_index = rugged.merge_commits(our_commit, their_commit)
     return false if merge_index.conflicts?
 
-    commit_with_hooks(user, target_branch) do |ref|
+    commit_with_hooks(user, target_branch) do
       actual_options = options.merge(
         parents: [our_commit, their_commit],
         tree: merge_index.write_tree(rugged),
-        update_ref: ref
       )
 
       Rugged::Commit.create(rugged, actual_options)
@@ -796,15 +798,14 @@ class Repository
 
     return false unless revert_tree_id
 
-    commit_with_hooks(user, base_branch) do |ref|
+    commit_with_hooks(user, base_branch) do
       committer = user_to_committer(user)
       source_sha = Rugged::Commit.create(rugged,
         message: commit.revert_message,
         author: committer,
         committer: committer,
         tree: revert_tree_id,
-        parents: [rugged.lookup(source_sha)],
-        update_ref: ref)
+        parents: [rugged.lookup(source_sha)])
     end
   end
 
@@ -814,7 +815,7 @@ class Repository
 
     return false unless cherry_pick_tree_id
 
-    commit_with_hooks(user, base_branch) do |ref|
+    commit_with_hooks(user, base_branch) do
       committer = user_to_committer(user)
       source_sha = Rugged::Commit.create(rugged,
         message: commit.message,
@@ -825,8 +826,7 @@ class Repository
         },
         committer: committer,
         tree: cherry_pick_tree_id,
-        parents: [rugged.lookup(source_sha)],
-        update_ref: ref)
+        parents: [rugged.lookup(source_sha)])
     end
   end
 
@@ -927,20 +927,6 @@ class Repository
     Gitlab::Popen.popen(args, path_to_repo)
   end
 
-  def with_tmp_ref(oldrev = nil)
-    random_string = SecureRandom.hex
-    tmp_ref = "refs/tmp/#{random_string}/head"
-
-    if oldrev && !Gitlab::Git.blank_ref?(oldrev)
-      rugged.references.create(tmp_ref, oldrev)
-    end
-
-    # Make commit in tmp ref
-    yield(tmp_ref)
-  ensure
-    rugged.references.delete(tmp_ref) rescue nil
-  end
-
   def commit_with_hooks(current_user, branch)
     update_autocrlf_option
 
@@ -953,33 +939,31 @@ class Repository
       oldrev = target_branch.target
     end
 
-    with_tmp_ref(oldrev) do |tmp_ref|
-      # Make commit in tmp ref
-      newrev = yield(tmp_ref)
+    # Make commit
+    newrev = yield(ref)
 
-      unless newrev
-        raise CommitError.new('Failed to create commit')
-      end
+    unless newrev
+      raise CommitError.new('Failed to create commit')
+    end
+
+    GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do
+      if was_empty || !target_branch
+        # Create branch
+        rugged.references.create(ref, newrev)
+      else
+        # Update head
+        current_head = find_branch(branch).target
 
-      GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do
-        if was_empty || !target_branch
-          # Create branch
-          rugged.references.create(ref, newrev)
+        # Make sure target branch was not changed during pre-receive hook
+        if current_head == oldrev
+          rugged.references.update(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
+          raise CommitError.new('Commit was rejected because branch received new push')
         end
       end
-
-      newrev
     end
+
+    newrev
   end
 
   def ls_files(ref)
diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb
index d874582d54f..757fc35a78f 100644
--- a/app/services/create_branch_service.rb
+++ b/app/services/create_branch_service.rb
@@ -15,21 +15,19 @@ class CreateBranchService < BaseService
       return error('Branch already exists')
     end
 
-    new_branch = nil
-
-    if source_project != @project
-      repository.with_tmp_ref do |tmp_ref|
-        repository.fetch_ref(
-          source_project.repository.path_to_repo,
-          "refs/heads/#{ref}",
-          tmp_ref
-        )
-
-        new_branch = repository.add_branch(current_user, branch_name, tmp_ref)
-      end
-    else
-      new_branch = repository.add_branch(current_user, branch_name, ref)
-    end
+    new_branch = if source_project != @project
+                   repository.fetch_ref(
+                     source_project.repository.path_to_repo,
+                     "refs/heads/#{ref}",
+                     "refs/heads/#{branch_name}"
+                   )
+
+                   repository.after_create_branch
+
+                   repository.find_branch(branch_name)
+                 else
+                   repository.add_branch(current_user, branch_name, ref)
+                 end
 
     if new_branch
       success(new_branch)