summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--Gemfile.lock4
-rw-r--r--app/controllers/projects/compare_controller.rb21
-rw-r--r--app/models/merge_request_diff.rb21
-rw-r--r--app/services/compare_service.rb27
-rw-r--r--app/services/merge_requests/build_service.rb11
-rw-r--r--app/views/notify/repository_push_email.html.haml4
-rw-r--r--app/views/notify/repository_push_email.text.haml2
-rw-r--r--app/workers/emails_on_push_worker.rb2
-rw-r--r--lib/api/entities.rb8
-rw-r--r--lib/api/repositories.rb2
-rw-r--r--lib/gitlab/compare_result.rb9
-rw-r--r--lib/gitlab/satellite/compare_action.rb26
12 files changed, 79 insertions, 58 deletions
diff --git a/Gemfile.lock b/Gemfile.lock
index 385a5fac69b..c77d20debd9 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -179,7 +179,7 @@ GEM
mime-types (~> 1.19)
gitlab_emoji (0.0.1.1)
emoji (~> 1.0.1)
- gitlab_git (6.0.1)
+ gitlab_git (6.1.0)
activesupport (~> 4.0)
charlock_holmes (~> 0.6)
gitlab-grit (~> 2.6)
@@ -244,7 +244,7 @@ GEM
json (~> 1.8)
multi_xml (>= 0.5.2)
httpauth (0.2.0)
- i18n (0.6.9)
+ i18n (0.6.11)
ice_nine (0.10.0)
jasmine (2.0.2)
jasmine-core (~> 2.0.0)
diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb
index eae96396574..7a671e8455d 100644
--- a/app/controllers/projects/compare_controller.rb
+++ b/app/controllers/projects/compare_controller.rb
@@ -8,14 +8,21 @@ class Projects::CompareController < Projects::ApplicationController
end
def show
- compare = Gitlab::Git::Compare.new(@repository.raw_repository, params[:from], params[:to], MergeRequestDiff::COMMITS_SAFE_SIZE)
+ base_ref = params[:from]
+ head_ref = params[:to]
- @commits = compare.commits
- @commit = compare.commit
- @diffs = compare.diffs
- @refs_are_same = compare.same
- @line_notes = []
- @diff_timeout = compare.timeout
+ compare_result = CompareService.new.execute(
+ current_user,
+ @project,
+ head_ref,
+ @project,
+ base_ref
+ )
+
+ @commits = compare_result.commits
+ @diffs = compare_result.diffs
+ @commit = @commits.last
+ @line_notes = []
end
def create
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index d3c07555b0c..248fa18353e 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -83,11 +83,7 @@ class MergeRequestDiff < ActiveRecord::Base
# Collect array of Git::Commit objects
# between target and source branches
def unmerged_commits
- commits = if merge_request.for_fork?
- compare_action.commits
- else
- repository.commits_between(target_branch, source_branch)
- end
+ commits = compare_result.commits
if commits.present?
commits = Commit.decorate(commits).
@@ -147,12 +143,7 @@ class MergeRequestDiff < ActiveRecord::Base
# Collect array of Git::Diff objects
# between target and source branches
def unmerged_diffs
- diffs = if merge_request.for_fork?
- compare_action.diffs
- else
- Gitlab::Git::Diff.between(repository, source_branch, target_branch)
- end
-
+ diffs = compare_result.diffs
diffs ||= []
diffs
rescue Gitlab::Git::Diff::TimeoutError => ex
@@ -166,13 +157,13 @@ class MergeRequestDiff < ActiveRecord::Base
private
- def compare_action
- Gitlab::Satellite::CompareAction.new(
+ def compare_result
+ @compare_result ||= CompareService.new.execute(
merge_request.author,
+ merge_request.source_project,
+ merge_request.source_branch,
merge_request.target_project,
merge_request.target_branch,
- merge_request.source_project,
- merge_request.source_branch
)
end
end
diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb
new file mode 100644
index 00000000000..c5e04702914
--- /dev/null
+++ b/app/services/compare_service.rb
@@ -0,0 +1,27 @@
+# Compare 2 branches for one repo or between repositories
+# and return Gitlab::CompareResult object that responds to commits and diffs
+class CompareService
+ def execute(current_user, source_project, source_branch, target_project, target_branch)
+ # Try to compare branches to get commits list and diffs
+ #
+ # Note: Use satellite only when need to compare between to repos
+ # because satellites are slower then operations on bare repo
+ if target_project == source_project
+ Gitlab::CompareResult.new(
+ Gitlab::Git::Compare.new(
+ target_project.repository.raw_repository,
+ target_branch,
+ source_branch,
+ )
+ )
+ else
+ Gitlab::Satellite::CompareAction.new(
+ current_user,
+ target_project,
+ target_branch,
+ source_project,
+ source_branch
+ ).result
+ end
+ end
+end
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 81dd8887395..1475973e543 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -19,16 +19,15 @@ module MergeRequests
# Generate suggested MR title based on source branch name
merge_request.title = merge_request.source_branch.titleize.humanize
- # Try to compare branches to get commits list and diffs
- compare_action = Gitlab::Satellite::CompareAction.new(
+ compare_result = CompareService.new.execute(
current_user,
+ merge_request.source_project,
+ merge_request.source_branch,
merge_request.target_project,
merge_request.target_branch,
- merge_request.source_project,
- merge_request.source_branch
)
- commits = compare_action.commits
+ commits = compare_result.commits
# At this point we decide if merge request can be created
# If we have at least one commit to merge -> creation allowed
@@ -38,7 +37,7 @@ module MergeRequests
merge_request.compare_failed = false
# Try to collect diff for merge request.
- diffs = compare_action.diffs
+ diffs = compare_result.diffs
if diffs.present?
merge_request.compare_diffs = diffs
diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml
index bf358fe70a9..0358810afdc 100644
--- a/app/views/notify/repository_push_email.html.haml
+++ b/app/views/notify/repository_push_email.html.haml
@@ -25,6 +25,4 @@
%br
- if @compare.timeout
- %h5 To prevent performance issues changes are hidden
-- elsif @compare.commits_over_limit?
- %h5 Changes are not shown due to large amount of commits
+ %h5 Huge diff. To prevent performance issues changes are hidden
diff --git a/app/views/notify/repository_push_email.text.haml b/app/views/notify/repository_push_email.text.haml
index ac337c7628a..4d7c972a16a 100644
--- a/app/views/notify/repository_push_email.text.haml
+++ b/app/views/notify/repository_push_email.text.haml
@@ -23,5 +23,3 @@ Changes:
\
- if @compare.timeout
Huge diff. To prevent performance issues it was hidden
-- elsif @compare.commits_over_limit?
- Changes are not shown due to large amount of commits
diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb
index 5e81810cbdb..2947c8e3ecd 100644
--- a/app/workers/emails_on_push_worker.rb
+++ b/app/workers/emails_on_push_worker.rb
@@ -13,7 +13,7 @@ class EmailsOnPushWorker
return true
end
- compare = Gitlab::Git::Compare.new(project.repository.raw_repository, before_sha, after_sha, MergeRequestDiff::COMMITS_SAFE_SIZE)
+ compare = Gitlab::Git::Compare.new(project.repository.raw_repository, before_sha, after_sha)
# Do not send emails if git compare failed
return false unless compare && compare.commits.present?
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 09fb97abf29..238416c5379 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -201,13 +201,13 @@ module API
class Compare < Grape::Entity
expose :commit, using: Entities::RepoCommit do |compare, options|
- if compare.commit
- Commit.new compare.commit
- end
+ Commit.decorate(compare.commits).last
end
+
expose :commits, using: Entities::RepoCommit do |compare, options|
- Commit.decorate compare.commits
+ Commit.decorate(compare.commits)
end
+
expose :diffs, using: Entities::RepoDiff do |compare, options|
compare.diffs
end
diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb
index d091fa4f035..461ce4e59cf 100644
--- a/lib/api/repositories.rb
+++ b/lib/api/repositories.rb
@@ -147,7 +147,7 @@ module API
get ':id/repository/compare' do
authorize! :download_code, user_project
required_attributes! [:from, :to]
- compare = Gitlab::Git::Compare.new(user_project.repository.raw_repository, params[:from], params[:to], MergeRequestDiff::COMMITS_SAFE_SIZE)
+ compare = Gitlab::Git::Compare.new(user_project.repository.raw_repository, params[:from], params[:to])
present compare, with: Entities::Compare
end
diff --git a/lib/gitlab/compare_result.rb b/lib/gitlab/compare_result.rb
new file mode 100644
index 00000000000..d72391dade5
--- /dev/null
+++ b/lib/gitlab/compare_result.rb
@@ -0,0 +1,9 @@
+module Gitlab
+ class CompareResult
+ attr_reader :commits, :diffs
+
+ def initialize(compare)
+ @commits, @diffs = compare.commits, compare.diffs
+ end
+ end
+end
diff --git a/lib/gitlab/satellite/compare_action.rb b/lib/gitlab/satellite/compare_action.rb
index e5b62576330..46c98a8f4ca 100644
--- a/lib/gitlab/satellite/compare_action.rb
+++ b/lib/gitlab/satellite/compare_action.rb
@@ -10,28 +10,16 @@ module Gitlab
@source_project, @source_branch = source_project, source_branch
end
- # Only show what is new in the source branch compared to the target branch, not the other way around.
- # The line below with merge_base is equivalent to diff with three dots (git diff branch1...branch2)
- # From the git documentation: "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B"
- def diffs
+ # Compare 2 repositories and return Gitlab::CompareResult object
+ def result
in_locked_and_timed_satellite do |target_repo|
prepare_satellite!(target_repo)
update_satellite_source_and_target!(target_repo)
- compare(target_repo).diffs
- end
- rescue Grit::Git::CommandFailed => ex
- raise BranchesWithoutParent
- end
- # Retrieve an array of commits between the source and the target
- def commits
- in_locked_and_timed_satellite do |target_repo|
- prepare_satellite!(target_repo)
- update_satellite_source_and_target!(target_repo)
- compare(target_repo).commits
+ Gitlab::CompareResult.new(compare(target_repo))
end
rescue Grit::Git::CommandFailed => ex
- handle_exception(ex)
+ raise BranchesWithoutParent
end
private
@@ -45,7 +33,11 @@ module Gitlab
end
def compare(repo)
- @compare ||= Gitlab::Git::Compare.new(Gitlab::Git::Repository.new(repo.path), "origin/#{@target_branch}", "source/#{@source_branch}", 10000)
+ @compare ||= Gitlab::Git::Compare.new(
+ Gitlab::Git::Repository.new(repo.path),
+ "origin/#{@target_branch}",
+ "source/#{@source_branch}"
+ )
end
end
end
View Lorry Controller Status