diff options
author | Jacob Vosmaer (GitLab) <jacob@gitlab.com> | 2018-07-18 11:12:57 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-07-18 11:12:57 +0000 |
commit | 5c651daa97b57478f39f252adb7562c31b27ebe7 (patch) | |
tree | cf71488a639749fc9db07fa4026e880941958f9b | |
parent | af6551f33b45a0063e20ddfa7f711842879b3a75 (diff) | |
download | gitlab-ce-5c651daa97b57478f39f252adb7562c31b27ebe7.tar.gz |
Remove unreachable Git code
-rw-r--r-- | lib/gitlab/git/commit.rb | 64 | ||||
-rw-r--r-- | lib/gitlab/git/repository.rb | 89 | ||||
-rw-r--r-- | lib/gitlab/git/repository_mirroring.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb | 22 | ||||
-rw-r--r-- | spec/lib/gitlab/git/commit_spec.rb | 18 | ||||
-rw-r--r-- | spec/lib/gitlab/git/repository_spec.rb | 45 | ||||
-rw-r--r-- | spec/migrations/migrate_process_commit_worker_jobs_spec.rb | 5 | ||||
-rw-r--r-- | spec/support/helpers/cycle_analytics_helpers.rb | 3 |
8 files changed, 38 insertions, 210 deletions
diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 4e2d817d12c..5b264868af0 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -1,4 +1,4 @@ -# 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 diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 2cbd9c218d4..19c79b6f7b7 100644 --- a/lib/gitlab/git/repository.rb +++ b/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 diff --git a/lib/gitlab/git/repository_mirroring.rb b/lib/gitlab/git/repository_mirroring.rb index e35ea5762eb..9faa62be28e 100644 --- a/lib/gitlab/git/repository_mirroring.rb +++ b/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 diff --git a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb index 211e3aaa94b..0735ebd6dcb 100644 --- a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index ee74c2769eb..0adb684765d 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 6480f6c407d..0365c3b20ef 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/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, '') diff --git a/spec/migrations/migrate_process_commit_worker_jobs_spec.rb b/spec/migrations/migrate_process_commit_worker_jobs_spec.rb index ac34efa4f9d..a30e6c23ac9 100644 --- a/spec/migrations/migrate_process_commit_worker_jobs_spec.rb +++ b/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 diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb index 32d9807f06a..c228bd2393b 100644 --- a/spec/support/helpers/cycle_analytics_helpers.rb +++ b/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( |