Commit 4cff66a6 authored by blackst0ne's avatar blackst0ne

Add 'squash and rebase' feature to CE

parent 6e354cb6
/*
The squash-before-merge button is EE only, but it's located right in the middle
of the readyToMerge state component template.
If we didn't declare this component in CE, we'd need to maintain a separate copy
of the readyToMergeState template in EE, which is pretty big and likely to change.
Instead, in CE, we declare the component, but it's hidden and is configured to do nothing.
In EE, the configuration extends this object to add a functioning squash-before-merge
button.
*/
<script>
export default {};
import Icon from '~/vue_shared/components/icon.vue';
import eventHub from '~/vue_merge_request_widget/event_hub';
import tooltip from '~/vue_shared/directives/tooltip';
export default {
components: {
Icon,
},
directives: {
tooltip,
},
props: {
mr: {
type: Object,
required: true,
},
isMergeButtonDisabled: {
type: Boolean,
required: true,
},
},
data() {
return {
squashBeforeMerge: this.mr.squash,
};
},
methods: {
updateSquashModel() {
eventHub.$emit('MRWidgetUpdateSquash', this.squashBeforeMerge);
},
},
};
</script>
<template>
<div class="accept-control inline">
<label class="merge-param-checkbox">
<input
type="checkbox"
name="squash"
class="qa-squash-checkbox"
:disabled="isMergeButtonDisabled"
v-model="squashBeforeMerge"
@change="updateSquashModel"
/>
{{ __('Squash commits') }}
</label>
<a
:href="mr.squashBeforeMergeHelpPath"
data-title="About this feature"
data-placement="bottom"
target="_blank"
rel="noopener noreferrer nofollow"
data-container="body"
v-tooltip
>
<icon
name="question-o"
/>
</a>
</div>
</template>
......@@ -6,11 +6,13 @@ import MergeRequest from '../../../merge_request';
import Flash from '../../../flash';
import statusIcon from '../mr_widget_status_icon.vue';
import eventHub from '../../event_hub';
import SquashBeforeMerge from './mr_widget_squash_before_merge.vue';
export default {
name: 'ReadyToMerge',
components: {
statusIcon,
'squash-before-merge': SquashBeforeMerge,
},
props: {
mr: { type: Object, required: true },
......@@ -101,6 +103,12 @@ export default {
return enableSquashBeforeMerge && commitsCount > 1;
},
},
created() {
eventHub.$on('MRWidgetUpdateSquash', this.handleUpdateSquash);
},
beforeDestroy() {
eventHub.$off('MRWidgetUpdateSquash', this.handleUpdateSquash);
},
methods: {
shouldShowMergeControls() {
return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText;
......@@ -128,13 +136,9 @@ export default {
commit_message: this.commitMessage,
merge_when_pipeline_succeeds: this.setToMergeWhenPipelineSucceeds,
should_remove_source_branch: this.removeSourceBranch === true,
squash: this.mr.squash,
};
// Only truthy in EE extension of this component
if (this.setAdditionalParams) {
this.setAdditionalParams(options);
}
this.isMakingRequest = true;
this.service.merge(options)
.then(res => res.data)
......@@ -154,6 +158,9 @@ export default {
new Flash('Something went wrong. Please try again.'); // eslint-disable-line
});
},
handleUpdateSquash(val) {
this.mr.squash = val;
},
initiateMergePolling() {
simplePoll((continuePolling, stopPolling) => {
this.handleMergePolling(continuePolling, stopPolling);
......
......@@ -15,6 +15,11 @@ export default class MergeRequestStore {
const currentUser = data.current_user;
const pipelineStatus = data.pipeline ? data.pipeline.details.status : null;
this.squash = data.squash;
this.squashBeforeMergeHelpPath = this.squashBeforeMergeHelpPath ||
data.squash_before_merge_help_path;
this.enableSquashBeforeMerge = this.enableSquashBeforeMerge || true;
this.title = data.title;
this.targetBranch = data.target_branch;
this.sourceBranch = data.source_branch;
......
......@@ -24,6 +24,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
:source_branch,
:source_project_id,
:state_event,
:squash,
:target_branch,
:target_project_id,
:task_num,
......
......@@ -253,7 +253,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end
def merge_params_attributes
[:should_remove_source_branch, :commit_message]
[:should_remove_source_branch, :commit_message, :squash]
end
def merge_when_pipeline_succeeds_active?
......@@ -282,7 +282,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
return :sha_mismatch if params[:sha] != @merge_request.diff_head_sha
@merge_request.update(merge_error: nil)
@merge_request.update(merge_error: nil, squash: merge_params.fetch(:squash, false))
if params[:merge_when_pipeline_succeeds].present?
return :failed unless @merge_request.actual_head_pipeline
......
......@@ -97,8 +97,9 @@ module MergeRequestsHelper
{
merge_when_pipeline_succeeds: true,
should_remove_source_branch: true,
sha: merge_request.diff_head_sha
}.merge(merge_params_ee(merge_request))
sha: merge_request.diff_head_sha,
squash: merge_request.squash
}
end
def tab_link_for(merge_request, tab, options = {}, &block)
......@@ -149,8 +150,4 @@ module MergeRequestsHelper
current_user.fork_of(project)
end
end
def merge_params_ee(merge_request)
{}
end
end
......@@ -1140,4 +1140,11 @@ class MergeRequest < ActiveRecord::Base
maintainer_push_possible? &&
Ability.allowed?(user, :push_code, source_project)
end
def squash_in_progress?
# The source project can be deleted
return false unless source_project
source_project.repository.squash_in_progress?(id)
end
end
......@@ -957,6 +957,14 @@ class Repository
remote_branch: merge_request.target_branch)
end
def squash(user, merge_request)
raw.squash(user, merge_request.id, branch: merge_request.target_branch,
start_sha: merge_request.diff_start_sha,
end_sha: merge_request.diff_head_sha,
author: merge_request.author,
message: merge_request.title)
end
private
# TODO Generice finder, later split this on finders by Ref or Oid
......
......@@ -10,6 +10,7 @@ class MergeRequestWidgetEntity < IssuableEntity
expose :merge_when_pipeline_succeeds
expose :source_branch
expose :source_project_id
expose :squash
expose :target_branch
expose :target_project_id
expose :allow_maintainer_to_push
......
......@@ -34,6 +34,19 @@ module MergeRequests
handle_merge_error(log_message: e.message, save_message_on_model: true)
end
def source
return merge_request.diff_head_sha unless merge_request.squash
squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute(merge_request)
case squash_result[:status]
when :success
squash_result[:squash_sha]
when :error
raise ::MergeRequests::MergeService::MergeError, squash_result[:message]
end
end
private
def error_check!
......@@ -116,9 +129,5 @@ module MergeRequests
def merge_request_info
merge_request.to_reference(full: true)
end
def source
@source ||= @merge_request.diff_head_sha
end
end
end
module MergeRequests
class SquashService < MergeRequests::WorkingCopyBaseService
def execute(merge_request)
@merge_request = merge_request
@repository = target_project.repository
squash || error('Failed to squash. Should be done manually.')
end
def squash
if merge_request.commits_count < 2
return success(squash_sha: merge_request.diff_head_sha)
end
if merge_request.squash_in_progress?
return error('Squash task canceled: another squash is already in progress.')
end
squash_sha = repository.squash(current_user, merge_request)
success(squash_sha: squash_sha)
rescue => e
log_error("Failed to squash merge request #{merge_request.to_reference(full: true)}:")
log_error(e.message)
false
end
end
end
......@@ -20,6 +20,8 @@
window.gl = window.gl || {};
window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget')}
window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
#js-vue-mr-widget.mr-widget
.content-block.content-block-small.emoji-list-container.js-noteable-awards
......
......@@ -15,3 +15,12 @@
= hidden_field_tag 'merge_request[force_remove_source_branch]', '0', id: nil
= check_box_tag 'merge_request[force_remove_source_branch]', '1', issuable.force_remove_source_branch?
Remove source branch when merge request is accepted.
.form-group
.col-sm-10.col-sm-offset-2
.checkbox
= label_tag 'merge_request[squash]' do
= hidden_field_tag 'merge_request[squash]', '0', id: nil
= check_box_tag 'merge_request[squash]', '1', issuable.squash
Squash commits when merge request is accepted.
= link_to 'About this feature', help_page_path('user/project/merge_requests/squash_and_merge')
---
title: Add `Squash and merge` to GitLab Core (CE)
merge_request: 18956
author: "@blackst0ne"
type: added
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddSquashToMergeRequests < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
def up
unless column_exists?(:merge_requests, :squash)
add_column_with_default :merge_requests, :squash, :boolean, default: false, allow_null: false
end
end
def down
remove_column :merge_requests, :squash if column_exists?(:merge_requests, :squash)
end
end
......@@ -1217,6 +1217,7 @@ ActiveRecord::Schema.define(version: 20180521171529) do
t.integer "latest_merge_request_diff_id"
t.string "rebase_commit_sha"
t.boolean "allow_maintainer_to_push"
t.boolean "squash", default: false, null: false
end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
......
......@@ -107,6 +107,7 @@ Parameters:
"changes_count": "1",
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"squash": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"time_stats": {
"time_estimate": 0,
......@@ -226,6 +227,7 @@ Parameters:
"changes_count": "1",
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"squash": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
......@@ -305,6 +307,7 @@ Parameters:
"changes_count": "1",
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"squash": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
......@@ -541,7 +544,8 @@ POST /projects/:id/merge_requests
| `labels` | string | no | Labels for MR as a comma-separated list |
| `milestone_id` | integer | no | The global ID of a milestone |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch |
| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch |
| `squash` | boolean | no | Squash commits into a single commit when merging |
```json
{
......@@ -595,6 +599,7 @@ POST /projects/:id/merge_requests
"changes_count": "1",
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"squash": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"allow_maintainer_to_push": false,
......@@ -627,6 +632,7 @@ PUT /projects/:id/merge_requests/:merge_request_iid
| `description` | string | no | Description of MR |
| `state_event` | string | no | New state (close/reopen) |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| `squash` | boolean | no | Squash commits into a single commit when merging |
| `discussion_locked` | boolean | no | Flag indicating if the merge request's discussion is locked. If the discussion is locked only project members can add, edit or resolve comments. |
| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch |
......@@ -683,6 +689,7 @@ Must include at least one non-required attribute from above.
"changes_count": "1",
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"squash": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"allow_maintainer_to_push": false,
......@@ -790,6 +797,7 @@ Parameters:
"changes_count": "1",
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"squash": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
......@@ -868,6 +876,7 @@ Parameters:
"changes_count": "1",
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"squash": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"time_stats": {
......@@ -1200,6 +1209,7 @@ Example response:
"changes_count": "1",
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"squash": false,
"web_url": "http://example.com/example/example/merge_requests/1"
},
"target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/merge_requests/7",
......
......@@ -29,12 +29,12 @@ With GitLab merge requests, you can:
- Enable [semi-linear history merge requests](#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch
- [Create new merge requests by email](#create-new-merge-requests-by-email)
- Allow maintainers of the target project to push directly to the fork by [allowing edits from maintainers](maintainer_access.md)
- [Squash and merge](squash_and_merge.md) for a cleaner commit history
With **[GitLab Enterprise Edition][ee]**, you can also:
- View the deployment process across projects with [Multi-Project Pipeline Graphs](https://docs.gitlab.com/ee/ci/multi_project_pipeline_graphs.html#multi-project-pipeline-graphs) **[PREMIUM]**
- Request [approvals](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html) from your managers **[STARTER]**
- [Squash and merge](https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html) for a cleaner commit history **[STARTER]**
- Analyze the impact of your changes with [Code Quality reports](https://docs.gitlab.com/ee/user/project/merge_requests/code_quality_diff.html) **[STARTER]**
## Use cases
......@@ -57,7 +57,7 @@ B. Consider you're a web developer writing a webpage for your company's:
1. Your changes are previewed with [Review Apps](../../../ci/review_apps/index.md)
1. You request your web designers for their implementation
1. You request the [approval](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html) from your manager **[STARTER]**
1. Once approved, your merge request is [squashed and merged](https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html), and [deployed to staging with GitLab Pages](https://about.gitlab.com/2016/08/26/ci-deployment-and-environments/) (Squash and Merge is available in GitLab Starter)
1. Once approved, your merge request is [squashed and merged](squash_and_merge.md), and [deployed to staging with GitLab Pages](https://about.gitlab.com/2016/08/26/ci-deployment-and-environments/)
1. Your production team [cherry picks](#cherry-pick-changes) the merge commit into production
## Merge requests per project
......
# Squash and merge
> [Introduced][ee-1024] in [GitLab Starter][ee] 8.17, and in [GitLab CE][ce] [11.0][ce-18956].
Combine all commits of your merge request into one and retain a clean history.
## Overview
Squashing lets you tidy up the commit history of a branch when accepting a merge
request. It applies all of the changes in the merge request as a single commit,
and then merges that commit using the merge method set for the project.
In other words, squashing a merge request turns a long list of commits:
![List of commits from a merge request][mr-commits]
Into a single commit on merge:
![A squashed commit followed by a merge commit][squashed-commit]
The squashed commit's commit message is the merge request title. And note that
the squashed commit is still followed by a merge commit, as the merge
method for this example repository uses a merge commit. Squashing also works
with the fast-forward merge strategy, see
[squashing and fast-forward merge](#squash-and-fast-forward-merge) for more
details.
## Use cases
When working on a feature branch, you sometimes want to commit your current
progress, but don't really care about the commit messages. Those 'work in
progress commits' don't necessarily contain important information and as such
you'd rather not include them in your target branch.
With squash and merge, when the merge request is ready to be merged,
all you have to do is enable squashing before you press merge to join
the commits include in the merge request into a single commit.
This way, the history of your base branch remains clean with
meaningful commit messages and is simpler to [revert] if necessary.
## Enabling squash for a merge request
Anyone who can create or edit a merge request can choose for it to be squashed
on the merge request form:
![Squash commits checkbox on edit form][squash-edit-form]
---
This can then be overridden at the time of accepting the merge request:
![Squash commits checkbox on accept merge request form][squash-mr-widget]
## Commit metadata for squashed commits
The squashed commit has the following metadata:
* Message: the title of the merge request.
* Author: the author of the merge request.
* Committer: the user who initiated the squash.
## Squash and fast-forward merge
When a project has the [fast-forward merge setting enabled][ff-merge], the merge
request must be able to be fast-forwarded without squashing in order to squash
it. This is because squashing is only available when accepting a merge request,
so a merge request may need to be rebased before squashing, even though
squashing can itself be considered equivalent to rebasing.
[ee-1024]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1024
[ce-18956]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18956
[mr-commits]: img/squash_mr_commits.png
[squashed-commit]: img/squash_squashed_commit.png
[squash-edit-form]: img/squash_edit_form.png
[squash-mr-widget]: img/squash_mr_widget.png
[ff-merge]: fast_forward_merge.md#enabling-fast-forward-merges
[ce]: https://about.gitlab.com/products/
[ee]: https://about.gitlab.com/products/
[revert]: revert_changes.md
......@@ -568,6 +568,8 @@ module API
expose :time_stats, using: 'API::Entities::IssuableTimeStats' do |merge_request|
merge_request
end
expose :squash
end
class MergeRequest < MergeRequestBasic
......
......@@ -10,12 +10,6 @@ module API
helpers do
params :optional_params_ee do
end
params :merge_params_ee do
end
def update_merge_request_ee(merge_request)
end
end
def self.update_params_at_least_one_of
......@@ -29,6 +23,7 @@ module API
target_branch
title
discussion_locked
squash
]
end
......@@ -146,6 +141,7 @@ module API
optional :labels, type: String, desc: 'Comma-separated list of label names'
optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging'
optional :allow_maintainer_to_push, type: Boolean, desc: 'Whether a maintainer of the target project can push to the source project'
optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge'
use :optional_params_ee
end
......@@ -308,8 +304,7 @@ module API
optional :merge_when_pipeline_succeeds, type: Boolean,
desc: 'When true, this merge request will be merged when the pipeline succeeds'
optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch'
use :merge_params_ee
optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge'
end
put ':id/merge_requests/:merge_request_iid/merge' do
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42317')
......@@ -327,7 +322,7 @@ module API
check_sha_param!(params, merge_request)
update_merge_request_ee(merge_request)
merge_request.update(squash: params[:squash]) if params[:squash]
merge_params = {
commit_message: params[:merge_commit_message],
......
......@@ -134,6 +134,8 @@ module API
expose :should_remove_source_branch?, as: :should_remove_source_branch
expose :force_remove_source_branch?, as: :force_remove_source_branch
expose :squash
expose :web_url do |merge_request, options|
Gitlab::UrlBuilder.build(merge_request)
end
......
......@@ -44,6 +44,7 @@ module API
optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request'
optional :labels, type: String, desc: 'Comma-separated list of label names'
optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging'
optional :squash, type: Boolean, desc: 'Squash commits when merging'
end
end
......@@ -166,7 +167,7 @@ module API
use :optional_params
at_least_one_of :title, :target_branch, :description, :assignee_id,
:milestone_id, :labels, :state_event,
:remove_source_branch
:remove_source_branch, :squash
end
put path do
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42127')
......@@ -195,6 +196,7 @@ module API
optional :merge_when_build_succeeds, type: Boolean,
desc: 'When true, this merge request will be merged when the build succeeds'
optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch'
optional :squash, type: Boolean, desc: 'When true, the commits will be squashed into a single commit on merge'
end
put "#{path}/merge" do
merge_request = find_project_merge_request(params[:merge_request_id])
......@@ -211,6 +213,8 @@ module API
render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409)
end
merge_request.update(squash: params[:squash]) if params[:squash]
merge_params = {
commit_message: params[:merge_commit_message],
should_remove_source_branch: params[:should_remove_source_branch]
......
......@@ -16,6 +16,10 @@ module QA
element :no_fast_forward_message, 'Fast-forward merge is not possible'
end
view 'app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.vue' do
element :squash_checkbox
end
def rebase!
click_element :mr_rebase_button
......@@ -41,6 +45,14 @@ module QA
has_text?('The changes were merged into')
end
end
def mark_to_squash
wait(reload: true) do
has_css?(element_selector_css(:squash_checkbox))
end
click_element :squash_checkbox
end
end
end
end
......
module QA
feature 'merge request squash commits', :core do
scenario 'when squash commits is marked before merge' do
Runtime::Browser.visit(:gitlab, Page::Main::Login)
Page::Main::Login.act { sign_in_using_credentials }
project = Factory::Resource::Project.fabricate! do |project|
project.name = "squash-before-merge"
end
merge_request = Factory::Resource::MergeRequest.fabricate! do |merge_request|
merge_request.project = project
merge_request.title = 'Squashing commits'
end
Factory::Repository::Push.fabricate! do |push|
push.project = project
push.commit_message = 'to be squashed'
push.branch_name = merge_request.source_branch
push.new_branch = false
push.file_name = 'other.txt'
push.file_content = "Test with unicode characters ❤✓€❄"
end
merge_request.visit!
Page::MergeRequest::Show.perform do |merge_request_page|
merge_request_page.mark_to_squash
merge_request_page.merge!
merge_request.project.visit!