Commit bea926f9 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'ff-merge-when-build-succeeds' into 'master'

Fast-forward merge when build succeeds + semi-linear history rebasing and merge commits.

Fixes https://gitlab.com/gitlab-org/gitlab-ee/issues/28.

I'll backport 1d6c271e and 9292d8e6 to CE.

To do:

- [x] Test
- [x] More screenshots :)

## Settings

Old:

![Screen_Shot_2016-01-07_at_16.56.42](/uploads/b5a8db9d761a5442d8787ea4913f6f93/Screen_Shot_2016-01-07_at_16.56.42.png)

(The "Rebase button" option used to only be available when "Only fast-forward merges" is checked)

New:

![Screen_Shot_2016-01-07_at_16.57.57](/uploads/1a000cc8bd02aa9e5b053cb52ed8c3c8/Screen_Shot_2016-01-07_at_16.57.57.png)


## Merge widget

### With "Fast-forward merge" or "Merge commit with semi-linear history", when ff merge is NOT possible

![Screen_Shot_2016-01-12_at_16.21.16](/uploads/e3e1ec924b9783b01054fea1446c8235/Screen_Shot_2016-01-12_at_16.21.16.png)

### After "Rebase" button is clicked

![Screen_Shot_2016-01-12_at_16.24.39](/uploads/99619afd73df5850b5620734875add05/Screen_Shot_2016-01-12_at_16.24.39.png)

### When MR is viewed during rebasing

![Screen_Shot_2016-01-12_at_16.24.42](/uploads/80b6d314880c80530810e029988dd216/Screen_Shot_2016-01-12_at_16.24.42.png)

### With "Fast-forward merge", when ff merge is possible

![Screen_Shot_2016-01-12_at_16.19.26](/uploads/0b9e3b82ace27149c9c337b09d519396/Screen_Shot_2016-01-12_at_16.19.26.png)

### With "Merge commit" or "Merge commit with semi-linear history", when ff merge is possible

![Screen_Shot_2016-01-12_at_16.20.07](/uploads/30589140b304db55bc58c2c7ae9e1c90/Screen_Shot_2016-01-12_at_16.20.07.png)

See merge request !110
parents 642bf454 504f19f9
......@@ -3,6 +3,7 @@ v 8.4.0 (unreleased)
- Fix "Commit was rejected by git hook", when max_file_size was set null in project's Git hooks
- Fix "Approvals are not reset after a new push is made if the request is coming from a fork"
- Fix "User is not automatically removed from suggested approvers list if user is deleted"
- Add option to enforce a semi-linear history by only allowing merge requests to be merged that have been rebased.
v 8.3.2
- No EE-specific changes
......@@ -14,9 +15,9 @@ v 8.3.0
- License information can now be retrieved via the API
- Show Kerberos clone url when Kerberos enabled and url different than HTTP url (Borja Aparicio)
- Fix bug with negative approvals required
- Add group contribution analytics page
- Add group contribution analytics page
- Add GitLab Pages
- Add group contribution statistics page
- Add group contribution statistics page
- Automatically import Kerberos identities from Active Directory when Kerberos is enabled (Alex Lossent)
- Canonicalization of Kerberos identities to always include realm (Alex Lossent)
......@@ -60,7 +61,7 @@ v 8.1.1
v 8.1.0
- Add documentation for "Share project with group" API call
- Added an issues template (Hannes Rosenögger)
- Add documentation for "Share project with group" API call
- Add documentation for "Share project with group" API call
- Ability to disable 'Share with Group' feature (via UI and API)
v 8.0.6
......
......@@ -23,7 +23,7 @@ class @MergeRequestWidget
$('.mr-widget-body').html("<h4>" + data.merge_error + "</h4>")
else
callback = -> merge_request_widget.mergeInProgress(deleteSourceBranch)
setTimeout(callback, 2000)
setTimeout(callback, 1000)
dataType: 'json'
rebaseInProgress: ->
......@@ -31,9 +31,8 @@ class @MergeRequestWidget
type: 'GET'
url: $('.merge-request').data('url')
success: (data) =>
debugger
if data["rebase_in_progress?"]
setTimeout(merge_request_widget.rebaseInProgress, 2000)
setTimeout(merge_request_widget.rebaseInProgress, 1000)
else
location.reload()
dataType: 'json'
......
......@@ -2,7 +2,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
before_action :module_enabled
before_action :merge_request, only: [
:edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check,
:ci_status, :toggle_subscription, :approve, :ff_merge, :rebase, :cancel_merge_when_build_succeeds
:ci_status, :toggle_subscription, :approve, :rebase, :cancel_merge_when_build_succeeds
]
before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits, :builds]
before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds]
......@@ -191,6 +191,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end
end
def rebase
return access_denied! unless @merge_request.can_be_merged_by?(current_user)
return render_404 unless @merge_request.approved?
RebaseWorker.perform_async(@merge_request.id, current_user.id)
end
def branch_from
#This is always source
@source_project = @merge_request.nil? ? @project : @merge_request.source_project
......@@ -248,26 +255,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
redirect_to merge_request_path(@merge_request)
end
def ff_merge
return access_denied! unless @merge_request.can_be_merged_by?(current_user)
return render_404 unless @merge_request.approved?
if @merge_request.ff_merge_possible?
MergeRequests::FfMergeService.new(merge_request.target_project, current_user).
execute(merge_request)
@status = true
else
@status = false
end
end
def rebase
return access_denied! unless @merge_request.can_be_merged_by?(current_user)
return render_404 unless @merge_request.approved?
RebaseWorker.perform_async(@merge_request.id, current_user.id)
end
protected
def selected_target_project
......
......@@ -256,8 +256,7 @@ class ProjectsController < ApplicationController
:approvals_before_merge,
:approver_ids,
:issues_template,
:merge_requests_ff_only_enabled,
:merge_requests_rebase_enabled,
:merge_method,
:merge_requests_template,
:mirror,
:mirror_user_id,
......
......@@ -14,14 +14,4 @@ module BranchesHelper
::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(branch_name)
end
def can_rebase?(project, branch_name)
if project.protected_branch? branch_name
can?(current_user, :push_code_to_protected_branches, project)
elsif can?(current_user, :push_code, project)
true
else
false
end
end
end
......@@ -261,7 +261,7 @@ class MergeRequest < ActiveRecord::Base
check_if_can_be_merged
can_be_merged?
can_be_merged? && !must_be_rebased?
end
def gitlab_merge_status
......@@ -577,6 +577,10 @@ class MergeRequest < ActiveRecord::Base
target_sha == source_sha_parent
end
def must_be_rebased?
self.project.ff_merge_must_be_possible? && !ff_merge_possible?
end
def rebase_dir_path
File.join(Gitlab.config.shared.path, 'tmp/rebase', source_project.id.to_s, id.to_s).to_s
end
......
......@@ -1057,4 +1057,32 @@ class Project < ActiveRecord::Base
PagesWorker.perform_in(5.minutes, :remove, namespace.path, temp_path)
end
end
def merge_method
if self.merge_requests_ff_only_enabled
:ff
elsif self.merge_requests_rebase_enabled
:rebase_merge
else
:merge
end
end
def merge_method=(method)
case method.to_s
when "ff"
self.merge_requests_ff_only_enabled = true
self.merge_requests_rebase_enabled = true
when "rebase_merge"
self.merge_requests_ff_only_enabled = false
self.merge_requests_rebase_enabled = true
when "merge"
self.merge_requests_ff_only_enabled = false
self.merge_requests_rebase_enabled = false
end
end
def ff_merge_must_be_possible?
self.merge_requests_ff_only_enabled || self.merge_requests_rebase_enabled
end
end
......@@ -94,7 +94,7 @@ class GitPushService
Issues::CloseService.new(project, authors[commit], {}).execute(issue, commit)
end
end
commit.create_cross_references!(authors[commit], closed_issues)
end
end
......
module MergeRequests
# MergeService class
#
# Do git merge and in case of success
# Do git fast-forward merge and in case of success
# mark merge request as merged and execute all hooks and notifications
# Executed when you do merge via GitLab UI
# Executed when you do fast-forward merge via GitLab UI
#
class FfMergeService < MergeRequests::BaseService
attr_reader :merge_request
def execute(merge_request)
@merge_request = merge_request
unless @merge_request.ff_merge_possible?
return error('Merge request is not mergeable')
end
merge_request.in_locked_state do
if update_head
after_merge
success
else
error('Can not merge changes')
end
end
end
class FfMergeService < MergeRequests::MergeService
private
def update_head
def commit
repository.ff_merge(current_user, merge_request.source_sha, merge_request.target_branch)
end
def after_merge
MergeRequests::PostMergeService.new(project, current_user).execute(merge_request)
end
end
end
......@@ -9,6 +9,11 @@ module MergeRequests
attr_reader :merge_request
def execute(merge_request)
if @project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService)
FfMergeService.new(project, current_user, params).execute(merge_request)
return
end
@merge_request = merge_request
return error('Merge request is not mergeable') unless @merge_request.mergeable?
......@@ -44,7 +49,7 @@ module MergeRequests
def after_merge
MergeRequests::PostMergeService.new(project, current_user).execute(merge_request)
if params[:should_remove_source_branch]
if params[:should_remove_source_branch].present?
DeleteBranchService.new(@merge_request.source_project, current_user).
execute(merge_request.source_branch)
end
......
......@@ -3,22 +3,39 @@
Merge requests:
.form-group
.col-sm-offset-2.col-sm-10
.checkbox
= f.label :merge_requests_ff_only_enabled do
= f.check_box :merge_requests_ff_only_enabled
%strong Only fast-forward merging
= label_tag :merge_method_merge, class: 'control-label' do
Merge method
.col-sm-10
.radio
= label_tag :project_merge_method_merge do
= f.radio_button :merge_method, :merge, class: "js-merge-method-radio"
%strong Merge commit
%br
%span.descr The accept merge request button will only show when a merge without a merge commit is possible.
%span.descr
A merge commit is created for every merge, and merging is allowed as long as there are no conflicts.
.form-group.rebase-feature
.col-sm-offset-2.col-sm-10
.checkbox
= f.label :merge_requests_rebase_enabled do
= f.check_box :merge_requests_rebase_enabled
%strong Rebase button
.radio
= label_tag :project_merge_method_rebase_merge do
= f.radio_button :merge_method, :rebase_merge, class: "js-merge-method-radio"
%strong Merge commit with semi-linear history
%br
%span.descr
A merge commit is created for every merge, but merging is only allowed if the branch has been rebased.
This way you get a history that reads linearly (as with fast-forward merges), with the addition of merge commits.
%br
%span.descr Allows rebasing of merge requests before fast-forward merge.
%span.descr
When the branch has not been rebased, the user is given the option to do so.
.radio
= label_tag :project_merge_method_ff do
= f.radio_button :merge_method, :ff, class: "js-merge-method-radio"
%strong Fast-forward merge
%br
%span.descr
No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch has been rebased.
%br
%span.descr
When the branch has not been rebased, the user is given the option to do so.
.form-group
= f.label :merge_requests_template, class: 'control-label' do
......@@ -67,14 +84,5 @@
%li There are no approvers
:coffeescript
new UsersSelect()
mergeRequestsRebaseVisibilityCheck = ->
is_rebase_enabled = $("input#project_merge_requests_ff_only_enabled").prop("checked")
$(".rebase-feature").toggle(is_rebase_enabled)
mergeRequestsRebaseVisibilityCheck()
$("input#project_merge_requests_ff_only_enabled").change ->
mergeRequestsRebaseVisibilityCheck()
:javascript
new UsersSelect();
- if @status
:plain
location.reload()
- else
:plain
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/reload'))}");
:plain
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/ff_accept'))}")
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/rebase', rebase_in_progress: true))}")
merge_request_widget.rebaseInProgress();
......@@ -19,9 +19,9 @@
= render 'projects/merge_requests/widget/open/merge_when_build_succeeds'
- elsif !@merge_request.can_be_merged_by?(current_user)
= render 'projects/merge_requests/widget/open/not_allowed'
- elsif @project.merge_requests_ff_only_enabled
= render 'projects/merge_requests/widget/open/ff_accept'
- elsif @merge_request.can_be_merged?
- elsif @merge_request.must_be_rebased?
= render 'projects/merge_requests/widget/open/rebase'
- else
= render 'projects/merge_requests/widget/open/accept'
- if @closes_issues.present?
......
......@@ -30,14 +30,20 @@
= label_tag :should_remove_source_branch, class: "remove_source_checkbox" do
= check_box_tag :should_remove_source_branch
Remove source branch
.accept-control.right
= link_to "#", class: "modify-merge-commit-link js-toggle-button" do
= icon('edit')
Modify commit message
.js-toggle-content.hide.prepend-top-default
= render 'shared/commit_message_container', params: params,
text: @merge_request.merge_commit_message,
rows: 14, hint: true
- if @project.merge_requests_ff_only_enabled
Fast-forward merge without a merge commit
- else
= link_to "#", class: "modify-merge-commit-link js-toggle-button" do
= icon('edit')
Modify commit message
- unless @project.merge_requests_ff_only_enabled
.js-toggle-content.hide.prepend-top-default
= render 'shared/commit_message_container', params: params,
text: @merge_request.merge_commit_message,
rows: 14, hint: true
= hidden_field_tag :merge_when_build_succeeds, "", autocomplete: "off"
......
......@@ -3,7 +3,7 @@
This merge request contains merge conflicts
%p
Please resolve these conflicts or
Please resolve these conflicts or
- if @merge_request.can_be_merged_by?(current_user)
#{link_to "merge this request manually", "#modal_merge_info", class: "how_to_merge_link vlink", "data-toggle" => "modal"}.
- else
......
- if @merge_request.ff_merge_possible?
= form_for [:ff_merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-requires-input' } do |f|
= hidden_field_tag :authenticity_token, form_authenticity_token
.accept-merge-holder.clearfix.js-toggle-container
.accept-action
= f.button class: "btn btn-create accept-mr" do
Accept Merge Request
.accept-control
Fast-forward merge without creating merge commit
- else
= form_for [:rebase, @project.namespace.becomes(Namespace), @project, @merge_request],
remote: true, method: :post, html: { class: 'accept-mr-form js-requires-input' } do |f|
= hidden_field_tag :authenticity_token, form_authenticity_token
.accept-merge-holder.clearfix.js-toggle-container
- if @merge_request.target_project.merge_requests_rebase_enabled && can_rebase?(@merge_request.source_project, @merge_request.source_branch)
- if @merge_request.rebase_in_progress?
%h4 Rebase in progress... It can take a while. Reload at will.
- else
.accept-action
= f.button class: "btn btn-reopen rebase-mr" do
Rebase onto #{@merge_request.target_branch}
.accept-control
Fast-forward merge is not possible. Branch must be rebased first
:coffeescript
$('.accept-mr-form').on 'ajax:before', ->
btn = $('.accept-mr')
btn.disable()
btn.html("<i class='fa fa-spinner fa-spin'></i> Merge in progress")
$('.rebase-mr-form').on 'ajax:before', ->
btn = $('.rebase-mr')
btn.disable()
btn.html("<i class='fa fa-spinner fa-spin'></i> Rebase in progress. It could take some time")
- if #{@merge_request.rebase_in_progress?}
$ ->
merge_request_widget.rebaseInProgress()
......@@ -5,7 +5,10 @@
- should_remove_source_branch = @merge_request.merge_params["should_remove_source_branch"].present?
%p
= succeed '.' do
The changes will be merged into
- if @project.merge_requests_ff_only_enabled
The changes will be fast-forward merged into
- else
The changes will be merged into
%span.label-branch= @merge_request.target_branch
- if should_remove_source_branch
The source branch will be removed.
......
- if @merge_request.rebase_in_progress? || (defined?(rebase_in_progress) && rebase_in_progress)
%h4
= icon("spinner spin")
Rebase in progress&hellip;
%p
This merge request is in the process of being rebased.
:javascript
$(function() {
merge_request_widget.rebaseInProgress()
});
- elsif !can_push_branch?(@merge_request.source_project, @merge_request.source_branch)
%h4
= icon("exclamation-triangle")
Fast-forward merge is not possible
%p
Rebase the source branch onto
%span.label-branch= @merge_request.target_branch
to allow this merge request to be merged.
- else
= form_for [:rebase, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'rebase-mr-form js-requires-input' } do |f|
= hidden_field_tag :authenticity_token, form_authenticity_token
.accept-merge-holder.clearfix.js-toggle-container
.accept-action
= f.button class: "btn btn-reopen js-rebase-button" do
Rebase onto #{@merge_request.target_branch}
.accept-control
Fast-forward merge is not possible. Rebase the source branch onto the target branch to allow this merge request to be merged.
:javascript
$('.rebase-mr-form').on('ajax:send', function() {
$('.rebase-mr-form :input').disable();
});
$('.js-rebase-button').on('click', function() {
$('.js-rebase-button').html("<i class='fa fa-spinner fa-spin'></i> Rebase in progress");
});
......@@ -622,7 +622,6 @@ Rails.application.routes.draw do
post :toggle_subscription
post :approve
post :rebase
post :ff_merge
end
collection do
......
......@@ -3,23 +3,30 @@ Feature: Project Ff Merge Requests
Given I sign in as a user
And I own project "Shop"
And project "Shop" have "Bug NS-05" open merge request with diffs inside
And ff merge enabled
And merge request "Bug NS-05" is mergeable
@javascript
Scenario: I do ff-only merge
Given merge request "Bug NS-05" is rebased
Given ff merge enabled
And merge request "Bug NS-05" is rebased
When I visit merge request page "Bug NS-05"
Then I should see ff-only merge button
When I accept this merge request
Then I should see merged request
@javascript
Scenario: I do rebase before ff-only merge
Given rebase before merge enabled
Given ff merge enabled
And rebase before merge enabled
When I visit merge request page "Bug NS-05"
Then I should see rebase button
When I press rebase button
Then I should see rebase in progress message
Scenario: I should do rebase before ff-only merge
@javascript
Scenario: I do rebase before regular merge
Given rebase before merge enabled
When I visit merge request page "Bug NS-05"
Then I should not see rebase button
And I should see rebase message
Then I should see rebase button
When I press rebase button
Then I should see rebase in progress message
......@@ -21,7 +21,7 @@ class Spinach::Features::ProjectFfMergeRequests < Spinach::FeatureSteps
end
step 'I should see ff-only merge button' do
expect(page).to have_content "Fast-forward merge without creating merge commit"
expect(page).to have_content "Fast-forward merge without a merge commit"
expect(page).to have_button 'Accept Merge Request'
end
......@@ -36,7 +36,7 @@ class Spinach::Features::ProjectFfMergeRequests < Spinach::FeatureSteps
end
step 'I should see merged request' do
page.within '.issue-box' do
page.within '.status-box' do
expect(page).to have_content "Merged"
end
end
......@@ -47,18 +47,10 @@ class Spinach::Features::ProjectFfMergeRequests < Spinach::FeatureSteps
project.save!
end
step 'I should not see rebase button' do
expect(page).to_not have_button "Rebase"
end
step 'I should see rebase button' do
expect(page).to have_button "Rebase"
end
step 'I should see rebase message' do
expect(page).to have_content "Fast-forward merge is not possible. Branch must be rebased first"
end
step 'merge request "Bug NS-05" is rebased' do
merge_request.source_branch = 'flatten-dir'
merge_request.target_branch = 'improve/awesome'
......@@ -73,7 +65,6 @@ class Spinach::Features::ProjectFfMergeRequests < Spinach::FeatureSteps
end
step 'I press rebase button' do
allow(RebaseWorker).to receive(:perform_async){ true }
click_button "Rebase"
end
......
require 'banzai'
module Banzai
# Common methods for ReferenceFilters that support an optional cross-project
# reference.
......
require 'active_support/core_ext/string/output_safety'
require 'banzai'
module Banzai
module Filter
......
require 'banzai'
module Banzai
module Filter
# Issues, Merge Requests, Snippets, Commits and Commit Ranges share
......
require 'banzai'
require 'html/pipeline/filter'
require 'uri'
......
require 'banzai'
module Banzai
module Filter
# HTML filter that replaces commit range references with links.
......
require 'banzai'
module Banzai
module Filter
# HTML filter that replaces commit references with links.
......
require 'action_controller'
require 'banzai'
require 'gitlab_emoji'
require 'html/pipeline/filter'
......
require 'banzai'
module Banzai
module Filter
# HTML filter that replaces external issue tracker references with links.
......
require 'banzai'
require 'html/pipeline/filter'
module Banzai
......
require 'banzai'
module Banzai
module Filter
# HTML filter that replaces issue references with links. References to
......
require 'banzai'
module Banzai
module Filter
# HTML filter that replaces label references with links.
......
require 'banzai'
require 'html/pipeline/filter'
module Banzai
......
require 'banzai'
module Banzai
module Filter
# HTML filter that replaces merge request references with links. References
......
require 'banzai'
require 'html/pipeline/filter'
module Banzai
......
require 'active_support/core_ext/string/output_safety'
require 'banzai'
require 'html/pipeline/filter'
module Banzai
......
require 'banzai'
require 'html/pipeline/filter'
module Banzai
......
require 'banzai'
require 'html/pipeline/filter'
require 'uri'
......
require 'banzai'
require 'html/pipeline/filter'
require 'html/pipeline/sanitization_filter'
......
require 'banzai'
module Banzai
module Filter
# HTML filter that replaces snippet references with links. References to
......
require 'banzai'
require 'html/pipeline/filter'
require 'rouge/plugins/redcarpet'
......
require 'banzai'
require 'html/pipeline/filter'
module Banzai
......
require 'banzai'
require 'task_list/filter'
module Banzai
......
require 'banzai'
require 'html/pipeline/filter'
require 'uri'
......
require 'banzai'
module Banzai
module Filter
# HTML filter that replaces user or group references with links.
......
require 'banzai'
module Banzai
class LazyReference
def self.load(refs)
......
require 'banzai'
module Banzai
module Pipeline
def self.[](name)
......
require 'banzai'
module Banzai
module Pipeline
class AsciidocPipeline < BasePipeline
......
require 'banzai'
module Banzai
module Pipeline
class AtomPipeline < FullPipeline
......
require 'banzai'
require 'html/pipeline'
module Banzai
......
require 'banzai'
module Banzai
module Pipeline
module CombinedPipeline
......
require 'banzai'
module Banzai
module Pipeline
class DescriptionPipeline < FullPipeline
......
require 'banzai'
module Banzai
module Pipeline
class EmailPipeline < FullPipeline
......
require 'banzai'
module Banzai
module Pipeline
class FullPipeline < CombinedPipeline.new(PlainMarkdownPipeline, GfmPipeline)
......
require 'banzai'
module Banzai
module Pipeline
class GfmPipeline < BasePipeline
......
require 'banzai'
module Banzai
module Pipeline
class NotePipeline < FullPipeline
......
require 'banzai'
module Banzai
module Pipeline
class PlainMarkdownPipeline < BasePipeline
......
require 'banzai'
module Banzai
module Pipeline
class PostProcessPipeline < BasePipeline
......
require 'banzai'
module Banzai
module Pipeline
class ReferenceExtractionPipeline < BasePipeline
......
require 'banzai'
module Banzai
module Pipeline
class SingleLinePipeline < GfmPipeline
......
require 'banzai'
module Banzai
# Extract possible GFM references from an arbitrary String for further processing.
class ReferenceExtractor
......
require 'banzai'
module Gitlab
module Markdown
class Pipeline
......
require 'banzai'
module Gitlab
# Extract possible GFM references from an arbitrary String for further processing.
class ReferenceExtractor < Banzai::ReferenceExtractor
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment