diff --git a/CHANGELOG b/CHANGELOG
index d06fc24d40a982842d91c57c5ec2b055ea19966d..d95072d99522e36ddba27161e58d7d9d1f842ebd 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -36,6 +36,9 @@ v 8.12.0 (unreleased)
- Use the default branch for displaying the project icon instead of master !5792 (Hannes Rosenögger)
- Adds response mime type to transaction metric action when it's not HTML
+v 8.11.4 (unreleased)
+ - Fix resolving conflicts on forks
+
v 8.11.3 (unreleased)
- Allow system info page to handle case where info is unavailable
- Label list shows all issues (opened or closed) with that label
diff --git a/app/services/merge_requests/resolve_service.rb b/app/services/merge_requests/resolve_service.rb
index adc71b0c2bcfb47ff7e5cda058af6197a06e2ace..bd8b9f8cfd42b38b6d438b6bc3718009c8e7ed49 100644
--- a/app/services/merge_requests/resolve_service.rb
+++ b/app/services/merge_requests/resolve_service.rb
@@ -1,11 +1,14 @@
module MergeRequests
class ResolveService < MergeRequests::BaseService
- attr_accessor :conflicts, :rugged, :merge_index
+ attr_accessor :conflicts, :rugged, :merge_index, :merge_request
def execute(merge_request)
@conflicts = merge_request.conflicts
@rugged = project.repository.rugged
@merge_index = conflicts.merge_index
+ @merge_request = merge_request
+
+ fetch_their_commit!
conflicts.files.each do |file|
write_resolved_file_to_index(file, params[:sections])
@@ -27,5 +30,16 @@ module MergeRequests
merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode)
merge_index.conflict_remove(our_path)
end
+
+ # If their commit (in the target project) doesn't exist in the source project, it
+ # can't be a parent for the merge commit we're about to create. If that's the case,
+ # fetch the target branch ref into the source project so the commit exists in both.
+ #
+ def fetch_their_commit!
+ return if rugged.include?(conflicts.their_commit.oid)
+
+ remote = rugged.remotes.create_anonymous(merge_request.target_project.repository.path_to_repo)
+ remote.fetch(merge_request.target_branch)
+ end
end
end
diff --git a/spec/services/merge_requests/resolve_service_spec.rb b/spec/services/merge_requests/resolve_service_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..d71932458fa049a80140f6337734b7fd6fe16c8d
--- /dev/null
+++ b/spec/services/merge_requests/resolve_service_spec.rb
@@ -0,0 +1,87 @@
+require 'spec_helper'
+
+describe MergeRequests::ResolveService do
+ let(:user) { create(:user) }
+ let(:project) { create(:project) }
+
+ let(:fork_project) do
+ create(:forked_project_with_submodules) do |fork_project|
+ fork_project.build_forked_project_link(forked_to_project_id: fork_project.id, forked_from_project_id: project.id)
+ fork_project.save
+ end
+ end
+
+ let(:merge_request) do
+ create(:merge_request,
+ source_branch: 'conflict-resolvable', source_project: project,
+ target_branch: 'conflict-start')
+ end
+
+ let(:merge_request_from_fork) do
+ create(:merge_request,
+ source_branch: 'conflict-resolvable-fork', source_project: fork_project,
+ target_branch: 'conflict-start', target_project: project)
+ end
+
+ describe '#execute' do
+ context 'with valid params' do
+ let(:params) do
+ {
+ sections: {
+ '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head',
+ '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
+ '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
+ '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin'
+ },
+ commit_message: 'This is a commit message!'
+ }
+ end
+
+ context 'when the source and target project are the same' do
+ before do
+ MergeRequests::ResolveService.new(project, user, params).execute(merge_request)
+ end
+
+ it 'creates a commit with the message' do
+ expect(merge_request.source_branch_head.message).to eq(params[:commit_message])
+ end
+
+ it 'creates a commit with the correct parents' do
+ expect(merge_request.source_branch_head.parents.map(&:id)).
+ to eq(['1450cd639e0bc6721eb02800169e464f212cde06',
+ '75284c70dd26c87f2a3fb65fd5a1f0b0138d3a6b'])
+ end
+ end
+
+ context 'when the source project is a fork and does not contain the HEAD of the target branch' do
+ let!(:target_head) do
+ project.repository.commit_file(user, 'new-file-in-target', '', 'Add new file in target', 'conflict-start', false)
+ end
+
+ before do
+ MergeRequests::ResolveService.new(fork_project, user, params).execute(merge_request_from_fork)
+ end
+
+ it 'creates a commit with the message' do
+ expect(merge_request_from_fork.source_branch_head.message).to eq(params[:commit_message])
+ end
+
+ it 'creates a commit with the correct parents' do
+ expect(merge_request_from_fork.source_branch_head.parents.map(&:id)).
+ to eq(['404fa3fc7c2c9b5dacff102f353bdf55b1be2813',
+ target_head])
+ end
+ end
+ end
+
+ context 'when a resolution is missing' do
+ let(:invalid_params) { { sections: { '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' } } }
+ let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) }
+
+ it 'raises a MissingResolution error' do
+ expect { service.execute(merge_request) }.
+ to raise_error(Gitlab::Conflict::File::MissingResolution)
+ end
+ end
+ end
+end
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index c7a45fc4ff9a62681bd9b548e5bfc70f3f9fafcf..0097dbf8fadc14b80a6961db6455f494b4f1cb81 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -6,7 +6,7 @@ module TestEnv
# When developing the seed repository, comment out the branch you will modify.
BRANCH_SHA = {
'empty-branch' => '7efb185',
- 'ends-with.json' => '98b0d8b3',
+ 'ends-with.json' => '98b0d8b',
'flatten-dir' => 'e56497b',
'feature' => '0b4bc9a',
'feature_conflict' => 'bb5206f',
@@ -37,9 +37,10 @@ module TestEnv
# need to keep all the branches in sync.
# We currently only need a subset of the branches
FORKED_BRANCH_SHA = {
- 'add-submodule-version-bump' => '3f547c08',
- 'master' => '5937ac0',
- 'remove-submodule' => '2a33e0c0'
+ 'add-submodule-version-bump' => '3f547c0',
+ 'master' => '5937ac0',
+ 'remove-submodule' => '2a33e0c',
+ 'conflict-resolvable-fork' => '404fa3f'
}
# Test environment
@@ -117,22 +118,7 @@ module TestEnv
system(*%W(#{Gitlab.config.git.bin_path} clone -q #{clone_url} #{repo_path}))
end
- Dir.chdir(repo_path) do
- branch_sha.each do |branch, sha|
- # Try to reset without fetching to avoid using the network.
- reset = %W(#{Gitlab.config.git.bin_path} update-ref refs/heads/#{branch} #{sha})
- unless system(*reset)
- if system(*%W(#{Gitlab.config.git.bin_path} fetch origin))
- unless system(*reset)
- raise 'The fetched test seed '\
- 'does not contain the required revision.'
- end
- else
- raise 'Could not fetch test seed repository.'
- end
- end
- end
- end
+ set_repo_refs(repo_path, branch_sha)
# We must copy bare repositories because we will push to them.
system(git_env, *%W(#{Gitlab.config.git.bin_path} clone -q --bare #{repo_path} #{repo_path_bare}))
@@ -144,6 +130,7 @@ module TestEnv
FileUtils.mkdir_p(target_repo_path)
FileUtils.cp_r("#{base_repo_path}/.", target_repo_path)
FileUtils.chmod_R 0755, target_repo_path
+ set_repo_refs(target_repo_path, BRANCH_SHA)
end
def repos_path
@@ -160,6 +147,7 @@ module TestEnv
FileUtils.mkdir_p(target_repo_path)
FileUtils.cp_r("#{base_repo_path}/.", target_repo_path)
FileUtils.chmod_R 0755, target_repo_path
+ set_repo_refs(target_repo_path, FORKED_BRANCH_SHA)
end
# When no cached assets exist, manually hit the root path to create them
@@ -209,4 +197,23 @@ module TestEnv
def git_env
{ 'GIT_TEMPLATE_DIR' => '' }
end
+
+ def set_repo_refs(repo_path, branch_sha)
+ Dir.chdir(repo_path) do
+ branch_sha.each do |branch, sha|
+ # Try to reset without fetching to avoid using the network.
+ reset = %W(#{Gitlab.config.git.bin_path} update-ref refs/heads/#{branch} #{sha})
+ unless system(*reset)
+ if system(*%W(#{Gitlab.config.git.bin_path} fetch origin))
+ unless system(*reset)
+ raise 'The fetched test seed '\
+ 'does not contain the required revision.'
+ end
+ else
+ raise 'Could not fetch test seed repository.'
+ end
+ end
+ end
+ end
+ end
end