Merge branch...

Merge branch 'backport-of-rd-3429-enabling-maximum-file-size-limit-in-repository-causes-pushes-to-fail' into 'master'

Backport of EE !4989

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

lib/gitlab/git.rb

 module Gitlab
   module Git
+    # The ID of empty tree.
+    # See http://stackoverflow.com/a/40884093/1856239 and
+    # https://github.com/git/git/blob/3ad8b5bf26362ac67c9020bf8c30eee54a84f56d/cache.h#L1011-L1012
+    EMPTY_TREE_ID = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'.freeze
     BLANK_SHA = ('0' * 40).freeze
     TAG_REF_PREFIX = "refs/tags/".freeze
     BRANCH_REF_PREFIX = "refs/heads/".freeze

lib/gitlab/git/raw_diff_change.rb

+module Gitlab
+  module Git
+    # This class behaves like a struct with fields :blob_id, :blob_size, :operation, :old_path, :new_path
+    # All those fields are (binary) strings or integers
+    class RawDiffChange
+      attr_reader :blob_id, :blob_size, :old_path, :new_path, :operation
+
+      def initialize(raw_change)
+        parse(raw_change)
+      end
+
+      private
+
+      # Input data has the following format:
+      #
+      # When a file has been modified:
+      # 7e3e39ebb9b2bf433b4ad17313770fbe4051649c 669 M\tfiles/ruby/popen.rb
+      #
+      # When a file has been renamed:
+      # 85bc2f9753afd5f4fc5d7c75f74f8d526f26b4f3 107 R060\tfiles/js/commit.js.coffee\tfiles/js/commit.coffee
+      def parse(raw_change)
+        @blob_id, @blob_size, @raw_operation, raw_paths = raw_change.split(' ', 4)
+        @operation = extract_operation
+        @old_path, @new_path = extract_paths(raw_paths)
+      end
+
+      def extract_paths(file_path)
+        case operation
+        when :renamed
+          file_path.split(/\t/)
+        when :deleted
+          [file_path, nil]
+        when :added
+          [nil, file_path]
+        else
+          [file_path, file_path]
+        end
+      end
+
+      def extract_operation
+        case @raw_operation&.first(1)
+        when 'A'
+          :added
+        when 'C'
+          :copied
+        when 'D'
+          :deleted
+        when 'M'
+          :modified
+        when 'R'
+          :renamed
+        when 'T'
+          :type_changed
+        else
+          :unknown
+        end
+      end
+    end
+  end
+end

lib/gitlab/git/repository.rb

@@ -558,6 +558,24 @@ module Gitlab
         count_commits(from: from, to: to, **options)
       end
+      # old_rev and new_rev are commit ID's
+      # the result of this method is an array of Gitlab::Git::RawDiffChange
+      def raw_changes_between(old_rev, new_rev)
+        result = []
+
+        circuit_breaker.perform do
+          Open3.pipeline_r(git_diff_cmd(old_rev, new_rev), format_git_cat_file_script, git_cat_file_cmd) do |last_stdout, wait_threads|
+            last_stdout.each_line { |line| result << ::Gitlab::Git::RawDiffChange.new(line.chomp!) }
+
+            if wait_threads.any? { |waiter| !waiter.value&.success? }
+              raise ::Gitlab::Git::Repository::GitError, "Unabled to obtain changes between #{old_rev} and #{new_rev}"
+            end
+          end
+        end
+
+        result
+      end
+
       # Returns the SHA of the most recent common ancestor of +from+ and +to+
       def merge_base(from, to)
         gitaly_migrate(:merge_base) do |is_enabled|
@@ -2483,6 +2501,35 @@ module Gitlab
         result.to_s(16)
       end
+
+      def build_git_cmd(*args)
+        object_directories = alternate_object_directories.join(File::PATH_SEPARATOR)
+
+        env = { 'PWD' => self.path }
+        env['GIT_ALTERNATE_OBJECT_DIRECTORIES'] = object_directories if object_directories.present?
+
+        [
+          env,
+          ::Gitlab.config.git.bin_path,
+          *args,
+          { chdir: self.path }
+        ]
+      end
+
+      def git_diff_cmd(old_rev, new_rev)
+        old_rev = old_rev == ::Gitlab::Git::BLANK_SHA ? ::Gitlab::Git::EMPTY_TREE_ID : old_rev
+
+        build_git_cmd('diff', old_rev, new_rev, '--raw')
+      end
+
+      def git_cat_file_cmd
+        format = '%(objectname) %(objectsize) %(rest)'
+        build_git_cmd('cat-file', "--batch-check=#{format}")
+      end
+
+      def format_git_cat_file_script
+        File.expand_path('../support/format-git-cat-file-input', __FILE__)
+      end
     end
   end
 end

lib/gitlab/git/support/format-git-cat-file-input

+#!/usr/bin/env ruby
+
+# This script formats the output of the `git diff <old_rev> <new_rev> --raw`
+# command so it can be processed by `git cat-file`
+
+# We need to convert this:
+# ":100644 100644 5f53439... 85bc2f9... R060\tfiles/js/commit.js.coffee\tfiles/js/commit.coffee"
+# To:
+# "85bc2f9 R\tfiles/js/commit.js.coffee\tfiles/js/commit.coffee"
+
+ARGF.each do |line|
+  _, _, old_blob_id, new_blob_id, rest = line.split(/\s/, 5)
+
+  old_blob_id.gsub!(/[^\h]/, '')
+  new_blob_id.gsub!(/[^\h]/, '')
+
+  # We can't pass '0000000...' to `git cat-file` given it will not return info about the deleted file
+  blob_id = new_blob_id =~ /\A0+\z/ ? old_blob_id : new_blob_id
+
+  $stdout.puts "#{blob_id} #{rest}"
+end

lib/gitlab/gitaly_client/commit_service.rb

@@ -3,10 +3,6 @@ module Gitlab
     class CommitService
       include Gitlab::EncodingHelper
-      # The ID of empty tree.
-      # See http://stackoverflow.com/a/40884093/1856239 and https://github.com/git/git/blob/3ad8b5bf26362ac67c9020bf8c30eee54a84f56d/cache.h#L1011-L1012
-      EMPTY_TREE_ID = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'.freeze
-
       def initialize(repository)
         @gitaly_repo = repository.gitaly_repository
         @repository = repository
@@ -37,7 +33,7 @@ module Gitlab
       def diff(from, to, options = {})
         from_id = case from
                   when NilClass
-                    EMPTY_TREE_ID
+                    Gitlab::Git::EMPTY_TREE_ID
                   else
                     if from.respond_to?(:oid)
                       # This is meant to match a Rugged::Commit. This should be impossible in
@@ -50,7 +46,7 @@ module Gitlab
         to_id = case to
                 when NilClass
-                  EMPTY_TREE_ID
+                  Gitlab::Git::EMPTY_TREE_ID
                 else
                   if to.respond_to?(:oid)
                     # This is meant to match a Rugged::Commit. This should be impossible in
@@ -352,7 +348,7 @@ module Gitlab
       end
       def diff_from_parent_request_params(commit, options = {})
-        parent_id = commit.parent_ids.first || EMPTY_TREE_ID
+        parent_id = commit.parent_ids.first || Gitlab::Git::EMPTY_TREE_ID
         diff_between_commits_request_params(parent_id, commit.id, options)
       end

lib/gitlab/utils.rb

@@ -73,6 +73,10 @@ module Gitlab
       nil
     end
+    def bytes_to_megabytes(bytes)
+      bytes.to_f / Numeric::MEGABYTE
+    end
+
     # Used in EE
     # Accepts either an Array or a String and returns an array
     def ensure_array_from_string(string_or_array)

spec/lib/gitlab/git/raw_diff_change_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Git::RawDiffChange do
+  let(:raw_change) { }
+  let(:change) { described_class.new(raw_change) }
+
+  context 'bad input' do
+    let(:raw_change) { 'foo' }
+
+    it 'does not set most of the attrs' do
+      expect(change.blob_id).to eq('foo')
+      expect(change.operation).to eq(:unknown)
+      expect(change.old_path).to be_blank
+      expect(change.new_path).to be_blank
+      expect(change.blob_size).to be_blank
+    end
+  end
+
+  context 'adding a file' do
+    let(:raw_change) { '93e123ac8a3e6a0b600953d7598af629dec7b735 59 A  bar/branch-test.txt' }
+
+    it 'initialize the proper attrs' do
+      expect(change.operation).to eq(:added)
+      expect(change.old_path).to be_blank
+      expect(change.new_path).to eq('bar/branch-test.txt')
+      expect(change.blob_id).to be_present
+      expect(change.blob_size).to be_present
+    end
+  end
+
+  context 'renaming a file' do
+    let(:raw_change) { "85bc2f9753afd5f4fc5d7c75f74f8d526f26b4f3 107 R060\tfiles/js/commit.js.coffee\tfiles/js/commit.coffee" }
+
+    it 'initialize the proper attrs' do
+      expect(change.operation).to eq(:renamed)
+      expect(change.old_path).to eq('files/js/commit.js.coffee')
+      expect(change.new_path).to eq('files/js/commit.coffee')
+      expect(change.blob_id).to be_present
+      expect(change.blob_size).to be_present
+    end
+  end
+
+  context 'modifying a file' do
+    let(:raw_change) { 'c60514b6d3d6bf4bec1030f70026e34dfbd69ad5 824 M  README.md' }
+
+    it 'initialize the proper attrs' do
+      expect(change.operation).to eq(:modified)
+      expect(change.old_path).to eq('README.md')
+      expect(change.new_path).to eq('README.md')
+      expect(change.blob_id).to be_present
+      expect(change.blob_size).to be_present
+    end
+  end
+
+  context 'deleting a file' do
+    let(:raw_change) { '60d7a906c2fd9e4509aeb1187b98d0ea7ce827c9 15364 D  files/.DS_Store' }
+
+    it 'initialize the proper attrs' do
+      expect(change.operation).to eq(:deleted)
+      expect(change.old_path).to eq('files/.DS_Store')
+      expect(change.new_path).to be_nil
+      expect(change.blob_id).to be_present
+      expect(change.blob_size).to be_present
+    end
+  end
+end

spec/lib/gitlab/git/repository_spec.rb

@@ -1043,6 +1043,44 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.to eq(17) }
   end
+  describe '#raw_changes_between' do
+    let(:old_rev) { }
+    let(:new_rev) { }
+    let(:changes) { repository.raw_changes_between(old_rev, new_rev) }
+
+    context 'initial commit' do
+      let(:old_rev) { Gitlab::Git::BLANK_SHA }
+      let(:new_rev) { '1a0b36b3cdad1d2ee32457c102a8c0b7056fa863' }
+
+      it 'returns the changes' do
+        expect(changes).to be_present
+        expect(changes.size).to eq(3)
+      end
+    end
+
+    context 'with an invalid rev' do
+      let(:old_rev) { 'foo' }
+      let(:new_rev) { 'bar' }
+
+      it 'returns an error' do
+        expect { changes }.to raise_error(Gitlab::Git::Repository::GitError)
+      end
+    end
+
+    context 'with valid revs' do
+      let(:old_rev) { 'fa1b1e6c004a68b7d8763b86455da9e6b23e36d6' }
+      let(:new_rev) { '4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6' }
+
+      it 'returns the changes' do
+        expect(changes.size).to eq(9)
+        expect(changes.first.operation).to eq(:modified)
+        expect(changes.first.new_path).to eq('.gitmodules')
+        expect(changes.last.operation).to eq(:added)
+        expect(changes.last.new_path).to eq('files/lfs/picture-invalid.png')
+      end
+    end
+  end
+
   describe '#merge_base' do
     shared_examples '#merge_base' do
       where(:from, :to, :result) do

spec/lib/gitlab/gitaly_client/commit_service_spec.rb

@@ -33,7 +33,7 @@ describe Gitlab::GitalyClient::CommitService do
         initial_commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863').raw
         request        = Gitaly::CommitDiffRequest.new(
           repository: repository_message,
-          left_commit_id: '4b825dc642cb6eb9a060e54bf8d69288fbee4904',
+          left_commit_id: Gitlab::Git::EMPTY_TREE_ID,
           right_commit_id: initial_commit.id,
           collapse_diffs: true,
           enforce_limits: true,
@@ -77,7 +77,7 @@ describe Gitlab::GitalyClient::CommitService do
         initial_commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863')
         request        = Gitaly::CommitDeltaRequest.new(
           repository: repository_message,
-          left_commit_id: '4b825dc642cb6eb9a060e54bf8d69288fbee4904',
+          left_commit_id: Gitlab::Git::EMPTY_TREE_ID,
           right_commit_id: initial_commit.id
         )
@@ -90,7 +90,7 @@ describe Gitlab::GitalyClient::CommitService do
   describe '#between' do
     let(:from) { 'master' }
-    let(:to) { '4b825dc642cb6eb9a060e54bf8d69288fbee4904' }
+    let(:to) { Gitlab::Git::EMPTY_TREE_ID }
     it 'sends an RPC request' do
       request = Gitaly::CommitsBetweenRequest.new(
@@ -155,7 +155,7 @@ describe Gitlab::GitalyClient::CommitService do
   end
   describe '#find_commit' do
-    let(:revision) { '4b825dc642cb6eb9a060e54bf8d69288fbee4904' }
+    let(:revision) { Gitlab::Git::EMPTY_TREE_ID }
     it 'sends an RPC request' do
       request = Gitaly::FindCommitRequest.new(
         repository: repository_message, revision: revision

spec/lib/gitlab/utils_spec.rb

 require 'spec_helper'
 describe Gitlab::Utils do
-  delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, :ensure_array_from_string, to: :described_class
+  delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, :ensure_array_from_string,
+   :bytes_to_megabytes, to: :described_class
   describe '.slugify' do
     {
@@ -97,4 +98,12 @@ describe Gitlab::Utils do
       expect(ensure_array_from_string(str)).to eq(%w[seven eight 9 10])
     end
   end
+
+  describe '.bytes_to_megabytes' do
+    it 'converts bytes to megabytes' do
+      bytes = 1.megabyte
+
+      expect(bytes_to_megabytes(bytes)).to eq(1)
+    end
+  end
 end

spec/support/bare_repo_operations.rb

 require 'zlib'
 class BareRepoOperations
-  # The ID of empty tree.
-  # See http://stackoverflow.com/a/40884093/1856239 and https://github.com/git/git/blob/3ad8b5bf26362ac67c9020bf8c30eee54a84f56d/cache.h#L1011-L1012
-  EMPTY_TREE_ID = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'.freeze
-
   include Gitlab::Popen
   def initialize(path_to_repo)
     @path_to_repo = path_to_repo
   end
-  def commit_tree(tree_id, msg, parent: EMPTY_TREE_ID)
+  def commit_tree(tree_id, msg, parent: Gitlab::Git::EMPTY_TREE_ID)
     commit_tree_args = ['commit-tree', tree_id, '-m', msg]
-    commit_tree_args += ['-p', parent] unless parent == EMPTY_TREE_ID
+    commit_tree_args += ['-p', parent] unless parent == Gitlab::Git::EMPTY_TREE_ID
     commit_id = execute(commit_tree_args)
     commit_id[0]
@@ -21,7 +17,7 @@ class BareRepoOperations
   # Based on https://stackoverflow.com/a/25556917/1856239
   def commit_file(file, dst_path, branch = 'master')
-    head_id = execute(['show', '--format=format:%H', '--no-patch', branch], allow_failure: true)[0] || EMPTY_TREE_ID
+    head_id = execute(['show', '--format=format:%H', '--no-patch', branch], allow_failure: true)[0] || Gitlab::Git::EMPTY_TREE_ID
     execute(['read-tree', '--empty'])
     execute(['read-tree', head_id])