From 29bb77e3c5b39706df09cc9906548598fe12d998 Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin <viakliushin@gitlab.com> Date: Tue, 13 Feb 2024 13:58:40 +0100 Subject: [PATCH] Remove unused code Originally, `DiffTree` was introduced to handle diffs for the initial commit. Gitaly didn't return a diff of the original commit when it was provided to FindChangedPaths RPC. To solve that I implemented a fix: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123950. It allows to provide a `DiffTree` instance to the RPC and use a tree interface to fetch changes even for the initial commit. Later, a better option was discovered: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143216. As a result, some old code is not needed anymore. Changelog: other --- lib/gitlab/git/diff_tree.rb | 14 ------------- spec/lib/gitlab/git/diff_tree_spec.rb | 28 +++++++++----------------- spec/lib/gitlab/git/repository_spec.rb | 7 +++---- 3 files changed, 13 insertions(+), 36 deletions(-) diff --git a/lib/gitlab/git/diff_tree.rb b/lib/gitlab/git/diff_tree.rb index a8f01078be77..34a32b65b00d 100644 --- a/lib/gitlab/git/diff_tree.rb +++ b/lib/gitlab/git/diff_tree.rb @@ -11,20 +11,6 @@ def initialize(left_tree_id, right_tree_id) @left_tree_id = left_tree_id @right_tree_id = right_tree_id end - - def self.from_commit(commit) - return unless commit.tree_id - - parent_tree_id = - if commit.parent_ids.blank? - Gitlab::Git::SHA1_EMPTY_TREE_ID - else - parent_id = commit.parent_ids.first - commit.repository.commit(parent_id).tree_id - end - - new(parent_tree_id, commit.tree_id) - end end end end diff --git a/spec/lib/gitlab/git/diff_tree_spec.rb b/spec/lib/gitlab/git/diff_tree_spec.rb index e82978bd4889..3b3297650bb6 100644 --- a/spec/lib/gitlab/git/diff_tree_spec.rb +++ b/spec/lib/gitlab/git/diff_tree_spec.rb @@ -3,28 +3,20 @@ require "spec_helper" RSpec.describe Gitlab::Git::DiffTree, feature_category: :source_code_management do - let_it_be(:project) { create(:project, :repository) } - let_it_be(:repository) { project.repository } + subject(:diff_tree) { described_class.new(left_tree_id, right_tree_id) } - describe '.from_commit' do - subject(:diff_tree) { described_class.from_commit(commit) } + let(:left_tree_id) { '1a0b36b3cdad1d2ee32457c102a8c0b7056fa863' } + let(:right_tree_id) { '60ecb67744cb56576c30214ff52294f8ce2def98' } - context 'when commit is an initial commit' do - let(:commit) { repository.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') } + describe '#left_tree_id' do + subject { diff_tree.left_tree_id } - it 'returns the expected diff tree object' do - expect(diff_tree.left_tree_id).to eq(Gitlab::Git::SHA1_EMPTY_TREE_ID) - expect(diff_tree.right_tree_id).to eq(commit.tree_id) - end - end + it { is_expected.to eq(left_tree_id) } + end - context 'when commit is a regular commit' do - let(:commit) { repository.commit('60ecb67744cb56576c30214ff52294f8ce2def98') } + describe '#right_tree_id' do + subject { diff_tree.right_tree_id } - it 'returns the expected diff tree object' do - expect(diff_tree.left_tree_id).to eq(commit.parent.tree_id) - expect(diff_tree.right_tree_id).to eq(commit.tree_id) - end - end + it { is_expected.to eq(right_tree_id) } end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 959074a79bf1..ddcdf2d559a3 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1706,7 +1706,6 @@ def create_commit(blobs) let_it_be(:commit_3) { repository.commit('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } let_it_be(:initial_commit) { repository.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') } - let_it_be(:diff_tree) { Gitlab::Git::DiffTree.from_commit(initial_commit) } let(:commit_1_files) do [ @@ -1739,7 +1738,7 @@ def create_commit(blobs) ] end - let(:diff_tree_files) do + let(:initial_commit_files) do [ Gitlab::Git::ChangedPath.new( status: :ADDED, path: ".gitignore", old_mode: "0", new_mode: "100644", @@ -1757,10 +1756,10 @@ def create_commit(blobs) end it 'returns a list of paths' do - collection = repository.find_changed_paths([commit_1, commit_2, commit_3, diff_tree]) + collection = repository.find_changed_paths([commit_1, commit_2, commit_3, initial_commit]) expect(collection).to be_a(Enumerable) - expect(collection.as_json).to eq((commit_1_files + commit_2_files + commit_3_files + diff_tree_files).as_json) + expect(collection.as_json).to eq((commit_1_files + commit_2_files + commit_3_files + initial_commit_files).as_json) end it 'returns only paths with valid SHAs' do -- GitLab