Remove Repository#lookup and unreachable rugged code

lib/gitlab/git/repository.rb

@@ -280,12 +280,6 @@ module Gitlab
         end.map(&:name)
       end
 
-      def rugged_head
-        rugged.head
-      rescue Rugged::ReferenceError
-        nil
-      end
-
       def archive_metadata(ref, storage_path, project_path, format = "tar.gz", append_sha:)
         ref ||= root_ref
         commit = Gitlab::Git::Commit.find(self, ref)
@@ -515,11 +509,6 @@ module Gitlab
         @refs_hash
       end
 
-      # Lookup for rugged object by oid or ref name
-      def lookup(oid_or_ref_name)
-        rugged.rev_parse(oid_or_ref_name)
-      end
-
       # Returns url for submodule
       #
       # Ex.
@@ -916,20 +905,6 @@ module Gitlab
         Gitlab::Git::Blob.batch(self, items, blob_size_limit: blob_size_limit)
       end
 
-      def commit_index(user, branch_name, index, options)
-        committer = user_to_committer(user)
-
-        OperationService.new(user, self).with_branch(branch_name) do
-          commit_params = options.merge(
-            tree: index.write_tree(rugged),
-            author: committer,
-            committer: committer
-          )
-
-          create_commit(commit_params)
-        end
-      end
-
       def fsck
         msg, status = gitaly_repository_client.fsck
 
@@ -1356,23 +1331,6 @@ module Gitlab
         end
       end
 
-      # We are trying to deprecate this method because it does a lot of work
-      # but it seems to be used only to look up submodule URL's.
-      # https://gitlab.com/gitlab-org/gitaly/issues/329
-      def submodules(ref)
-        commit = rev_parse_target(ref)
-        return {} unless commit
-
-        begin
-          content = blob_content(commit, ".gitmodules")
-        rescue InvalidBlobName
-          return {}
-        end
-
-        parser = GitmodulesParser.new(content)
-        fill_submodule_ids(commit, parser.parse)
-      end
-
       def gitaly_submodule_url_for(ref, path)
         # We don't care about the contents so 1 byte is enough. Can't request 0 bytes, 0 means unlimited.
         commit_object = gitaly_commit_client.tree_entry(ref, path, 1)
@@ -1395,68 +1353,6 @@ module Gitlab
         Gitlab::Git::HookEnv.all(gl_repository).values_at(*ALLOWED_OBJECT_RELATIVE_DIRECTORIES_VARIABLES).flatten.compact
       end
 
-      # Get the content of a blob for a given commit.  If the blob is a commit
-      # (for submodules) then return the blob's OID.
-      def blob_content(commit, blob_name)
-        blob_entry = tree_entry(commit, blob_name)
-
-        unless blob_entry
-          raise InvalidBlobName.new("Invalid blob name: #{blob_name}")
-        end
-
-        case blob_entry[:type]
-        when :commit
-          blob_entry[:oid]
-        when :tree
-          raise InvalidBlobName.new("#{blob_name} is a tree, not a blob")
-        when :blob
-          rugged.lookup(blob_entry[:oid]).content
-        end
-      end
-
-      # Fill in the 'id' field of a submodule hash from its values
-      # as-of +commit+. Return a Hash consisting only of entries
-      # from the submodule hash for which the 'id' field is filled.
-      def fill_submodule_ids(commit, submodule_data)
-        submodule_data.each do |path, data|
-          id = begin
-            blob_content(commit, path)
-          rescue InvalidBlobName
-            nil
-          end
-          data['id'] = id
-        end
-        submodule_data.select { |path, data| data['id'] }
-      end
-
-      # Find the entry for +path+ in the tree for +commit+
-      def tree_entry(commit, path)
-        pathname = Pathname.new(path)
-        first = true
-        tmp_entry = nil
-
-        pathname.each_filename do |dir|
-          if first
-            tmp_entry = commit.tree[dir]
-            first = false
-          elsif tmp_entry.nil?
-            return nil
-          else
-            begin
-              tmp_entry = rugged.lookup(tmp_entry[:oid])
-            rescue Rugged::OdbError, Rugged::InvalidError, Rugged::ReferenceError
-              return nil
-            end
-
-            return nil unless tmp_entry.type == :tree
-
-            tmp_entry = tmp_entry[dir]
-          end
-        end
-
-        tmp_entry
-      end
-
       # Return the Rugged patches for the diff between +from+ and +to+.
       def diff_patches(from, to, options = {}, *paths)
         options ||= {}
@@ -1496,75 +1392,6 @@ module Gitlab
         gitaly_repository_client.apply_gitattributes(revision)
       end
 
-      def rugged_copy_gitattributes(ref)
-        begin
-          commit = lookup(ref)
-        rescue Rugged::ReferenceError
-          raise InvalidRef.new("Ref #{ref} is invalid")
-        end
-
-        # Create the paths
-        info_dir_path = File.join(path, 'info')
-        info_attributes_path = File.join(info_dir_path, 'attributes')
-
-        begin
-          # Retrieve the contents of the blob
-          gitattributes_content = blob_content(commit, '.gitattributes')
-        rescue InvalidBlobName
-          # No .gitattributes found. Should now remove any info/attributes and return
-          File.delete(info_attributes_path) if File.exist?(info_attributes_path)
-          return
-        end
-
-        # Create the info directory if needed
-        Dir.mkdir(info_dir_path) unless File.directory?(info_dir_path)
-
-        # Write the contents of the .gitattributes file to info/attributes
-        # Use binary mode to prevent Rails from converting ASCII-8BIT to UTF-8
-        File.open(info_attributes_path, "wb") do |file|
-          file.write(gitattributes_content)
-        end
-      end
-
-      def rugged_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 local_fetch_ref(source_path, source_ref:, target_ref:)
         args = %W(fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref})
         run_git(args)
@@ -1634,12 +1461,6 @@ module Gitlab
       def sha_from_ref(ref)
         rev_parse_target(ref).oid
       end
-
-      def create_commit(params = {})
-        params[:message].delete!("\r")
-
-        Rugged::Commit.create(rugged, params)
-      end
     end
   end
 end

spec/lib/gitlab/git/diff_spec.rb

@@ -269,7 +269,7 @@ EOT
     before do
       # TODO use a Gitaly diff object instead
       rugged_commit = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-        repository.lookup('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
+        repository.rugged.rev_parse('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
       end
 
       @diffs = rugged_commit.parents[0].diff(rugged_commit).patches

spec/lib/gitlab/git/index_spec.rb

@@ -5,7 +5,7 @@ describe Gitlab::Git::Index, seed_helper: true do
   let(:index) { described_class.new(repository) }
 
   before do
-    index.read_tree(repository.lookup('master').tree)
+    index.read_tree(lookup('master').tree)
   end
 
   around do |example|
@@ -30,7 +30,7 @@ describe Gitlab::Git::Index, seed_helper: true do
         entry = index.get(options[:file_path])
 
         expect(entry).not_to be_nil
-        expect(repository.lookup(entry[:oid]).content).to eq(options[:content])
+        expect(lookup(entry[:oid]).content).to eq(options[:content])
       end
     end
 
@@ -54,7 +54,7 @@ describe Gitlab::Git::Index, seed_helper: true do
         index.create(options)
 
         entry = index.get(options[:file_path])
-        expect(repository.lookup(entry[:oid]).content).to eq(Base64.decode64(options[:content]))
+        expect(lookup(entry[:oid]).content).to eq(Base64.decode64(options[:content]))
       end
     end
 
@@ -68,7 +68,7 @@ describe Gitlab::Git::Index, seed_helper: true do
         index.create(options)
 
         entry = index.get(options[:file_path])
-        expect(repository.lookup(entry[:oid]).content).to eq("Hello,\nWorld")
+        expect(lookup(entry[:oid]).content).to eq("Hello,\nWorld")
       end
     end
   end
@@ -135,7 +135,7 @@ describe Gitlab::Git::Index, seed_helper: true do
 
         entry = index.get(options[:file_path])
 
-        expect(repository.lookup(entry[:oid]).content).to eq(options[:content])
+        expect(lookup(entry[:oid]).content).to eq(options[:content])
       end
 
       it 'preserves file mode' do
@@ -190,7 +190,7 @@ describe Gitlab::Git::Index, seed_helper: true do
         entry = index.get(options[:file_path])
 
         expect(entry).not_to be_nil
-        expect(repository.lookup(entry[:oid]).content).to eq(options[:content])
+        expect(lookup(entry[:oid]).content).to eq(options[:content])
       end
 
       it 'preserves file mode' do
@@ -232,4 +232,8 @@ describe Gitlab::Git::Index, seed_helper: true do
       end
     end
   end
+
+  def lookup(revision)
+    repository.rugged.rev_parse(revision)
+  end
 end

spec/lib/gitlab/git/repository_spec.rb

@@ -321,90 +321,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  context '#submodules' do
-    around do |example|
-      # TODO #submodules will be removed, has been migrated to gitaly
-      Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-        example.run
-      end
-    end
-
-    let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') }
-
-    context 'where repo has submodules' do
-      let(:submodules) { repository.send(:submodules, 'master') }
-      let(:submodule) { submodules.first }
-
-      it { expect(submodules).to be_kind_of Hash }
-      it { expect(submodules.empty?).to be_falsey }
-
-      it 'should have valid data' do
-        expect(submodule).to eq([
-          "six", {
-            "id" => "409f37c4f05865e4fb208c771485f211a22c4c2d",
-            "name" => "six",
-            "url" => "git://github.com/randx/six.git"
-          }
-        ])
-      end
-
-      it 'should handle nested submodules correctly' do
-        nested = submodules['nested/six']
-        expect(nested['name']).to eq('nested/six')
-        expect(nested['url']).to eq('git://github.com/randx/six.git')
-        expect(nested['id']).to eq('24fb71c79fcabc63dfd8832b12ee3bf2bf06b196')
-      end
-
-      it 'should handle deeply nested submodules correctly' do
-        nested = submodules['deeper/nested/six']
-        expect(nested['name']).to eq('deeper/nested/six')
-        expect(nested['url']).to eq('git://github.com/randx/six.git')
-        expect(nested['id']).to eq('24fb71c79fcabc63dfd8832b12ee3bf2bf06b196')
-      end
-
-      it 'should not have an entry for an invalid submodule' do
-        expect(submodules).not_to have_key('invalid/path')
-      end
-
-      it 'should not have an entry for an uncommited submodule dir' do
-        submodules = repository.send(:submodules, 'fix-existing-submodule-dir')
-        expect(submodules).not_to have_key('submodule-existing-dir')
-      end
-
-      it 'should handle tags correctly' do
-        submodules = repository.send(:submodules, 'v1.2.1')
-
-        expect(submodules.first).to eq([
-          "six", {
-            "id" => "409f37c4f05865e4fb208c771485f211a22c4c2d",
-            "name" => "six",
-            "url" => "git://github.com/randx/six.git"
-          }
-        ])
-      end
-
-      it 'should not break on invalid syntax' do
-        allow(repository).to receive(:blob_content).and_return(<<-GITMODULES.strip_heredoc)
-          [submodule "six"]
-          path = six
-          url = git://github.com/randx/six.git
-
-          [submodule]
-          foo = bar
-        GITMODULES
-
-        expect(submodules).to have_key('six')
-      end
-    end
-
-    context 'where repo doesn\'t have submodules' do
-      let(:submodules) { repository.send(:submodules, '6d39438') }
-      it 'should return an empty hash' do
-        expect(submodules).to be_empty
-      end
-    end
-  end
-
   describe '#commit_count' do
     shared_examples 'simple commit counting' do
       it { expect(repository.commit_count("master")).to eq(25) }