summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlejandro Rodríguez <alejorro70@gmail.com>2017-12-12 22:55:30 -0300
committerAlejandro Rodríguez <alejorro70@gmail.com>2017-12-14 16:02:50 -0300
commit240945f87edf80e7f9a4e991b99fd4ade28aaabd (patch)
tree6e536b76bbe02b5db4d2cac85e89ff0c26256554
parent31fa9313991881258b4697cb507cfc8ab205b7dc (diff)
downloadgitlab-ce-gitaly-conflicts-prep.tar.gz
Simplify conflict resolver interfacegitaly-conflicts-prep
This does two things: - Pass commit oids instead of `Gitlab::Git::Commit`s. We only need the former. - Depend on only the target repository for conflict listing. For conflict resolution, treat one repository as a remote one so that we can implement it as such in Gitaly.
-rw-r--r--lib/gitlab/conflict/file_collection.rb6
-rw-r--r--lib/gitlab/git/conflict/resolver.rb59
-rw-r--r--spec/services/merge_requests/conflicts/resolve_service_spec.rb2
3 files changed, 33 insertions, 34 deletions
diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb
index b9099ce256a..76aee5a3deb 100644
--- a/lib/gitlab/conflict/file_collection.rb
+++ b/lib/gitlab/conflict/file_collection.rb
@@ -4,11 +4,11 @@ module Gitlab
attr_reader :merge_request, :resolver
def initialize(merge_request)
- source_repo = merge_request.source_project.repository.raw
our_commit = merge_request.source_branch_head.raw
their_commit = merge_request.target_branch_head.raw
target_repo = merge_request.target_project.repository.raw
- @resolver = Gitlab::Git::Conflict::Resolver.new(source_repo, our_commit, target_repo, their_commit)
+ @source_repo = merge_request.source_project.repository.raw
+ @resolver = Gitlab::Git::Conflict::Resolver.new(target_repo, our_commit.id, their_commit.id)
@merge_request = merge_request
end
@@ -18,7 +18,7 @@ module Gitlab
target_branch: merge_request.target_branch,
commit_message: commit_message || default_commit_message
}
- resolver.resolve_conflicts(user, files, args)
+ resolver.resolve_conflicts(@source_repo, user, files, args)
ensure
@merge_request.clear_memoized_shas
end
diff --git a/lib/gitlab/git/conflict/resolver.rb b/lib/gitlab/git/conflict/resolver.rb
index de8cce41b6d..03e5c0fcd6f 100644
--- a/lib/gitlab/git/conflict/resolver.rb
+++ b/lib/gitlab/git/conflict/resolver.rb
@@ -5,38 +5,31 @@ module Gitlab
ConflictSideMissing = Class.new(StandardError)
ResolutionError = Class.new(StandardError)
- def initialize(repository, our_commit, target_repository, their_commit)
- @repository = repository
- @our_commit = our_commit.rugged_commit
+ def initialize(target_repository, our_commit_oid, their_commit_oid)
@target_repository = target_repository
- @their_commit = their_commit.rugged_commit
+ @our_commit_oid = our_commit_oid
+ @their_commit_oid = their_commit_oid
end
def conflicts
@conflicts ||= begin
- target_index = @target_repository.rugged.merge_commits(@our_commit, @their_commit)
+ target_index = @target_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid)
# We don't need to do `with_repo_branch_commit` here, because the target
# project always fetches source refs when creating merge request diffs.
- target_index.conflicts.map do |conflict|
- raise ConflictSideMissing unless conflict[:theirs] && conflict[:ours]
-
- Gitlab::Git::Conflict::File.new(
- @target_repository,
- @our_commit.oid,
- conflict,
- target_index.merge_file(conflict[:ours][:path])[:data]
- )
- end
+ conflict_files(@target_repository, target_index)
end
end
- def resolve_conflicts(user, files, source_branch:, target_branch:, commit_message:)
- @repository.with_repo_branch_commit(@target_repository, target_branch) do
+ def resolve_conflicts(source_repository, user, files, source_branch:, target_branch:, commit_message:)
+ source_repository.with_repo_branch_commit(@target_repository, target_branch) do
+ index = source_repository.rugged.merge_commits(@our_commit_oid, @their_commit_oid)
+ conflicts = conflict_files(source_repository, index)
+
files.each do |file_params|
- conflict_file = conflict_for_path(file_params[:old_path], file_params[:new_path])
+ conflict_file = conflict_for_path(conflicts, file_params[:old_path], file_params[:new_path])
- write_resolved_file_to_index(conflict_file, file_params)
+ write_resolved_file_to_index(source_repository, index, conflict_file, file_params)
end
unless index.conflicts.empty?
@@ -47,14 +40,14 @@ module Gitlab
commit_params = {
message: commit_message,
- parents: [@our_commit, @their_commit].map(&:oid)
+ parents: [@our_commit_oid, @their_commit_oid]
}
- @repository.commit_index(user, source_branch, index, commit_params)
+ source_repository.commit_index(user, source_branch, index, commit_params)
end
end
- def conflict_for_path(old_path, new_path)
+ def conflict_for_path(conflicts, old_path, new_path)
conflicts.find do |conflict|
conflict.their_path == old_path && conflict.our_path == new_path
end
@@ -62,15 +55,20 @@ module Gitlab
private
- # We can only write when getting the merge index from the source
- # project, because we will write to that project. We don't use this all
- # the time because this fetches a ref into the source project, which
- # isn't needed for reading.
- def index
- @index ||= @repository.rugged.merge_commits(@our_commit, @their_commit)
+ def conflict_files(repository, index)
+ index.conflicts.map do |conflict|
+ raise ConflictSideMissing unless conflict[:theirs] && conflict[:ours]
+
+ Gitlab::Git::Conflict::File.new(
+ repository,
+ @our_commit_oid,
+ conflict,
+ index.merge_file(conflict[:ours][:path])[:data]
+ )
+ end
end
- def write_resolved_file_to_index(file, params)
+ def write_resolved_file_to_index(repository, index, file, params)
if params[:sections]
resolved_lines = file.resolve_lines(params[:sections])
new_file = resolved_lines.map { |line| line[:full_line] }.join("\n")
@@ -82,7 +80,8 @@ module Gitlab
our_path = file.our_path
- index.add(path: our_path, oid: @repository.rugged.write(new_file, :blob), mode: file.our_mode)
+ oid = repository.rugged.write(new_file, :blob)
+ index.add(path: our_path, oid: oid, mode: file.our_mode)
index.conflict_remove(our_path)
end
end
diff --git a/spec/services/merge_requests/conflicts/resolve_service_spec.rb b/spec/services/merge_requests/conflicts/resolve_service_spec.rb
index 5376083e7f5..e28d8d7ae5c 100644
--- a/spec/services/merge_requests/conflicts/resolve_service_spec.rb
+++ b/spec/services/merge_requests/conflicts/resolve_service_spec.rb
@@ -213,7 +213,7 @@ describe MergeRequests::Conflicts::ResolveService do
MergeRequests::Conflicts::ListService.new(merge_request).conflicts.resolver
end
let(:regex_conflict) do
- resolver.conflict_for_path('files/ruby/regex.rb', 'files/ruby/regex.rb')
+ resolver.conflict_for_path(resolver.conflicts, 'files/ruby/regex.rb', 'files/ruby/regex.rb')
end
let(:invalid_params) do