Commit 3dea71ce0f8e78d7022ea25312af7c812c777d79
Exists in
spb-stable
and in
2 other branches
Merge branch 'satellite-diff-collector' into 'master'
Add CompareAction class for collecting commits and diffs using satellites
Showing
2 changed files
with
67 additions
and
2 deletions
Show diff stats
app/models/merge_request_diff.rb
| @@ -86,7 +86,7 @@ class MergeRequestDiff < ActiveRecord::Base | @@ -86,7 +86,7 @@ class MergeRequestDiff < ActiveRecord::Base | ||
| 86 | # between target and source branches | 86 | # between target and source branches |
| 87 | def unmerged_commits | 87 | def unmerged_commits |
| 88 | commits = if merge_request.for_fork? | 88 | commits = if merge_request.for_fork? |
| 89 | - Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).commits_between | 89 | + compare_action.commits |
| 90 | else | 90 | else |
| 91 | repository.commits_between(target_branch, source_branch) | 91 | repository.commits_between(target_branch, source_branch) |
| 92 | end | 92 | end |
| @@ -150,7 +150,7 @@ class MergeRequestDiff < ActiveRecord::Base | @@ -150,7 +150,7 @@ class MergeRequestDiff < ActiveRecord::Base | ||
| 150 | # between target and source branches | 150 | # between target and source branches |
| 151 | def unmerged_diffs | 151 | def unmerged_diffs |
| 152 | diffs = if merge_request.for_fork? | 152 | diffs = if merge_request.for_fork? |
| 153 | - Gitlab::Satellite::MergeAction.new(merge_request.author, merge_request).diffs_between_satellite | 153 | + compare_action.diffs |
| 154 | else | 154 | else |
| 155 | Gitlab::Git::Diff.between(repository, source_branch, target_branch) | 155 | Gitlab::Git::Diff.between(repository, source_branch, target_branch) |
| 156 | end | 156 | end |
| @@ -165,4 +165,16 @@ class MergeRequestDiff < ActiveRecord::Base | @@ -165,4 +165,16 @@ class MergeRequestDiff < ActiveRecord::Base | ||
| 165 | def repository | 165 | def repository |
| 166 | merge_request.target_project.repository | 166 | merge_request.target_project.repository |
| 167 | end | 167 | end |
| 168 | + | ||
| 169 | + private | ||
| 170 | + | ||
| 171 | + def compare_action | ||
| 172 | + Gitlab::Satellite::CompareAction.new( | ||
| 173 | + merge_request.author, | ||
| 174 | + merge_request.target_project, | ||
| 175 | + merge_request.target_branch, | ||
| 176 | + merge_request.source_project, | ||
| 177 | + merge_request.source_branch | ||
| 178 | + ) | ||
| 179 | + end | ||
| 168 | end | 180 | end |
| @@ -0,0 +1,53 @@ | @@ -0,0 +1,53 @@ | ||
| 1 | +module Gitlab | ||
| 2 | + module Satellite | ||
| 3 | + class CompareAction < Action | ||
| 4 | + def initialize(user, target_project, target_branch, source_project, source_branch) | ||
| 5 | + super user, target_project | ||
| 6 | + | ||
| 7 | + @target_project, @target_branch = target_project, target_branch | ||
| 8 | + @source_project, @source_branch = source_project, source_branch | ||
| 9 | + end | ||
| 10 | + | ||
| 11 | + # Only show what is new in the source branch compared to the target branch, not the other way around. | ||
| 12 | + # The line below with merge_base is equivalent to diff with three dots (git diff branch1...branch2) | ||
| 13 | + # From the git documentation: "git diff A...B" is equivalent to "git diff $(git-merge-base A B) B" | ||
| 14 | + def diffs | ||
| 15 | + in_locked_and_timed_satellite do |target_repo| | ||
| 16 | + prepare_satellite!(target_repo) | ||
| 17 | + update_satellite_source_and_target!(target_repo) | ||
| 18 | + common_commit = target_repo.git.native(:merge_base, default_options, ["origin/#{@target_branch}", "source/#{@source_branch}"]).strip | ||
| 19 | + #this method doesn't take default options | ||
| 20 | + diffs = target_repo.diff(common_commit, "source/#{@source_branch}") | ||
| 21 | + diffs = diffs.map { |diff| Gitlab::Git::Diff.new(diff) } | ||
| 22 | + diffs | ||
| 23 | + end | ||
| 24 | + rescue Grit::Git::CommandFailed => ex | ||
| 25 | + handle_exception(ex) | ||
| 26 | + end | ||
| 27 | + | ||
| 28 | + # Retrieve an array of commits between the source and the target | ||
| 29 | + def commits | ||
| 30 | + in_locked_and_timed_satellite do |target_repo| | ||
| 31 | + prepare_satellite!(target_repo) | ||
| 32 | + update_satellite_source_and_target!(target_repo) | ||
| 33 | + commits = target_repo.commits_between("origin/#{@target_branch}", "source/#{@source_branch}") | ||
| 34 | + commits = commits.map { |commit| Gitlab::Git::Commit.new(commit, nil) } | ||
| 35 | + commits | ||
| 36 | + end | ||
| 37 | + rescue Grit::Git::CommandFailed => ex | ||
| 38 | + handle_exception(ex) | ||
| 39 | + end | ||
| 40 | + | ||
| 41 | + private | ||
| 42 | + | ||
| 43 | + # Assumes a satellite exists that is a fresh clone of the projects repo, prepares satellite for diffs | ||
| 44 | + def update_satellite_source_and_target!(target_repo) | ||
| 45 | + target_repo.remote_add('source', @source_project.repository.path_to_repo) | ||
| 46 | + target_repo.remote_fetch('source') | ||
| 47 | + target_repo.git.checkout(default_options({b: true}), @target_branch, "origin/#{@target_branch}") | ||
| 48 | + rescue Grit::Git::CommandFailed => ex | ||
| 49 | + handle_exception(ex) | ||
| 50 | + end | ||
| 51 | + end | ||
| 52 | + end | ||
| 53 | +end |