kotovalexarian-likes-gitlab
/
gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Update merge request's merge_commit for branch update 

Analyze new commits graph to determine each commit's merge commit.

Fix "merged with [commit]" info for merge requests being merged
automatically by other actions.

Allow analyzing upto the relevant commit
This commit is contained in:
Mark Chao 2018-11-03 17:36:19 +08:00
parent a89a73c1cc
commit 1f7647f446
6 changed files with 236 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
app/services/merge_requests/refresh_service.rb
View File

@ -58,13 +58,22 @@ module MergeRequests
.preload(:latest_merge_request_diff)
.where(target_branch: @push.branch_name).to_a
.select(&:diff_head_commit)
.select do |merge_request|
commit_ids.include?(merge_request.diff_head_sha) &&
merge_request.merge_request_diff.state != 'empty'
end
merge_requests = filter_merge_requests(merge_requests)
merge_requests = merge_requests.select do |merge_request|
commit_ids.include?(merge_request.diff_head_sha) &&
merge_request.merge_request_diff.state != 'empty'
end
return if merge_requests.empty?
analyzer = Gitlab::BranchPushMergeCommitAnalyzer.new(
@commits.reverse,
relevant_commit_ids: merge_requests.map(&:diff_head_sha)
)
merge_requests.each do |merge_request|
merge_request.merge_commit_sha = analyzer.get_merge_commit(merge_request.diff_head_sha)
filter_merge_requests(merge_requests).each do |merge_request|
MergeRequests::PostMergeService
.new(merge_request.target_project, @current_user)
.execute(merge_request)

6
changelogs/unreleased/48889-populate-merge_commit_sha.yml Normal file
View File

@ -0,0 +1,6 @@
---
title: Fix "merged with [commit]" info for merge requests being merged automatically
by other actions
merge_request: 22794
author:
type: fixed

121
lib/gitlab/branch_push_merge_commit_analyzer.rb Normal file
View File

@ -0,0 +1,121 @@
# frozen_string_literal: true
module Gitlab
# Analyse a graph of commits from a push to a branch,
# for each commit, analyze that if it is the head of a merge request,
# then what should its merge_commit be, relative to the branch.
#
# A----->B----->C----->D target branch
# | ^
# | |
# +-->E----->F--+ merged branch
# | ^
# | |
# +->G--+
#
# (See merge-commit-analyze-after branch in gitlab-test)
#
# Assuming
# - A is already in remote
# - B~D are all in its own branch with its own merge request, targeting the target branch
#
# When D is finally pushed to the target branch,
# what are the merge commits for all the other merge requests?
#
# We can walk backwards from the HEAD commit D,
# and find status of its parents.
# First we determine if commit belongs to the target branch (i.e. A, B, C, D),
# and then determine its merge commit.
#
# +--------+-----------------+--------------+
# | Commit | Direct ancestor | Merge commit |
# +--------+-----------------+--------------+
# | D | Y | D |
# +--------+-----------------+--------------+
# | C | Y | C |
# +--------+-----------------+--------------+
# | F | | C |
# +--------+-----------------+--------------+
# | B | Y | B |
# +--------+-----------------+--------------+
# | E | | C |
# +--------+-----------------+--------------+
# | G | | C |
# +--------+-----------------+--------------+
#
# By examining the result, it can be said that
#
# - If commit is direct ancestor of HEAD, its merge commit is itself.
# - Otherwise, the merge commit is the same as its child's merge commit.
#
class BranchPushMergeCommitAnalyzer
class CommitDecorator < SimpleDelegator
attr_accessor :merge_commit
attr_writer :direct_ancestor # boolean
def direct_ancestor?
@direct_ancestor
end
# @param child_commit [CommitDecorator]
# @param first_parent [Boolean] whether `self` is the first parent of `child_commit`
def set_merge_commit(child_commit, first_parent:)
# If child commit is a direct ancestor, its first parent is also the direct ancestor.
# We assume direct ancestors matches the trail of the target branch over time,
# This assumption is correct most of the time, especially for gitlab managed merges,
# but there are exception cases which can't be solved (https://stackoverflow.com/a/49754723/474597)
@direct_ancestor = first_parent && child_commit.direct_ancestor?
@merge_commit = direct_ancestor? ? self : child_commit.merge_commit
end
end
# @param commits [Array] list of commits, must be ordered from the child (tip) of the graph back to the ancestors
def initialize(commits, relevant_commit_ids: nil)
@commits = commits
@id_to_commit = {}
@commits.each do |commit|
@id_to_commit[commit.id] = CommitDecorator.new(commit)
if relevant_commit_ids
relevant_commit_ids.delete(commit.id)
break if relevant_commit_ids.empty? # Only limit the analyze up to relevant_commit_ids
end
end
analyze
end
def get_merge_commit(id)
get_commit(id).merge_commit.id
end
private
def analyze
head_commit = get_commit(@commits.first.id)
head_commit.direct_ancestor = true
head_commit.merge_commit = head_commit
# Analyzing a commit requires its child commit be analyzed first,
# which is the case here since commits are ordered from child to parent.
@id_to_commit.each_value do |commit|
analyze_parents(commit)
end
end
def analyze_parents(commit)
commit.parent_ids.each.with_index do |parent_commit_id, i|
parent_commit = get_commit(parent_commit_id)
next if parent_commit.nil? # parent commit may not be part of new commits
parent_commit.set_merge_commit(commit, first_parent: i == 0)
end
end
def get_commit(id)
@id_to_commit[id]
end
end
end

43
spec/lib/gitlab/branch_push_merge_commit_analyzer_spec.rb Normal file
View File

@ -0,0 +1,43 @@
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::BranchPushMergeCommitAnalyzer do
let(:project) { create(:project, :repository) }
let(:oldrev) { 'merge-commit-analyze-before' }
let(:newrev) { 'merge-commit-analyze-after' }
let(:commits) { project.repository.commits_between(oldrev, newrev).reverse }
subject { described_class.new(commits) }
describe '#get_merge_commit' do
let(:expected_merge_commits) do
{
'646ece5cfed840eca0a4feb21bcd6a81bb19bda3' => '646ece5cfed840eca0a4feb21bcd6a81bb19bda3',
'29284d9bcc350bcae005872d0be6edd016e2efb5' => '29284d9bcc350bcae005872d0be6edd016e2efb5',
'5f82584f0a907f3b30cfce5bb8df371454a90051' => '29284d9bcc350bcae005872d0be6edd016e2efb5',
'8a994512e8c8f0dfcf22bb16df6e876be7a61036' => '29284d9bcc350bcae005872d0be6edd016e2efb5',
'689600b91aabec706e657e38ea706ece1ee8268f' => '29284d9bcc350bcae005872d0be6edd016e2efb5',
'db46a1c5a5e474aa169b6cdb7a522d891bc4c5f9' => 'db46a1c5a5e474aa169b6cdb7a522d891bc4c5f9'
}
end
it 'returns correct merge commit SHA for each commit' do
expected_merge_commits.each do |commit, merge_commit|
expect(subject.get_merge_commit(commit)).to eq(merge_commit)
end
end
context 'when relevant_commit_ids is provided' do
let(:relevant_commit_id) { '8a994512e8c8f0dfcf22bb16df6e876be7a61036' }
subject { described_class.new(commits, relevant_commit_ids: [relevant_commit_id]) }
it 'returns correct merge commit' do
expected_merge_commits.each do |commit, merge_commit|
subject = described_class.new(commits, relevant_commit_ids: [commit])
expect(subject.get_merge_commit(commit)).to eq(merge_commit)
end
end
end
end
end

49
spec/services/merge_requests/refresh_service_spec.rb
View File

@ -621,4 +621,53 @@ describe MergeRequests::RefreshService do
@fork_build_failed_todo.reload
end
end
describe 'updating merge_commit' do
let(:service) { described_class.new(project, user) }
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:oldrev) { TestEnv::BRANCH_SHA['merge-commit-analyze-before'] }
let(:newrev) { TestEnv::BRANCH_SHA['merge-commit-analyze-after'] } # Pretend branch is now updated
let!(:merge_request) do
create(
:merge_request,
source_project: project,
source_branch: 'merge-commit-analyze-after',
target_branch: 'merge-commit-analyze-before',
target_project: project,
merge_user: user
)
end
let!(:merge_request_side_branch) do
create(
:merge_request,
source_project: project,
source_branch: 'merge-commit-analyze-side-branch',
target_branch: 'merge-commit-analyze-before',
target_project: project,
merge_user: user
)
end
subject { service.execute(oldrev, newrev, 'refs/heads/merge-commit-analyze-before') }
it "updates merge requests' merge_commits" do
expect(Gitlab::BranchPushMergeCommitAnalyzer).to receive(:new).and_wrap_original do |original_method, commits|
expect(commits.map(&:id)).to eq(%w{646ece5cfed840eca0a4feb21bcd6a81bb19bda3 29284d9bcc350bcae005872d0be6edd016e2efb5 5f82584f0a907f3b30cfce5bb8df371454a90051 8a994512e8c8f0dfcf22bb16df6e876be7a61036 689600b91aabec706e657e38ea706ece1ee8268f db46a1c5a5e474aa169b6cdb7a522d891bc4c5f9})
original_method.call(commits)
end
subject
merge_request.reload
merge_request_side_branch.reload
expect(merge_request.merge_commit.id).to eq('646ece5cfed840eca0a4feb21bcd6a81bb19bda3')
expect(merge_request_side_branch.merge_commit.id).to eq('29284d9bcc350bcae005872d0be6edd016e2efb5')
end
end
end

3
spec/support/helpers/test_env.rb
View File

@ -54,6 +54,9 @@ module TestEnv
'add_images_and_changes' => '010d106',
'update-gitlab-shell-v-6-0-1' => '2f61d70',
'update-gitlab-shell-v-6-0-3' => 'de78448',
'merge-commit-analyze-before' => '1adbdef',
'merge-commit-analyze-side-branch' => '8a99451',
'merge-commit-analyze-after' => '646ece5',
'2-mb-file' => 'bf12d25',
'before-create-delete-modify-move' => '845009f',
'between-create-delete-modify-move' => '3f5f443',