Merge branch 'gitaly-remove-worktree' into 'master'

Remove unreachable Git code

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

lib/gitlab/git/commit.rb

-# Gitlab::Git::Commit is a wrapper around native Rugged::Commit object
+# Gitlab::Git::Commit is a wrapper around Gitaly::GitCommit
 module Gitlab
   module Git
     class Commit
@@ -55,7 +55,6 @@ module Gitlab
 
           # A rugged reference?
           commit_id = Gitlab::Git::Ref.dereference_object(commit_id)
-          return decorate(repo, commit_id) if commit_id.is_a?(Rugged::Commit)
 
           # Some weird thing?
           return nil unless commit_id.is_a?(String)
@@ -68,9 +67,7 @@ module Gitlab
           end
 
           decorate(repo, commit) if commit
-        rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError,
-               Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository,
-               Rugged::OdbError, Rugged::TreeError, ArgumentError
+        rescue Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository, ArgumentError
           nil
         end
 
@@ -142,20 +139,6 @@ module Gitlab
           Gitlab::Git::Commit.new(repository, commit, ref)
         end
 
-        # Returns the `Rugged` sorting type constant for one or more given
-        # sort types. Valid keys are `:none`, `:topo`, and `:date`, or an array
-        # containing more than one of them. `:date` uses a combination of date and
-        # topological sorting to closer mimic git's native ordering.
-        def rugged_sort_type(sort_type)
-          @rugged_sort_types ||= {
-            none: Rugged::SORT_NONE,
-            topo: Rugged::SORT_TOPO,
-            date: Rugged::SORT_DATE | Rugged::SORT_TOPO
-          }
-
-          @rugged_sort_types.fetch(sort_type, Rugged::SORT_NONE)
-        end
-
         def shas_with_signatures(repository, shas)
           Gitlab::GitalyClient::CommitService.new(repository).filter_shas_with_signatures(shas)
         end
@@ -223,8 +206,6 @@ module Gitlab
         case raw_commit
         when Hash
           init_from_hash(raw_commit)
-        when Rugged::Commit
-          init_from_rugged(raw_commit)
         when Gitaly::GitCommit
           init_from_gitaly(raw_commit)
         else
@@ -265,23 +246,6 @@ module Gitlab
         @repository.gitaly_commit_client.diff_from_parent(self, options)
       end
 
-      # Not to be called directly, but right now its used for tests and in old
-      # migrations
-      def rugged_diff_from_parent(options = {})
-        options ||= {}
-        break_rewrites = options[:break_rewrites]
-        actual_options = Gitlab::Git::Diff.filter_diff_options(options)
-
-        diff = if rugged_commit.parents.empty?
-                 rugged_commit.diff(actual_options.merge(reverse: true))
-               else
-                 rugged_commit.parents[0].diff(rugged_commit, actual_options)
-               end
-
-        diff.find_similar!(break_rewrites: break_rewrites)
-        diff
-      end
-
       def deltas
         @deltas ||= begin
           deltas = @repository.gitaly_commit_client.commit_deltas(self)
@@ -352,14 +316,6 @@ module Gitlab
         encode! @committer_email
       end
 
-      def rugged_commit
-        @rugged_commit ||= if raw_commit.is_a?(Rugged::Commit)
-                             raw_commit
-                           else
-                             @repository.rev_parse_target(id)
-                           end
-      end
-
       def merge_commit?
         parent_ids.size > 1
       end
@@ -405,22 +361,6 @@ module Gitlab
         end
       end
 
-      def init_from_rugged(commit)
-        author = commit.author
-        committer = commit.committer
-
-        @raw_commit = commit
-        @id = commit.oid
-        @message = commit.message
-        @authored_date = author[:time]
-        @committed_date = committer[:time]
-        @author_name = author[:name]
-        @author_email = author[:email]
-        @committer_name = committer[:name]
-        @committer_email = committer[:email]
-        @parent_ids = commit.parents.map(&:oid)
-      end
-
       def init_from_gitaly(commit)
         @raw_commit = commit
         @id = commit.id

lib/gitlab/git/repository.rb

@@ -252,14 +252,6 @@ module Gitlab
         end
       end
 
-      def batch_existence(object_ids, existing: true)
-        filter_method = existing ? :select : :reject
-
-        object_ids.public_send(filter_method) do |oid| # rubocop:disable GitlabSecurity/PublicSend
-          rugged.exists?(oid)
-        end
-      end
-
       # Returns an Array of branch and tag names
       def ref_names
         branch_names + tag_names
@@ -409,13 +401,6 @@ module Gitlab
         end
       end
 
-      # Return the object that +revspec+ points to.  If +revspec+ is an
-      # annotated tag, then return the tag's target instead.
-      def rev_parse_target(revspec)
-        obj = rugged.rev_parse(revspec)
-        Ref.dereference_object(obj)
-      end
-
       # Counts the amount of commits between `from` and `to`.
       def count_commits_between(from, to, options = {})
         count_commits(from: from, to: to, **options)
@@ -1132,33 +1117,6 @@ module Gitlab
         run_git!(args, lazy_block: block)
       end
 
-      def with_worktree(worktree_path, branch, sparse_checkout_files: nil, env:)
-        base_args = %w(worktree add --detach)
-
-        # Note that we _don't_ want to test for `.present?` here: If the caller
-        # passes an non nil empty value it means it still wants sparse checkout
-        # but just isn't interested in any file, perhaps because it wants to
-        # checkout files in by a changeset but that changeset only adds files.
-        if sparse_checkout_files
-          # Create worktree without checking out
-          run_git!(base_args + ['--no-checkout', worktree_path], env: env)
-          worktree_git_path = run_git!(%w(rev-parse --git-dir), chdir: worktree_path).chomp
-
-          configure_sparse_checkout(worktree_git_path, sparse_checkout_files)
-
-          # After sparse checkout configuration, checkout `branch` in worktree
-          run_git!(%W(checkout --detach #{branch}), chdir: worktree_path, env: env)
-        else
-          # Create worktree and checkout `branch` in it
-          run_git!(base_args + [worktree_path, branch], env: env)
-        end
-
-        yield
-      ensure
-        FileUtils.rm_rf(worktree_path) if File.exist?(worktree_path)
-        FileUtils.rm_rf(worktree_git_path) if worktree_git_path && File.exist?(worktree_git_path)
-      end
-
       def checksum
         # The exists? RPC is much cheaper, so we perform this request first
         raise NoRepository, "Repository does not exists" unless exists?
@@ -1204,38 +1162,6 @@ module Gitlab
         end
       end
 
-      # Adding a worktree means checking out the repository. For large repos,
-      # this can be very expensive, so set up sparse checkout for the worktree
-      # to only check out the files we're interested in.
-      def configure_sparse_checkout(worktree_git_path, files)
-        run_git!(%w(config core.sparseCheckout true))
-
-        return if files.empty?
-
-        worktree_info_path = File.join(worktree_git_path, 'info')
-        FileUtils.mkdir_p(worktree_info_path)
-        File.write(File.join(worktree_info_path, 'sparse-checkout'), files)
-      end
-
-      def rugged_fetch_source_branch(source_repository, source_branch, local_ref)
-        with_repo_branch_commit(source_repository, source_branch) do |commit|
-          if commit
-            write_ref(local_ref, commit.sha)
-            true
-          else
-            false
-          end
-        end
-      end
-
-      def worktree_path(prefix, id)
-        id = id.to_s
-        raise ArgumentError, "worktree id can't be empty" unless id.present?
-        raise ArgumentError, "worktree id can't contain slashes " if id.include?("/")
-
-        File.join(path, 'gitlab-worktree', "#{prefix}-#{id}")
-      end
-
       def git_env_for_user(user)
         {
           'GIT_COMMITTER_NAME' => user.name,
@@ -1296,17 +1222,6 @@ module Gitlab
         Gitlab::Git::HookEnv.all(gl_repository).values_at(*ALLOWED_OBJECT_RELATIVE_DIRECTORIES_VARIABLES).flatten.compact
       end
 
-      # Return the Rugged patches for the diff between +from+ and +to+.
-      def diff_patches(from, to, options = {}, *paths)
-        options ||= {}
-        break_rewrites = options[:break_rewrites]
-        actual_options = Gitlab::Git::Diff.filter_diff_options(options.merge(paths: paths))
-
-        diff = rugged.diff(from, to, actual_options)
-        diff.find_similar!(break_rewrites: break_rewrites)
-        diff.each_patch
-      end
-
       def sort_branches(branches, sort_by)
         case sort_by
         when 'name'
@@ -1394,10 +1309,6 @@ module Gitlab
       def rev_list_param(spec)
         spec == :all ? ['--all'] : spec
       end
-
-      def sha_from_ref(ref)
-        rev_parse_target(ref).oid
-      end
     end
   end
 end

lib/gitlab/git/repository_mirroring.rb

@@ -50,7 +50,7 @@ module Gitlab
           name = ref.name.sub(%r{\Arefs/remotes/#{remote_name}/}, '')
 
           begin
-            target_commit = Gitlab::Git::Commit.find(self, ref.target)
+            target_commit = Gitlab::Git::Commit.find(self, ref.target.oid)
             branches << Gitlab::Git::Branch.new(self, name, ref.target, target_commit)
           rescue Rugged::ReferenceError
             # Omit invalid branch

spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb

@@ -9,6 +9,11 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :m
     let(:merge_request) { merge_requests.create!(iid: 1, target_project_id: project.id, source_project_id: project.id, target_branch: 'feature', source_branch: 'master').becomes(MergeRequest) }
     let(:merge_request_diff) { MergeRequest.find(merge_request.id).create_merge_request_diff }
     let(:updated_merge_request_diff) { MergeRequestDiff.find(merge_request_diff.id) }
+    let(:rugged) do
+      Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+        project.repository.rugged
+      end
+    end
 
     before do
       allow_any_instance_of(MergeRequestDiff)
@@ -299,11 +304,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :m
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
       let(:first_commit) { project.repository.commit(merge_request_diff.head_commit_sha) }
       let(:expected_commits) { commits }
-      let(:diffs) do
-        Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-          first_commit.rugged_diff_from_parent.patches
-        end
-      end
+      let(:diffs) { rugged_diff(first_commit.sha).patches }
       let(:expected_diffs) { [] }
 
       include_examples 'updated MR diff'
@@ -313,14 +314,15 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :m
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
       let(:first_commit) { project.repository.commit(merge_request_diff.head_commit_sha) }
       let(:expected_commits) { commits }
-      let(:diffs) do
-        Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-          first_commit.rugged_diff_from_parent.deltas
-        end
-      end
+      let(:diffs) { rugged_diff(first_commit.sha).deltas }
       let(:expected_diffs) { [] }
 
       include_examples 'updated MR diff'
     end
+
+    def rugged_diff(commit_sha)
+      rugged_commit = rugged.lookup(commit_sha)
+      rugged_commit.parents[0].diff(rugged_commit)
+    end
   end
 end

spec/lib/gitlab/git/commit_spec.rb

@@ -27,7 +27,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
       }
 
       @parents = [repo.head.target]
-      @gitlab_parents = @parents.map { |c| described_class.decorate(repository, c) }
+      @gitlab_parents = @parents.map { |c| described_class.find(repository, c.oid) }
       @tree = @parents.first.tree
 
       sha = Rugged::Commit.create(
@@ -41,7 +41,7 @@ describe Gitlab::Git::Commit, seed_helper: true do
       )
 
       @raw_commit = repo.lookup(sha)
-      @commit = described_class.new(repository, @raw_commit)
+      @commit = described_class.find(repository, sha)
     end
 
     it { expect(@commit.short_id).to eq(@raw_commit.oid[0..10]) }
@@ -488,13 +488,15 @@ describe Gitlab::Git::Commit, seed_helper: true do
     end
   end
 
-  describe '#init_from_rugged' do
-    let(:gitlab_commit) { described_class.new(repository, rugged_commit) }
-    subject { gitlab_commit }
+  skip 'move this test to gitaly-ruby' do
+    describe '#init_from_rugged' do
+      let(:gitlab_commit) { described_class.new(repository, rugged_commit) }
+      subject { gitlab_commit }
 
-    describe '#id' do
-      subject { super().id }
-      it { is_expected.to eq(SeedRepo::Commit::ID) }
+      describe '#id' do
+        subject { super().id }
+        it { is_expected.to eq(SeedRepo::Commit::ID) }
+      end
     end
   end
 

spec/lib/gitlab/git/repository_spec.rb

@@ -639,21 +639,21 @@ describe Gitlab::Git::Repository, seed_helper: true do
   describe "#log" do
     shared_examples 'repository log' do
       let(:commit_with_old_name) do
-        Gitlab::Git::Commit.decorate(repository, @commit_with_old_name_id)
+        Gitlab::Git::Commit.find(repository, @commit_with_old_name_id)
       end
       let(:commit_with_new_name) do
-        Gitlab::Git::Commit.decorate(repository, @commit_with_new_name_id)
+        Gitlab::Git::Commit.find(repository, @commit_with_new_name_id)
       end
       let(:rename_commit) do
-        Gitlab::Git::Commit.decorate(repository, @rename_commit_id)
+        Gitlab::Git::Commit.find(repository, @rename_commit_id)
       end
 
       before(:context) do
         # Add new commits so that there's a renamed file in the commit history
         repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged
-        @commit_with_old_name_id = new_commit_edit_old_file(repo)
-        @rename_commit_id = new_commit_move_file(repo)
-        @commit_with_new_name_id = new_commit_edit_new_file(repo)
+        @commit_with_old_name_id = new_commit_edit_old_file(repo).oid
+        @rename_commit_id = new_commit_move_file(repo).oid
+        @commit_with_new_name_id = new_commit_edit_new_file(repo).oid
       end
 
       after(:context) do
@@ -855,8 +855,8 @@ describe Gitlab::Git::Repository, seed_helper: true do
 
         def commit_files(commit)
           Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-            commit.rugged_diff_from_parent.deltas.flat_map do |delta|
-              [delta.old_file[:path], delta.new_file[:path]].uniq.compact
+            commit.deltas.flat_map do |delta|
+              [delta.old_path, delta.new_path].uniq.compact
             end
           end
         end
@@ -893,10 +893,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
     context 'when Gitaly find_commits feature is enabled' do
       it_behaves_like 'repository log'
     end
-
-    context 'when Gitaly find_commits feature is disabled', :disable_gitaly do
-      it_behaves_like 'repository log'
-    end
   end
 
   describe '#count_commits_between' do
@@ -1441,31 +1437,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe '#batch_existence' do
-    let(:refs) { ['deadbeef', SeedRepo::RubyBlob::ID, '909e6157199'] }
-
-    around do |example|
-      # TODO #batch_existence isn't used anywhere, can we remove it?
-      Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-        example.run
-      end
-    end
-
-    it 'returns existing refs back' do
-      result = repository.batch_existence(refs)
-
-      expect(result).to eq([SeedRepo::RubyBlob::ID])
-    end
-
-    context 'existing: true' do
-      it 'inverts meaning and returns non-existing refs' do
-        result = repository.batch_existence(refs, existing: false)
-
-        expect(result).to eq(%w(deadbeef 909e6157199))
-      end
-    end
-  end
-
   describe '#local_branches' do
     before(:all) do
       @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')

spec/migrations/migrate_process_commit_worker_jobs_spec.rb

@@ -6,11 +6,12 @@ require Rails.root.join('db', 'migrate', '20161124141322_migrate_process_commit_
 describe MigrateProcessCommitWorkerJobs do
   let(:project) { create(:project, :legacy_storage, :repository) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
   let(:user) { create(:user) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
-  let(:commit) do
+  let(:rugged) do
     Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-      project.commit.raw.rugged_commit
+      project.repository.rugged
     end
   end
+  let(:commit) { rugged.rev_parse(project.commit.id) }
 
   describe 'Project' do
     describe 'find_including_path' do

spec/support/helpers/cycle_analytics_helpers.rb

@@ -125,7 +125,8 @@ module CycleAnalyticsHelpers
         _, opts = args
 
         commit = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-          raw_repository.commit(branch_update.newrev).rugged_commit
+          rugged = raw_repository.rugged
+          rugged.rev_parse(branch_update.newrev)
         end
 
         branch_update.newrev = commit.amend(