summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2017-09-28 11:11:10 +0100
committerSean McGivern <sean@gitlab.com>2017-09-28 11:11:10 +0100
commitb63e8d64c1a2d39a7e18f22b526e48bab2c79956 (patch)
treed83eb0d2fba53327b2ebbb3a6e5e1663ad30551a
parente03bad12bf579a83a814c07399ef9bdc37e2f120 (diff)
downloadgitlab-ce-b63e8d64c1a2d39a7e18f22b526e48bab2c79956.tar.gz
Handle error when fetching ref for MR with deleted source branch38319-nomethoderror-undefined-method-sha-for-nil-nilclass
If the ref doesn't exist, and the source branch is deleted, we can't get it back easily. Previously, we ignored this error by shelling out, so replicate that behaviour.
-rw-r--r--changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml5
-rw-r--r--lib/gitlab/git/repository.rb9
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb78
3 files changed, 90 insertions, 2 deletions
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