diff options
-rw-r--r-- | app/models/commit.rb | 17 | ||||
-rw-r--r-- | app/models/merge_request_diff.rb | 4 | ||||
-rw-r--r-- | app/models/repository.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/conflict/file_collection.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/cycle_analytics/plan_event_fetcher.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/git/blame.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/git/commit.rb | 124 | ||||
-rw-r--r-- | lib/gitlab/git/commit_stats.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/git/diff_collection.rb | 3 | ||||
-rw-r--r-- | lib/gitlab/git/repository.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/commit.rb | 14 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/commit_service.rb | 32 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/ref_service.rb | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/git/commit_spec.rb | 57 | ||||
-rw-r--r-- | spec/lib/gitlab/git/repository_spec.rb | 21 | ||||
-rw-r--r-- | spec/lib/gitlab/gitaly_client/commit_service_spec.rb | 28 | ||||
-rw-r--r-- | spec/migrations/migrate_process_commit_worker_jobs_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/commit_spec.rb | 4 |
20 files changed, 180 insertions, 158 deletions
diff --git a/app/models/commit.rb b/app/models/commit.rb index 7940733f557..638fddc5d3d 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -55,7 +55,8 @@ class Commit end def from_hash(hash, project) - new(Gitlab::Git::Commit.new(hash), project) + raw_commit = Gitlab::Git::Commit.new(project.repository.raw, hash) + new(raw_commit, project) end def valid_hash?(key) @@ -320,21 +321,11 @@ class Commit end def raw_diffs(*args) - if Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs) - Gitlab::GitalyClient::CommitService.new(project.repository).diff_from_parent(self, *args) - else - raw.diffs(*args) - end + raw.diffs(*args) end def raw_deltas - @deltas ||= Gitlab::GitalyClient.migrate(:commit_deltas) do |is_enabled| - if is_enabled - Gitlab::GitalyClient::CommitService.new(project.repository).commit_deltas(self) - else - raw.deltas - end - end + @deltas ||= raw.deltas end def diffs(diff_options = nil) diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index d9d746ccf41..58050e1f438 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -282,9 +282,7 @@ class MergeRequestDiff < ActiveRecord::Base def load_commits commits = st_commits.presence || merge_request_diff_commits - commits.map do |commit| - Commit.new(Gitlab::Git::Commit.new(commit.to_hash), merge_request.source_project) - end + commits.map { |commit| Commit.from_hash(commit.to_hash, project) } end def save_diffs diff --git a/app/models/repository.rb b/app/models/repository.rb index 3b5d0e00c70..ff82b958255 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -64,6 +64,8 @@ class Repository @raw_repository ||= initialize_raw_repository end + alias_method :raw, :raw_repository + # Return absolute path to repository def path_to_repo @path_to_repo ||= File.expand_path( @@ -763,7 +765,7 @@ class Repository index = Gitlab::Git::Index.new(raw_repository) if start_commit - index.read_tree(start_commit.raw_commit.tree) + index.read_tree(start_commit.rugged_commit.tree) parents = [start_commit.sha] else parents = [] diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index 1611eba31da..d671867e7c7 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -77,8 +77,8 @@ EOM def initialize(merge_request, project) @merge_request = merge_request - @our_commit = merge_request.source_branch_head.raw.raw_commit - @their_commit = merge_request.target_branch_head.raw.raw_commit + @our_commit = merge_request.source_branch_head.raw.rugged_commit + @their_commit = merge_request.target_branch_head.raw.rugged_commit @project = project end end diff --git a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb index b260822788d..2479b4a7706 100644 --- a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb @@ -54,7 +54,7 @@ module Gitlab end def serialize_commit(event, commit, query) - commit = Commit.new(Gitlab::Git::Commit.new(commit.to_hash), @project) + commit = Commit.from_hash(commit.to_hash, @project) AnalyticsCommitSerializer.new(project: @project, total_time: event['total_time']).represent(commit) end diff --git a/lib/gitlab/git/blame.rb b/lib/gitlab/git/blame.rb index 8dbe25e55f6..31effdba292 100644 --- a/lib/gitlab/git/blame.rb +++ b/lib/gitlab/git/blame.rb @@ -16,7 +16,7 @@ module Gitlab def each @blames.each do |blame| yield( - Gitlab::Git::Commit.new(blame.commit), + Gitlab::Git::Commit.new(@repo, blame.commit), blame.line ) end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 600d886e818..9256663f454 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -14,7 +14,7 @@ module Gitlab attr_accessor *SERIALIZE_KEYS # rubocop:disable Lint/AmbiguousOperator - delegate :tree, to: :raw_commit + delegate :tree, to: :rugged_commit def ==(other) return false unless other.is_a?(Gitlab::Git::Commit) @@ -50,19 +50,29 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/321 def find(repo, commit_id = "HEAD") + # Already a commit? return commit_id if commit_id.is_a?(Gitlab::Git::Commit) - return decorate(commit_id) if commit_id.is_a?(Rugged::Commit) - obj = if commit_id.is_a?(String) - repo.rev_parse_target(commit_id) - else - Gitlab::Git::Ref.dereference_object(commit_id) - end + # A rugged reference? + commit_id = Gitlab::Git::Ref.dereference_object(commit_id) + return decorate(repo, commit_id) if commit_id.is_a?(Rugged::Commit) - return nil unless obj.is_a?(Rugged::Commit) + # Some weird thing? + return nil unless commit_id.is_a?(String) - decorate(obj) - rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError, Gitlab::Git::Repository::NoRepository + commit = repo.gitaly_migrate(:find_commit) do |is_enabled| + if is_enabled + repo.gitaly_commit_client.find_commit(commit_id) + else + obj = repo.rev_parse_target(commit_id) + + obj.is_a?(Rugged::Commit) ? obj : nil + end + end + + decorate(repo, commit) if commit + rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError, + Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository nil end @@ -102,7 +112,7 @@ module Gitlab if is_enabled repo.gitaly_commit_client.between(base, head) else - repo.rugged_commits_between(base, head).map { |c| decorate(c) } + repo.rugged_commits_between(base, head).map { |c| decorate(repo, c) } end end rescue Rugged::ReferenceError @@ -169,7 +179,7 @@ module Gitlab offset = actual_options[:skip] limit = actual_options[:max_count] walker.each(offset: offset, limit: limit) do |commit| - commits.push(decorate(commit)) + commits.push(decorate(repo, commit)) end walker.reset @@ -183,27 +193,8 @@ module Gitlab Gitlab::GitalyClient::CommitService.new(repo).find_all_commits(options) end - def decorate(commit, ref = nil) - Gitlab::Git::Commit.new(commit, ref) - end - - # Returns a diff object for the changes introduced by +rugged_commit+. - # If +rugged_commit+ doesn't have a parent, then the diff is between - # this commit and an empty repo. See Repository#diff for the keys - # allowed in the +options+ hash. - def diff_from_parent(rugged_commit, 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 + def decorate(repository, commit, ref = nil) + Gitlab::Git::Commit.new(repository, commit, ref) end # Returns the `Rugged` sorting type constant for one or more given @@ -221,7 +212,7 @@ module Gitlab end end - def initialize(raw_commit, head = nil) + def initialize(repository, raw_commit, head = nil) raise "Nil as raw commit passed" unless raw_commit case raw_commit @@ -229,12 +220,13 @@ module Gitlab init_from_hash(raw_commit) when Rugged::Commit init_from_rugged(raw_commit) - when Gitlab::GitalyClient::Commit + when Gitaly::GitCommit init_from_gitaly(raw_commit) else raise "Invalid raw commit type: #{raw_commit.class}" end + @repository = repository @head = head end @@ -269,19 +261,50 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/324 def to_diff - diff_from_parent.patch + rugged_diff_from_parent.patch end # Returns a diff object for the changes from this commit's first parent. # If there is no parent, then the diff is between this commit and an - # empty repo. See Repository#diff for keys allowed in the +options+ + # empty repo. See Repository#diff for keys allowed in the +options+ # hash. def diff_from_parent(options = {}) - Commit.diff_from_parent(raw_commit, options) + Gitlab::GitalyClient.migrate(:commit_raw_diffs) do |is_enabled| + if is_enabled + @repository.gitaly_commit_client.diff_from_parent(self, options) + else + rugged_diff_from_parent(options) + end + end + end + + 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 ||= diff_from_parent.each_delta.map { |d| Gitlab::Git::Diff.new(d) } + @deltas ||= begin + deltas = Gitlab::GitalyClient.migrate(:commit_deltas) do |is_enabled| + if is_enabled + @repository.gitaly_commit_client.commit_deltas(self) + else + rugged_diff_from_parent.each_delta + end + end + + deltas.map { |delta| Gitlab::Git::Diff.new(delta) } + end end def has_zero_stats? @@ -309,14 +332,7 @@ module Gitlab end def parents - case raw_commit - when Rugged::Commit - raw_commit.parents.map { |c| Gitlab::Git::Commit.new(c) } - when Gitlab::GitalyClient::Commit - parent_ids.map { |oid| self.class.find(raw_commit.repository, oid) }.compact - else - raise NotImplementedError, "commit source doesn't support #parents" - end + parent_ids.map { |oid| self.class.find(@repository, oid) }.compact end # Get the gpg signature of this commit. @@ -334,7 +350,7 @@ module Gitlab def to_patch(options = {}) begin - raw_commit.to_mbox(options) + rugged_commit.to_mbox(options) rescue Rugged::InvalidError => ex if ex.message =~ /commit \w+ is a merge commit/i 'Patch format is not currently supported for merge commits.' @@ -382,6 +398,14 @@ 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 + private def init_from_hash(hash) @@ -415,10 +439,10 @@ module Gitlab # subject from the message to make it clearer when there's one # available but not the other. @message = (commit.body.presence || commit.subject).dup - @authored_date = Time.at(commit.author.date.seconds) + @authored_date = Time.at(commit.author.date.seconds).utc @author_name = commit.author.name.dup @author_email = commit.author.email.dup - @committed_date = Time.at(commit.committer.date.seconds) + @committed_date = Time.at(commit.committer.date.seconds).utc @committer_name = commit.committer.name.dup @committer_email = commit.committer.email.dup @parent_ids = commit.parent_ids diff --git a/lib/gitlab/git/commit_stats.rb b/lib/gitlab/git/commit_stats.rb index 57c29ad112c..00acb4763e9 100644 --- a/lib/gitlab/git/commit_stats.rb +++ b/lib/gitlab/git/commit_stats.rb @@ -16,7 +16,7 @@ module Gitlab @deletions = 0 @total = 0 - diff = commit.diff_from_parent + diff = commit.rugged_diff_from_parent diff.each_patch do |p| # TODO: Use the new Rugged convenience methods when they're released diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb index 87ed9c3ea26..6a601561c2a 100644 --- a/lib/gitlab/git/diff_collection.rb +++ b/lib/gitlab/git/diff_collection.rb @@ -28,7 +28,6 @@ module Gitlab @limits = self.class.collection_limits(options) @enforce_limits = !!options.fetch(:limits, true) @expanded = !!options.fetch(:expanded, true) - @from_gitaly = options.fetch(:from_gitaly, false) @line_count = 0 @byte_count = 0 @@ -44,7 +43,7 @@ module Gitlab return if @iterator.nil? Gitlab::GitalyClient.migrate(:commit_raw_diffs) do |is_enabled| - if is_enabled && @from_gitaly + if is_enabled && @iterator.is_a?(Gitlab::GitalyClient::DiffStitcher) each_gitaly_patch(&block) else each_rugged_patch(&block) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index f246393cfbc..371f8797ff2 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -321,7 +321,7 @@ module Gitlab options[:limit] ||= 0 options[:offset] ||= 0 - raw_log(options).map { |c| Commit.decorate(c) } + raw_log(options).map { |c| Commit.decorate(self, c) } end def count_commits(options) diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index c90ef282fdd..70177cd0fec 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -100,5 +100,9 @@ module Gitlab path = Rails.root.join(SERVER_VERSION_FILE) path.read.chomp end + + def self.encode(s) + s.dup.force_encoding(Encoding::ASCII_8BIT) + end end end diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb deleted file mode 100644 index 61fe462d762..00000000000 --- a/lib/gitlab/gitaly_client/commit.rb +++ /dev/null @@ -1,14 +0,0 @@ -module Gitlab - module GitalyClient - class Commit - attr_reader :repository, :gitaly_commit - - delegate :id, :subject, :body, :author, :committer, :parent_ids, to: :gitaly_commit - - def initialize(repository, gitaly_commit) - @repository = repository - @gitaly_commit = gitaly_commit - end - end - end -end diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 3f577ac8530..692d7e02eef 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -29,22 +29,21 @@ module Gitlab request = Gitaly::CommitDiffRequest.new(request_params) response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request) - Gitlab::Git::DiffCollection.new(GitalyClient::DiffStitcher.new(response), options.merge(from_gitaly: true)) + GitalyClient::DiffStitcher.new(response) end def commit_deltas(commit) request = Gitaly::CommitDeltaRequest.new(commit_diff_request_params(commit)) response = GitalyClient.call(@repository.storage, :diff_service, :commit_delta, request) - response.flat_map do |msg| - msg.deltas.map { |d| Gitlab::Git::Diff.new(d) } - end + + response.flat_map { |msg| msg.deltas } end def tree_entry(ref, path, limit = nil) request = Gitaly::TreeEntryRequest.new( repository: @gitaly_repo, revision: ref, - path: path.dup.force_encoding(Encoding::ASCII_8BIT), + path: GitalyClient.encode(path), limit: limit.to_i ) @@ -100,15 +99,14 @@ module Gitlab def last_commit_for_path(revision, path) request = Gitaly::LastCommitForPathRequest.new( repository: @gitaly_repo, - revision: revision.force_encoding(Encoding::ASCII_8BIT), - path: path.to_s.force_encoding(Encoding::ASCII_8BIT) + revision: GitalyClient.encode(revision), + path: GitalyClient.encode(path.to_s) ) gitaly_commit = GitalyClient.call(@repository.storage, :commit_service, :last_commit_for_path, request).commit return unless gitaly_commit - commit = GitalyClient::Commit.new(@repository, gitaly_commit) - Gitlab::Git::Commit.new(commit) + Gitlab::Git::Commit.new(@repository, gitaly_commit) end def between(from, to) @@ -167,10 +165,21 @@ module Gitlab response.reduce("") { |memo, msg| memo << msg.data } end + def find_commit(revision) + request = Gitaly::FindCommitRequest.new( + repository: @gitaly_repo, + revision: GitalyClient.encode(revision) + ) + + response = GitalyClient.call(@repository.storage, :commit_service, :find_commit, request) + + response.commit + end + private def commit_diff_request_params(commit, options = {}) - parent_id = commit.parents[0]&.id || EMPTY_TREE_ID + parent_id = commit.parent_ids.first || EMPTY_TREE_ID { repository: @gitaly_repo, @@ -183,8 +192,7 @@ module Gitlab def consume_commits_response(response) response.flat_map do |message| message.commits.map do |gitaly_commit| - commit = GitalyClient::Commit.new(@repository, gitaly_commit) - Gitlab::Git::Commit.new(commit) + Gitlab::Git::Commit.new(@repository, gitaly_commit) end end end diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index b0f7548b7dc..919fb68b8c7 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -16,8 +16,7 @@ module Gitlab response.flat_map do |message| message.branches.map do |branch| - gitaly_commit = GitalyClient::Commit.new(@repository, branch.target) - target_commit = Gitlab::Git::Commit.decorate(gitaly_commit) + target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target) Gitlab::Git::Branch.new(@repository, branch.name, branch.target.id, target_commit) end end @@ -102,8 +101,7 @@ module Gitlab response.flat_map do |message| message.tags.map do |gitaly_tag| if gitaly_tag.target_commit.present? - commit = GitalyClient::Commit.new(@repository, gitaly_tag.target_commit) - gitaly_commit = Gitlab::Git::Commit.new(commit) + gitaly_commit = Gitlab::Git::Commit.decorate(@repository, gitaly_tag.target_commit) end Gitlab::Git::Tag.new( @@ -141,7 +139,7 @@ module Gitlab committer_email: response.commit_committer.email.dup } - Gitlab::Git::Commit.decorate(hash) + Gitlab::Git::Commit.decorate(@repository, hash) end end end 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 18843cbe992..f4dfa53f050 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 @@ -170,7 +170,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do context 'when the merge request diffs are Rugged::Patch instances' do let(:commits) { merge_request_diff.commits.map(&:to_hash) } let(:first_commit) { merge_request.project.repository.commit(merge_request_diff.head_commit_sha) } - let(:diffs) { first_commit.diff_from_parent.patches } + let(:diffs) { first_commit.rugged_diff_from_parent.patches } let(:expected_diffs) { [] } include_examples 'updated MR diff' @@ -179,7 +179,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do context 'when the merge request diffs are Rugged::Diff::Delta instances' do let(:commits) { merge_request_diff.commits.map(&:to_hash) } let(:first_commit) { merge_request.project.repository.commit(merge_request_diff.head_commit_sha) } - let(:diffs) { first_commit.diff_from_parent.deltas } + let(:diffs) { first_commit.rugged_diff_from_parent.deltas } let(:expected_diffs) { [] } include_examples 'updated MR diff' diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 26d7a364f5b..c531d4b055f 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe Gitlab::Git::Commit, seed_helper: true do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } - let(:commit) { Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID) } + let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } let(:rugged_commit) do repository.rugged.lookup(SeedRepo::Commit::ID) end @@ -24,7 +24,7 @@ describe Gitlab::Git::Commit, seed_helper: true do } @parents = [repo.head.target] - @gitlab_parents = @parents.map { |c| Gitlab::Git::Commit.decorate(c) } + @gitlab_parents = @parents.map { |c| described_class.decorate(repository, c) } @tree = @parents.first.tree sha = Rugged::Commit.create( @@ -38,7 +38,7 @@ describe Gitlab::Git::Commit, seed_helper: true do ) @raw_commit = repo.lookup(sha) - @commit = Gitlab::Git::Commit.new(@raw_commit) + @commit = described_class.new(repository, @raw_commit) end it { expect(@commit.short_id).to eq(@raw_commit.oid[0..10]) } @@ -66,6 +66,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe "Commit info from gitaly commit" do let(:id) { 'f00' } + let(:parent_ids) { %w(b45 b46) } let(:subject) { "My commit".force_encoding('ASCII-8BIT') } let(:body) { subject + "My body".force_encoding('ASCII-8BIT') } let(:committer) do @@ -88,10 +89,11 @@ describe Gitlab::Git::Commit, seed_helper: true do subject: subject, body: body, author: author, - committer: committer + committer: committer, + parent_ids: parent_ids ) end - let(:commit) { described_class.new(Gitlab::GitalyClient::Commit.new(repository, gitaly_commit)) } + let(:commit) { described_class.new(repository, gitaly_commit) } it { expect(commit.short_id).to eq(id[0..10]) } it { expect(commit.id).to eq(id) } @@ -102,6 +104,7 @@ describe Gitlab::Git::Commit, seed_helper: true do it { expect(commit.author_name).to eq(author.name) } it { expect(commit.committer_name).to eq(committer.name) } it { expect(commit.committer_email).to eq(committer.email) } + it { expect(commit.parent_ids).to eq(parent_ids) } context 'no body' do let(:body) { "".force_encoding('ASCII-8BIT') } @@ -113,45 +116,45 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'Class methods' do describe '.find' do it "should return first head commit if without params" do - expect(Gitlab::Git::Commit.last(repository).id).to eq( + expect(described_class.last(repository).id).to eq( repository.rugged.head.target.oid ) end it "should return valid commit" do - expect(Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID)).to be_valid_commit + expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_valid_commit end it "should return valid commit for tag" do - expect(Gitlab::Git::Commit.find(repository, 'v1.0.0').id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') + expect(described_class.find(repository, 'v1.0.0').id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') end it "should return nil for non-commit ids" do blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb") - expect(Gitlab::Git::Commit.find(repository, blob.id)).to be_nil + expect(described_class.find(repository, blob.id)).to be_nil end it "should return nil for parent of non-commit object" do blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb") - expect(Gitlab::Git::Commit.find(repository, "#{blob.id}^")).to be_nil + expect(described_class.find(repository, "#{blob.id}^")).to be_nil end it "should return nil for nonexisting ids" do - expect(Gitlab::Git::Commit.find(repository, "+123_4532530XYZ")).to be_nil + expect(described_class.find(repository, "+123_4532530XYZ")).to be_nil end context 'with broken repo' do let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH) } it 'returns nil' do - expect(Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID)).to be_nil + expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil end end end describe '.last_for_path' do context 'no path' do - subject { Gitlab::Git::Commit.last_for_path(repository, 'master') } + subject { described_class.last_for_path(repository, 'master') } describe '#id' do subject { super().id } @@ -160,7 +163,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end context 'path' do - subject { Gitlab::Git::Commit.last_for_path(repository, 'master', 'files/ruby') } + subject { described_class.last_for_path(repository, 'master', 'files/ruby') } describe '#id' do subject { super().id } @@ -169,7 +172,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end context 'ref + path' do - subject { Gitlab::Git::Commit.last_for_path(repository, SeedRepo::Commit::ID, 'encoding') } + subject { described_class.last_for_path(repository, SeedRepo::Commit::ID, 'encoding') } describe '#id' do subject { super().id } @@ -181,7 +184,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe '.where' do context 'path is empty string' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'master', path: '', @@ -199,7 +202,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'path is nil' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'master', path: nil, @@ -217,7 +220,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref is branch name' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'master', path: 'files', @@ -237,7 +240,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref is commit id' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: "874797c3a73b60d2187ed6e2fcabd289ff75171e", path: 'files', @@ -257,7 +260,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref is tag' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'v1.0.0', path: 'files', @@ -278,7 +281,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe '.between' do subject do - commits = Gitlab::Git::Commit.between(repository, SeedRepo::Commit::PARENT_ID, SeedRepo::Commit::ID) + commits = described_class.between(repository, SeedRepo::Commit::PARENT_ID, SeedRepo::Commit::ID) commits.map { |c| c.id } end @@ -294,12 +297,12 @@ describe Gitlab::Git::Commit, seed_helper: true do it 'should return a return a collection of commits' do commits = described_class.find_all(repository) - expect(commits).to all( be_a_kind_of(Gitlab::Git::Commit) ) + expect(commits).to all( be_a_kind_of(described_class) ) end context 'max_count' do subject do - commits = Gitlab::Git::Commit.find_all( + commits = described_class.find_all( repository, max_count: 50 ) @@ -322,7 +325,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref + max_count + skip' do subject do - commits = Gitlab::Git::Commit.find_all( + commits = described_class.find_all( repository, ref: 'master', max_count: 50, @@ -374,7 +377,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe '#init_from_rugged' do - let(:gitlab_commit) { Gitlab::Git::Commit.new(rugged_commit) } + let(:gitlab_commit) { described_class.new(repository, rugged_commit) } subject { gitlab_commit } describe '#id' do @@ -384,7 +387,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe '#init_from_hash' do - let(:commit) { Gitlab::Git::Commit.new(sample_commit_hash) } + let(:commit) { described_class.new(repository, sample_commit_hash) } subject { commit } describe '#id' do @@ -451,7 +454,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe '#ref_names' do - let(:commit) { Gitlab::Git::Commit.find(repository, 'master') } + let(:commit) { described_class.find(repository, 'master') } subject { commit.ref_names(repository) } it 'has 1 element' do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 20d6b58d6d1..858616117d5 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -505,17 +505,22 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe "#log" do - commit_with_old_name = nil - commit_with_new_name = nil - rename_commit = nil + let(:commit_with_old_name) do + Gitlab::Git::Commit.decorate(repository, @commit_with_old_name_id) + end + let(:commit_with_new_name) do + Gitlab::Git::Commit.decorate(repository, @commit_with_new_name_id) + end + let(:rename_commit) do + Gitlab::Git::Commit.decorate(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 = Gitlab::Git::Commit.decorate(new_commit_edit_old_file(repo)) - rename_commit = Gitlab::Git::Commit.decorate(new_commit_move_file(repo)) - commit_with_new_name = Gitlab::Git::Commit.decorate(new_commit_edit_new_file(repo)) + @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) end after(:context) do @@ -754,7 +759,7 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:options) { { ref: 'master', path: ['PROCESS.md', 'README.md'] } } def commit_files(commit) - commit.diff_from_parent.deltas.flat_map do |delta| + commit.rugged_diff_from_parent.deltas.flat_map do |delta| [delta.old_file[:path], delta.new_file[:path]].uniq.compact end end diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index d71e0f84c65..7fe698fcb18 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::GitalyClient::CommitService do context 'when a commit does not have a parent' do it 'sends an RPC request with empty tree ref as left commit' do - initial_commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') + initial_commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863').raw request = Gitaly::CommitDiffRequest.new( repository: repository_message, left_commit_id: '4b825dc642cb6eb9a060e54bf8d69288fbee4904', @@ -46,18 +46,10 @@ describe Gitlab::GitalyClient::CommitService do end end - it 'returns a Gitlab::Git::DiffCollection' do + it 'returns a Gitlab::GitalyClient::DiffStitcher' do ret = client.diff_from_parent(commit) - expect(ret).to be_kind_of(Gitlab::Git::DiffCollection) - end - - it 'passes options to Gitlab::Git::DiffCollection' do - options = { max_files: 31, max_lines: 13, from_gitaly: true } - - expect(Gitlab::Git::DiffCollection).to receive(:new).with(kind_of(Enumerable), options) - - client.diff_from_parent(commit, options) + expect(ret).to be_kind_of(Gitlab::GitalyClient::DiffStitcher) end end @@ -120,4 +112,18 @@ describe Gitlab::GitalyClient::CommitService do client.tree_entries(repository, revision, path) end end + + describe '#find_commit' do + let(:revision) { '4b825dc642cb6eb9a060e54bf8d69288fbee4904' } + it 'sends an RPC request' do + request = Gitaly::FindCommitRequest.new( + repository: repository_message, revision: revision + ) + + expect_any_instance_of(Gitaly::CommitService::Stub).to receive(:find_commit) + .with(request, kind_of(Hash)).and_return(double(commit: nil)) + + described_class.new(repository).find_commit(revision) + end + end end diff --git a/spec/migrations/migrate_process_commit_worker_jobs_spec.rb b/spec/migrations/migrate_process_commit_worker_jobs_spec.rb index cf2d5827306..e5793a3c0ee 100644 --- a/spec/migrations/migrate_process_commit_worker_jobs_spec.rb +++ b/spec/migrations/migrate_process_commit_worker_jobs_spec.rb @@ -6,7 +6,7 @@ require Rails.root.join('db', 'migrate', '20161124141322_migrate_process_commit_ describe MigrateProcessCommitWorkerJobs do let(:project) { create(:project, :repository) } let(:user) { create(:user) } - let(:commit) { project.commit.raw.raw_commit } + let(:commit) { project.commit.raw.rugged_commit } describe 'Project' do describe 'find_including_path' do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 08693b5da33..c18c635d811 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -33,7 +33,6 @@ describe Commit do describe '#to_reference' do let(:project) { create(:project, :repository, path: 'sample-project') } - let(:commit) { project.commit } it 'returns a String reference to the object' do expect(commit.to_reference).to eq commit.id @@ -47,7 +46,6 @@ describe Commit do describe '#reference_link_text' do let(:project) { create(:project, :repository, path: 'sample-project') } - let(:commit) { project.commit } it 'returns a String reference to the object' do expect(commit.reference_link_text).to eq commit.short_id @@ -191,7 +189,7 @@ eos it { expect(data).to be_a(Hash) } it { expect(data[:message]).to include('adds bar folder and branch-test text file to check Repository merged_to_root_ref method') } - it { expect(data[:timestamp]).to eq('2016-09-27T14:37:46+00:00') } + it { expect(data[:timestamp]).to eq('2016-09-27T14:37:46Z') } it { expect(data[:added]).to eq(["bar/branch-test.txt"]) } it { expect(data[:modified]).to eq([]) } it { expect(data[:removed]).to eq([]) } |