Don't execute git hooks if you create branch as part of other change

What does this MR do?

Currently, our procedure for adding a commit requires us to execute CreateBranchService before file creation. It's OK, but also we do execute git hooks (the PostReceive sidekiq job) as part of this process. However, this hook is execute before the file is actually committed, so the ref is updated.

Secondly, we do execute a git hooks after committing file and updating ref. This results in duplicate PostReceive jobs, where the first one is completely invalid.

This change makes the branch creation, something that is intermediate step of bigger process (file creation or update, commit cherry pick or revert) to not execute git hooks.

Why was this MR needed?

Adds a to CreateBranchService an option to disable hooks execution.

Does this MR meet the acceptance criteria?

• CHANGELOG entry added
• Documentation created/updated
• API support added
• Tests
  • Added for this feature/bug
  • All builds are passing
• Conform by the merge request performance guides
• Conform by the style guides
• Branch has no merge conflicts with master (if it does - rebase it please)
• Squashed related commits together

What are the relevant issue numbers?

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/23439

app/models/repository.rb

@@ -786,8 +786,12 @@ def root_ref
     @root_ref ||= cache.fetch(:root_ref) { raw_repository.root_ref }
   end
 
-  def commit_dir(user, path, message, branch, author_email: nil, author_name: nil)
-    update_branch_with_hooks(user, branch) do |ref|
+  def commit_dir(user, path, message, branch,
+    author_email: nil, author_name: nil, source_branch: nil)
+    update_branch_with_hooks(
+      user,
+      branch,
+      source_branch: source_branch) do |ref|
       options = {
         commit: {
           branch: ref,
@@ -802,8 +806,12 @@ def commit_dir(user, path, message, branch, author_email: nil, author_name: nil)
     end
   end
 
-  def commit_file(user, path, content, message, branch, update, author_email: nil, author_name: nil)
-    update_branch_with_hooks(user, branch) do |ref|
+  def commit_file(user, path, content, message, branch, update,
+    author_email: nil, author_name: nil, source_branch: nil)
+    update_branch_with_hooks(
+      user,
+      branch,
+      source_branch: source_branch) do |ref|
       options = {
         commit: {
           branch: ref,
@@ -823,8 +831,13 @@ def commit_file(user, path, content, message, branch, update, author_email: nil,
     end
   end
 
-  def update_file(user, path, content, branch:, previous_path:, message:, author_email: nil, author_name: nil)
-    update_branch_with_hooks(user, branch) do |ref|
+  def update_file(user, path, content,
+    branch:, previous_path:, message:,
+    author_email: nil, author_name: nil, source_branch: nil)
+    update_branch_with_hooks(
+      user,
+      branch,
+      source_branch: source_branch) do |ref|
       options = {
         commit: {
           branch: ref,
@@ -849,8 +862,12 @@ def update_file(user, path, content, branch:, previous_path:, message:, author_e
     end
   end
 
-  def remove_file(user, path, message, branch, author_email: nil, author_name: nil)
-    update_branch_with_hooks(user, branch) do |ref|
+  def remove_file(user, path, message, branch,
+    author_email: nil, author_name: nil, source_branch: nil)
+    update_branch_with_hooks(
+      user,
+      branch,
+      source_branch: source_branch) do |ref|
       options = {
         commit: {
           branch: ref,
@@ -868,17 +885,18 @@ def remove_file(user, path, message, branch, author_email: nil, author_name: nil
     end
   end
 
-  def multi_action(user:, branch:, message:, actions:, author_email: nil, author_name: nil)
-    update_branch_with_hooks(user, branch) do |ref|
+  def multi_action(user:, branch:, message:, actions:,
+    author_email: nil, author_name: nil, source_branch: nil)
+    update_branch_with_hooks(
+      user,
+      branch,
+      source_branch: source_branch) do |ref|
       index = rugged.index
       parents = []
-      branch = find_branch(ref)
 
-      if branch
-        last_commit = branch.dereferenced_target
-        index.read_tree(last_commit.raw_commit.tree)
-        parents = [last_commit.sha]
-      end
+      last_commit = find_branch(ref).dereferenced_target
+      index.read_tree(last_commit.raw_commit.tree)
+      parents = [last_commit.sha]
 
       actions.each do |action|
         case action[:action]
@@ -967,7 +985,10 @@ def revert(user, commit, base_branch, revert_tree_id = nil)
 
     return false unless revert_tree_id
 
-    update_branch_with_hooks(user, base_branch) do
+    update_branch_with_hooks(
+      user,
+      base_branch,
+      source_branch: revert_tree_id) do
       committer = user_to_committer(user)
       source_sha = Rugged::Commit.create(rugged,
         message: commit.revert_message,
@@ -984,7 +1005,10 @@ def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil)
 
     return false unless cherry_pick_tree_id
 
-    update_branch_with_hooks(user, base_branch) do
+    update_branch_with_hooks(
+      user,
+      base_branch,
+      source_branch: cherry_pick_tree_id) do
       committer = user_to_committer(user)
       source_sha = Rugged::Commit.create(rugged,
         message: commit.message,
@@ -1082,11 +1106,11 @@ def create_ref(ref, ref_path)
     fetch_ref(path_to_repo, ref, ref_path)
   end
 
-  def update_branch_with_hooks(current_user, branch)
+  def update_branch_with_hooks(current_user, branch, source_branch: nil)
     update_autocrlf_option
 
     ref = Gitlab::Git::BRANCH_REF_PREFIX + branch
-    target_branch = find_branch(branch)
+    target_branch, new_branch_added = raw_ensure_branch(branch, source_branch)
     was_empty = empty?
 
     # Make commit
@@ -1096,7 +1120,7 @@ def update_branch_with_hooks(current_user, branch)
       raise CommitError.new('Failed to create commit')
     end
 
-    if rugged.lookup(newrev).parent_ids.empty? || target_branch.nil?
+    if rugged.lookup(newrev).parent_ids.empty?
       oldrev = Gitlab::Git::BLANK_SHA
     else
       oldrev = rugged.merge_base(newrev, target_branch.dereferenced_target.sha)
@@ -1105,11 +1129,9 @@ def update_branch_with_hooks(current_user, branch)
     GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do
       update_ref!(ref, newrev, oldrev)
 
-      if was_empty || !target_branch
-        # If repo was empty expire cache
-        after_create if was_empty
-        after_create_branch
-      end
+      # If repo was empty expire cache
+      after_create if was_empty
+      after_create_branch if was_empty || new_branch_added
     end
 
     newrev
@@ -1169,4 +1191,28 @@ def keep_around_ref_name(sha)
   def repository_event(event, tags = {})
     Gitlab::Metrics.add_event(event, { path: path_with_namespace }.merge(tags))
   end
+
+  def raw_ensure_branch(branch_name, source_branch)
+    old_branch = find_branch(branch_name)
+
+    if old_branch
+      [old_branch, false]
+    elsif source_branch
+      oldrev = Gitlab::Git::BLANK_SHA
+      ref    = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
+      target = commit(source_branch).try(:id)
+
+      unless target
+        raise CommitError.new(
+          "Cannot find branch #{branch_name} nor #{source_branch}")
+      end
+
+      update_ref!(ref, target, oldrev)
+
+      [find_branch(branch_name), true]
+    else
+      raise CommitError.new(
+        "Cannot find branch #{branch_name} and source_branch is not set")
+    end
+  end
 end