Merge branch 'contrain-run-git' into 'master'

Make Gitlab::Git::Repository#run_git private

See merge request gitlab-org/gitlab-ce!16622

2 files changed, 97 insertions, 83 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 824e18bec78..5101c087a50 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -166,16 +166,10 @@ class Repository
       return []
     end
 
-    raw_repository.gitaly_migrate(:commits_by_message) do |is_enabled|
-      commits =
-        if is_enabled
-          find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
-        else
-          find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
-        end
-
-      CommitCollection.new(project, commits, ref)
+    commits = raw_repository.find_commits_by_message(query, ref, path, limit, offset).map do |c|
+      commit(c)
     end
+    CommitCollection.new(project, commits, ref)
   end
 
   def find_branch(name, fresh_repo: true)
@@ -740,23 +734,6 @@ class Repository
     Commit.order_by(collection: commits, order_by: order_by, sort: sort)
   end
 
-  def refs_contains_sha(ref_type, sha)
-    args = %W(#{ref_type} --contains #{sha})
-    names = run_git(args).first
-
-    if names.respond_to?(:split)
-      names = names.split("\n").map(&:strip)
-
-      names.each do |name|
-        name.slice! '* '
-      end
-
-      names
-    else
-      []
-    end
-  end
-
   def branch_names_contains(sha)
     refs_contains_sha('branch', sha)
   end
@@ -921,25 +898,6 @@ class Repository
     end
   end
 
-  def search_files_by_content(query, ref)
-    return [] if empty? || query.blank?
-
-    offset = 2
-    args = %W(grep -i -I -n -z --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref})
-
-    run_git(args).first.scrub.split(/^--$/)
-  end
-
-  def search_files_by_name(query, ref)
-    safe_query = Regexp.escape(query.sub(/^\/*/, ""))
-
-    return [] if empty? || safe_query.blank?
-
-    args = %W(ls-tree --full-tree -r #{ref || root_ref} --name-status | #{safe_query})
-
-    run_git(args).first.lines.map(&:strip)
-  end
-
   def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil)
     unless remote_name
       remote_name = "tmp-#{SecureRandom.hex}"
@@ -973,6 +931,18 @@ class Repository
     raw_repository.ls_files(actual_ref)
   end
 
+  def search_files_by_content(query, ref)
+    return [] if empty? || query.blank?
+
+    raw_repository.search_files_by_content(query, ref)
+  end
+
+  def search_files_by_name(query, ref)
+    return [] if empty?
+
+    raw_repository.search_files_by_name(query, ref)
+  end
+
   def copy_gitattributes(ref)
     actual_ref = ref || root_ref
     begin
@@ -1133,25 +1103,4 @@ class Repository
   def rugged_can_be_merged?(their_commit, our_commit)
     !rugged.merge_commits(our_commit, their_commit).conflicts?
   end
-
-  def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
-    ref ||= root_ref
-
-    args = %W(
-      log #{ref} --pretty=%H --skip #{offset}
-      --max-count #{limit} --grep=#{query} --regexp-ignore-case
-    )
-    args = args.concat(%W(-- #{path})) if path.present?
-
-    git_log_results = run_git(args).first.lines
-
-    git_log_results.map { |c| commit(c.chomp) }.compact
-  end
-
-  def find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
-    raw_repository
-      .gitaly_commit_client
-      .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
-      .map { |c| commit(c) }
-  end
 end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 6ecb3ad6c70..d6436c8c8a6 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -1126,23 +1126,6 @@ module Gitlab
       end
 
       # Refactoring aid; allows us to copy code from app/models/repository.rb
-      def run_git(args, chdir: path, env: {}, nice: false, &block)
-        cmd = [Gitlab.config.git.bin_path, *args]
-        cmd.unshift("nice") if nice
-        circuit_breaker.perform do
-          popen(cmd, chdir, env, &block)
-        end
-      end
-
-      def run_git!(args, chdir: path, env: {}, nice: false, &block)
-        output, status = run_git(args, chdir: chdir, env: env, nice: nice, &block)
-
-        raise GitError, output unless status.zero?
-
-        output
-      end
-
-      # Refactoring aid; allows us to copy code from app/models/repository.rb
       def run_git_with_timeout(args, timeout, env: {})
         circuit_breaker.perform do
           popen_with_timeout([Gitlab.config.git.bin_path, *args], timeout, path, env)
@@ -1365,6 +1348,52 @@ module Gitlab
         raise CommandError.new(e)
       end
 
+      def refs_contains_sha(ref_type, sha)
+        args = %W(#{ref_type} --contains #{sha})
+        names = run_git(args).first
+
+        if names.respond_to?(:split)
+          names = names.split("\n").map(&:strip)
+
+          names.each do |name|
+            name.slice! '* '
+          end
+
+          names
+        else
+          []
+        end
+      end
+
+      def search_files_by_content(query, ref)
+        return [] if empty? || query.blank?
+
+        offset = 2
+        args = %W(grep -i -I -n -z --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref})
+
+        run_git(args).first.scrub.split(/^--$/)
+      end
+
+      def search_files_by_name(query, ref)
+        safe_query = Regexp.escape(query.sub(/^\/*/, ""))
+
+        return [] if empty? || safe_query.blank?
+
+        args = %W(ls-tree --full-tree -r #{ref || root_ref} --name-status | #{safe_query})
+
+        run_git(args).first.lines.map(&:strip)
+      end
+
+      def find_commits_by_message(query, ref, path, limit, offset)
+        gitaly_migrate(:commits_by_message) do |is_enabled|
+          if is_enabled
+            find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
+          else
+            find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
+          end
+        end
+      end
+
       private
 
       def shell_write_ref(ref_path, ref, old_ref)
@@ -1386,6 +1415,22 @@ module Gitlab
         Rails.logger.error "Unable to create #{ref_path} reference for repository #{path}: #{ex}"
       end
 
+      def run_git(args, chdir: path, env: {}, nice: false, &block)
+        cmd = [Gitlab.config.git.bin_path, *args]
+        cmd.unshift("nice") if nice
+        circuit_breaker.perform do
+          popen(cmd, chdir, env, &block)
+        end
+      end
+
+      def run_git!(args, chdir: path, env: {}, nice: false, &block)
+        output, status = run_git(args, chdir: chdir, env: env, nice: nice, &block)
+
+        raise GitError, output unless status.zero?
+
+        output
+      end
+
       def fresh_worktree?(path)
         File.exist?(path) && !clean_stuck_worktree(path)
       end
@@ -2161,6 +2206,26 @@ module Gitlab
       def gitlab_projects_error
         raise CommandError, @gitlab_projects.output
       end
+
+      def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
+        ref ||= root_ref
+
+        args = %W(
+          log #{ref} --pretty=%H --skip #{offset}
+          --max-count #{limit} --grep=#{query} --regexp-ignore-case
+        )
+        args = args.concat(%W(-- #{path})) if path.present?
+
+        git_log_results = run_git(args).first.lines
+
+        git_log_results.map { |c| commit(c.chomp) }.compact
+      end
+
+      def find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
+        gitaly_commit_client
+          .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
+          .map { |c| commit(c) }
+      end
     end
   end
 end