diff --git a/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml b/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml new file mode 100644 index 00000000000..f3c39827590 --- /dev/null +++ b/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml @@ -0,0 +1,5 @@ +--- +title: Fix 500 error on merged merge requests when GitLab is restored from a backup +merge_request: +author: +type: fixed diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 8c1df650567..6baff362dad 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -947,7 +947,11 @@ module Gitlab if start_repository == self yield commit(start_branch_name) else - sha = start_repository.commit(start_branch_name).sha + start_commit = start_repository.commit(start_branch_name) + + return yield nil unless start_commit + + sha = start_commit.sha if branch_commit = commit(sha) yield branch_commit @@ -976,8 +980,9 @@ module Gitlab with_repo_branch_commit(source_repository, source_branch) do |commit| if commit write_ref(local_ref, commit.sha) + true else - raise Rugged::ReferenceError, 'source repository is empty' + false end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 4fc26c625a5..5effaf2b043 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1332,6 +1332,84 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#with_repo_branch_commit' do + context 'when comparing with the same repository' do + let(:start_repository) { repository } + + context 'when the branch exists' do + let(:start_branch_name) { 'master' } + + it 'yields the commit' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(an_instance_of(Gitlab::Git::Commit)) + end + end + + context 'when the branch does not exist' do + let(:start_branch_name) { 'definitely-not-master' } + + it 'yields nil' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(nil) + end + end + end + + context 'when comparing with another repository' do + let(:start_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + + context 'when the branch exists' do + let(:start_branch_name) { 'master' } + + it 'yields the commit' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(an_instance_of(Gitlab::Git::Commit)) + end + end + + context 'when the branch does not exist' do + let(:start_branch_name) { 'definitely-not-master' } + + it 'yields nil' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(nil) + end + end + end + end + + describe '#fetch_source_branch' do + let(:local_ref) { 'refs/merge-requests/1/head' } + + context 'when the branch exists' do + let(:source_branch) { 'master' } + + it 'writes the ref' do + expect(repository).to receive(:write_ref).with(local_ref, /\h{40}/) + + repository.fetch_source_branch(repository, source_branch, local_ref) + end + + it 'returns true' do + expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(true) + end + end + + context 'when the branch does not exist' do + let(:source_branch) { 'definitely-not-master' } + + it 'does not write the ref' do + expect(repository).not_to receive(:write_ref) + + repository.fetch_source_branch(repository, source_branch, local_ref) + end + + it 'returns false' do + expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(false) + end + end + end + def create_remote_branch(repository, remote_name, branch_name, source_branch_name) source_branch = repository.branches.find { |branch| branch.name == source_branch_name } rugged = repository.rugged