Merge remote-tracking branch 'upstream/master' into no-ivar-in-modules

* upstream/master: (168 commits)
  Update CHANGELOG.md for 10.0.1
  Remove Grit settings from default settings
  Fix duplicate key errors in PostDeployMigrateUserExternalMailData migration
  Workaround for #38259
  Workaround for n+1 in Projects::TreeController#show
  Removed old icons from project page
  Make branches page translatable
  fix typo in icons section
  Don't show it if there's no project.
  Update CHANGELOG.md for 10.0.0
  Inform user that current shared projects will remain shared
  Allow the git circuit breaker to correctly handle missing repository storages
  Reserve refs/replace cos `git-replace` is using it
  Resolve "Better SVG Usage in the Frontend"
  Replace the 'project/service.feature' spinach test with an rspec analog
  Replace the 'project/shortcuts.feature' spinach test with an rspec analog
  Removed two legacy config options
  Fix rendering double note issue.
  IssueNotes: Switch back to Write pane when note cancel or submit.
  Upgrade Nokogiri because of CVE-2017-9050
  ...

12 files changed, 206 insertions, 35 deletions

diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb
index 8d96826f6ee..a4336facee5 100644
--- a/lib/gitlab/git/blob.rb
+++ b/lib/gitlab/git/blob.rb
@@ -32,6 +32,8 @@ module Gitlab
             else
               blob = repository.lookup(sha)
 
+              next unless blob.is_a?(Rugged::Blob)
+
               new(
                 id: blob.oid,
                 size: blob.size,
diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb
index 1f370686186..1957c254c28 100644
--- a/lib/gitlab/git/commit.rb
+++ b/lib/gitlab/git/commit.rb
@@ -413,6 +413,10 @@ module Gitlab
                            end
       end
 
+      def merge_commit?
+        parent_ids.size > 1
+      end
+
       private
 
       def init_from_hash(hash)
diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb
index a23c8cf0dd1..096301d300f 100644
--- a/lib/gitlab/git/diff.rb
+++ b/lib/gitlab/git/diff.rb
@@ -206,6 +206,10 @@ module Gitlab
         Diff.binary_message(@old_path, @new_path)
       end
 
+      def has_binary_notice?
+        @diff.start_with?('Binary')
+      end
+
       private
 
       def init_from_rugged(rugged)
diff --git a/lib/gitlab/git/hook.rb b/lib/gitlab/git/hook.rb
index cc35d77c6e4..208e4bbaf60 100644
--- a/lib/gitlab/git/hook.rb
+++ b/lib/gitlab/git/hook.rb
@@ -83,13 +83,14 @@ module Gitlab
       def call_update_hook(gl_id, oldrev, newrev, ref)
         Dir.chdir(repo_path) do
           stdout, stderr, status = Open3.capture3({ 'GL_ID' => gl_id }, path, ref, oldrev, newrev)
-          [status.success?, stderr.presence || stdout]
+          [status.success?, (stderr.presence || stdout).gsub(/\R/, "<br>").html_safe]
         end
       end
 
       def retrieve_error_message(stderr, stdout)
-        err_message = stderr.gets
-        err_message.blank? ? stdout.gets : err_message
+        err_message = stderr.read
+        err_message = err_message.blank? ? stdout.read : err_message
+        err_message.gsub(/\R/, "<br>").html_safe
       end
     end
   end
diff --git a/lib/gitlab/git/operation_service.rb b/lib/gitlab/git/operation_service.rb
index dcdec818f5e..786e2e7e8dc 100644
--- a/lib/gitlab/git/operation_service.rb
+++ b/lib/gitlab/git/operation_service.rb
@@ -1,6 +1,8 @@
 module Gitlab
   module Git
     class OperationService
+      include Gitlab::Git::Popen
+
       WithBranchResult = Struct.new(:newrev, :repo_created, :branch_created) do
         alias_method :repo_created?, :repo_created
         alias_method :branch_created?, :branch_created
@@ -15,9 +17,7 @@ module Gitlab
         end
 
         # Refactoring aid
-        unless new_repository.is_a?(Gitlab::Git::Repository)
-          raise "expected a Gitlab::Git::Repository, got #{new_repository}"
-        end
+        Gitlab::Git.check_namespace!(new_repository)
 
         @repository = new_repository
       end
@@ -152,7 +152,7 @@ module Gitlab
         # (and have!) accidentally reset the ref to an earlier state, clobbering
         # commits. See also https://github.com/libgit2/libgit2/issues/1534.
         command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z]
-        _, status = Gitlab::Popen.popen(
+        _, status = popen(
           command,
           repository.path) do |stdin|
           stdin.write("update #{ref}\x00#{newrev}\x00#{oldrev}\x00")
diff --git a/lib/gitlab/git/popen.rb b/lib/gitlab/git/popen.rb
index 10c15b316f5..054d45895a5 100644
--- a/lib/gitlab/git/popen.rb
+++ b/lib/gitlab/git/popen.rb
@@ -5,17 +5,21 @@ require 'open3'
 module Gitlab
   module Git
     module Popen
-      def popen(cmd, path)
+      def popen(cmd, path, vars = {})
         unless cmd.is_a?(Array)
           raise "System commands must be given as an array of strings"
         end
 
-        vars = { "PWD" => path }
+        path ||= Dir.pwd
+        vars['PWD'] = path
         options = { chdir: path }
 
         cmd_output = ""
         cmd_status = 0
         Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
+          yield(stdin) if block_given?
+          stdin.close
+
           cmd_output << stdout.read
           cmd_output << stderr.read
           cmd_status = wait_thr.value.exitstatus
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index c499ff101b5..10ba29acbd1 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -19,6 +19,7 @@ module Gitlab
       InvalidRef = Class.new(StandardError)
       GitError = Class.new(StandardError)
       DeleteBranchError = Class.new(StandardError)
+      CreateTreeError = Class.new(StandardError)
 
       class << self
         # Unlike `new`, `create` takes the storage path, not the storage name
@@ -474,7 +475,15 @@ module Gitlab
       # diff options.  The +options+ hash can also include :break_rewrites to
       # split larger rewrites into delete/add pairs.
       def diff(from, to, options = {}, *paths)
-        Gitlab::Git::DiffCollection.new(diff_patches(from, to, options, *paths), options)
+        iterator = gitaly_migrate(:diff_between) do |is_enabled|
+          if is_enabled
+            gitaly_commit_client.diff(from, to, options.merge(paths: paths))
+          else
+            diff_patches(from, to, options, *paths)
+          end
+        end
+
+        Gitlab::Git::DiffCollection.new(iterator, options)
       end
 
       # Returns a RefName for a given SHA
@@ -489,7 +498,7 @@ module Gitlab
 
             # Not found -> ["", 0]
             # Found -> ["b8d95eb4969eefacb0a58f6a28f6803f8070e7b9 commit\trefs/environments/production/77\n", 0]
-            Gitlab::Popen.popen(args, @path).first.split.last
+            popen(args, @path).first.split.last
           end
         end
       end
@@ -684,6 +693,88 @@ module Gitlab
         nil
       end
 
+      def revert(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
+        OperationService.new(user, self).with_branch(
+          branch_name,
+          start_branch_name: start_branch_name,
+          start_repository: start_repository
+        ) do |start_commit|
+
+          Gitlab::Git.check_namespace!(commit, start_repository)
+
+          revert_tree_id = check_revert_content(commit, start_commit.sha)
+          raise CreateTreeError unless revert_tree_id
+
+          committer = user_to_committer(user)
+
+          create_commit(message: message,
+                        author: committer,
+                        committer: committer,
+                        tree: revert_tree_id,
+                        parents: [start_commit.sha])
+        end
+      end
+
+      def check_revert_content(target_commit, source_sha)
+        args = [target_commit.sha, source_sha]
+        args << { mainline: 1 } if target_commit.merge_commit?
+
+        revert_index = rugged.revert_commit(*args)
+        return false if revert_index.conflicts?
+
+        tree_id = revert_index.write_tree(rugged)
+        return false unless diff_exists?(source_sha, tree_id)
+
+        tree_id
+      end
+
+      def cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
+        OperationService.new(user, self).with_branch(
+          branch_name,
+          start_branch_name: start_branch_name,
+          start_repository: start_repository
+        ) do |start_commit|
+
+          Gitlab::Git.check_namespace!(commit, start_repository)
+
+          cherry_pick_tree_id = check_cherry_pick_content(commit, start_commit.sha)
+          raise CreateTreeError unless cherry_pick_tree_id
+
+          committer = user_to_committer(user)
+
+          create_commit(message: message,
+                        author: {
+                            email: commit.author_email,
+                            name: commit.author_name,
+                            time: commit.authored_date
+                        },
+                        committer: committer,
+                        tree: cherry_pick_tree_id,
+                        parents: [start_commit.sha])
+        end
+      end
+
+      def check_cherry_pick_content(target_commit, source_sha)
+        args = [target_commit.sha, source_sha]
+        args << 1 if target_commit.merge_commit?
+
+        cherry_pick_index = rugged.cherrypick_commit(*args)
+        return false if cherry_pick_index.conflicts?
+
+        tree_id = cherry_pick_index.write_tree(rugged)
+        return false unless diff_exists?(source_sha, tree_id)
+
+        tree_id
+      end
+
+      def diff_exists?(sha1, sha2)
+        rugged.diff(sha1, sha2).size > 0
+      end
+
+      def user_to_committer(user)
+        Gitlab::Git.committer_hash(email: user.email, name: user.name)
+      end
+
       def create_commit(params = {})
         params[:message].delete!("\r")
 
@@ -709,9 +800,7 @@ module Gitlab
         end
 
         command = %W[#{Gitlab.config.git.bin_path} update-ref --stdin -z]
-        message, status = Gitlab::Popen.popen(
-          command,
-          path) do |stdin|
+        message, status = popen(command, path) do |stdin|
           stdin.write(instructions.join)
         end
 
@@ -835,7 +924,7 @@ module Gitlab
       end
 
       def with_repo_branch_commit(start_repository, start_branch_name)
-        raise "expected Gitlab::Git::Repository, got #{start_repository}" unless start_repository.is_a?(Gitlab::Git::Repository)
+        Gitlab::Git.check_namespace!(start_repository)
 
         return yield nil if start_repository.empty_repo?
 
@@ -950,8 +1039,8 @@ module Gitlab
         @gitaly_repository_client ||= Gitlab::GitalyClient::RepositoryService.new(self)
       end
 
-      def gitaly_migrate(method, &block)
-        Gitlab::GitalyClient.migrate(method, &block)
+      def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block)
+        Gitlab::GitalyClient.migrate(method, status: status, &block)
       rescue GRPC::NotFound => e
         raise NoRepository.new(e)
       rescue GRPC::BadStatus => e
@@ -962,14 +1051,17 @@ module Gitlab
 
       # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'.
       def branches_filter(filter: nil, sort_by: nil)
-        branches = rugged.branches.each(filter).map do |rugged_ref|
-          begin
-            target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target)
-            Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit)
-          rescue Rugged::ReferenceError
-            # Omit invalid branch
-          end
-        end.compact
+        # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37464
+        branches = Gitlab::GitalyClient.allow_n_plus_1_calls do
+          rugged.branches.each(filter).map do |rugged_ref|
+            begin
+              target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target)
+              Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit)
+            rescue Rugged::ReferenceError
+              # Omit invalid branch
+            end
+          end.compact
+        end
 
         sort_branches(branches, sort_by)
       end
diff --git a/lib/gitlab/git/rev_list.rb b/lib/gitlab/git/rev_list.rb
index 2b5785a1f08..e0943d3a3eb 100644
--- a/lib/gitlab/git/rev_list.rb
+++ b/lib/gitlab/git/rev_list.rb
@@ -3,6 +3,8 @@
 module Gitlab
   module Git
     class RevList
+      include Gitlab::Git::Popen
+
       attr_reader :oldrev, :newrev, :path_to_repo
 
       def initialize(path_to_repo:, newrev:, oldrev: nil)
@@ -26,7 +28,7 @@ module Gitlab
       private
 
       def execute(args)
-        output, status = Gitlab::Popen.popen(args, nil, Gitlab::Git::Env.all.stringify_keys)
+        output, status = popen(args, nil, Gitlab::Git::Env.all.stringify_keys)
 
         unless status.zero?
           raise "Got a non-zero exit code while calling out `#{args.join(' ')}`."
diff --git a/lib/gitlab/git/storage.rb b/lib/gitlab/git/storage.rb
index e28be4b8a38..08e6c29abad 100644
--- a/lib/gitlab/git/storage.rb
+++ b/lib/gitlab/git/storage.rb
@@ -11,6 +11,7 @@ module Gitlab
       end
 
       CircuitOpen = Class.new(Inaccessible)
+      Misconfiguration = Class.new(Inaccessible)
 
       REDIS_KEY_PREFIX = 'storage_accessible:'.freeze
 
diff --git a/lib/gitlab/git/storage/circuit_breaker.rb b/lib/gitlab/git/storage/circuit_breaker.rb
index 9ea9367d4b7..1eaa2d83fb6 100644
--- a/lib/gitlab/git/storage/circuit_breaker.rb
+++ b/lib/gitlab/git/storage/circuit_breaker.rb
@@ -28,14 +28,26 @@ module Gitlab
         def self.for_storage(storage)
           cached_circuitbreakers = RequestStore.fetch(:circuitbreaker_cache) do
             Hash.new do |hash, storage_name|
-              hash[storage_name] = new(storage_name)
+              hash[storage_name] = build(storage_name)
             end
           end
 
           cached_circuitbreakers[storage]
         end
 
-        def initialize(storage, hostname = Gitlab::Environment.hostname)
+        def self.build(storage, hostname = Gitlab::Environment.hostname)
+          config = Gitlab.config.repositories.storages[storage]
+
+          if !config.present?
+            NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Storage '#{storage}' is not configured"))
+          elsif !config['path'].present?
+            NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Path for storage '#{storage}' is not configured"))
+          else
+            new(storage, hostname)
+          end
+        end
+
+        def initialize(storage, hostname)
           @storage = storage
           @hostname = hostname
 
@@ -64,6 +76,10 @@ module Gitlab
           recent_failure || too_many_failures
         end
 
+        def failure_info
+          @failure_info ||= get_failure_info
+        end
+
         # Memoizing the `storage_available` call means we only do it once per
         # request when the storage is available.
         #
@@ -121,10 +137,12 @@ module Gitlab
           end
         end
 
-        def failure_info
-          @failure_info ||= get_failure_info
+        def cache_key
+          @cache_key ||= "#{Gitlab::Git::Storage::REDIS_KEY_PREFIX}#{storage}:#{hostname}"
         end
 
+        private
+
         def get_failure_info
           last_failure, failure_count = Gitlab::Git::Storage.redis.with do |redis|
             redis.hmget(cache_key, :last_failure, :failure_count)
@@ -134,10 +152,6 @@ module Gitlab
 
           FailureInfo.new(last_failure, failure_count.to_i)
         end
-
-        def cache_key
-          @cache_key ||= "#{Gitlab::Git::Storage::REDIS_KEY_PREFIX}#{storage}:#{hostname}"
-        end
       end
     end
   end
diff --git a/lib/gitlab/git/storage/health.rb b/lib/gitlab/git/storage/health.rb
index 2d723147f4f..1564e94b7f7 100644
--- a/lib/gitlab/git/storage/health.rb
+++ b/lib/gitlab/git/storage/health.rb
@@ -78,7 +78,7 @@ module Gitlab
 
         def failing_circuit_breakers
           @failing_circuit_breakers ||= failing_on_hosts.map do |hostname|
-            CircuitBreaker.new(storage_name, hostname)
+            CircuitBreaker.build(storage_name, hostname)
           end
         end
 
diff --git a/lib/gitlab/git/storage/null_circuit_breaker.rb b/lib/gitlab/git/storage/null_circuit_breaker.rb
new file mode 100644
index 00000000000..297c043d054
--- /dev/null
+++ b/lib/gitlab/git/storage/null_circuit_breaker.rb
@@ -0,0 +1,47 @@
+module Gitlab
+  module Git
+    module Storage
+      class NullCircuitBreaker
+        # These will have actual values
+        attr_reader :storage,
+                    :hostname
+
+        # These will always have nil values
+        attr_reader :storage_path,
+                    :failure_wait_time,
+                    :failure_reset_time,
+                    :storage_timeout
+
+        def initialize(storage, hostname, error: nil)
+          @storage = storage
+          @hostname = hostname
+          @error = error
+        end
+
+        def perform
+          @error ? raise(@error) : yield
+        end
+
+        def circuit_broken?
+          !!@error
+        end
+
+        def failure_count_threshold
+          1
+        end
+
+        def last_failure
+          circuit_broken? ? Time.now : nil
+        end
+
+        def failure_count
+          circuit_broken? ? 1 : 0
+        end
+
+        def failure_info
+          Gitlab::Git::Storage::CircuitBreaker::FailureInfo.new(last_failure, failure_count)
+        end
+      end
+    end
+  end
+end