Merge branch 'squash' into 'master'

Allow squashing MRs

Closes #150

See merge request !1024

app/assets/stylesheets/pages/merge_requests.scss

@@ -56,15 +56,24 @@
       &.right {
         float: right;
         padding-right: 0;
+      }
 
-        a {
-          color: $gl-text-color;
-        }
+      .modify-merge-commit-link {
+        color: $gl-text-color;
       }
 
-      .remove_source_checkbox {
+      .merge-param-checkbox {
         margin: 0;
       }
+
+      a .fa-question-circle {
+        color: $gl-text-color-secondary;
+
+        &:hover,
+        &:focus {
+          color: $link-hover-color;
+        }
+      }
     }
   }
 

app/controllers/projects/merge_requests_controller.rb

@@ -338,7 +338,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       return
     end
 
-    @merge_request.update(merge_error: nil)
+    @merge_request.update(merge_error: nil, squash: merge_params[:squash])
 
     if params[:merge_when_build_succeeds].present?
       unless @merge_request.head_pipeline
@@ -699,6 +699,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       approvals_before_merge
       approver_group_ids
       approver_ids
+      squash
     ]
   end
 
@@ -716,7 +717,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def merge_params
-    params.permit(:should_remove_source_branch, :commit_message)
+    params.permit(:should_remove_source_branch, :commit_message, :squash)
   end
 
   # Make sure merge requests created before 8.0

app/helpers/merge_requests_helper.rb

@@ -181,4 +181,16 @@ module MergeRequestsHelper
   def different_base?(version1, version2)
     version1 && version2 && version1.base_commit_sha != version2.base_commit_sha
   end
+
+  def merge_params(merge_request)
+    {
+      merge_when_build_succeeds: true,
+      should_remove_source_branch: true,
+      sha: merge_request.diff_head_sha
+    }.merge(merge_params_ee(merge_request))
+  end
+
+  def merge_params_ee(merge_request)
+    { squash: merge_request.squash }
+  end
 end

app/models/merge_request.rb

@@ -812,6 +812,10 @@ class MergeRequest < ActiveRecord::Base
     File.join(Gitlab.config.shared.path, 'tmp/rebase', source_project.id.to_s, id.to_s).to_s
   end
 
+  def squash_dir_path
+    File.join(Gitlab.config.shared.path, 'tmp/squash', source_project.id.to_s, id.to_s).to_s
+  end
+
   def rebase_in_progress?
     # The source project can be deleted
     return false unless source_project
@@ -820,14 +824,26 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def clean_stuck_rebase
-    expiration_time = Time.now - 15.minutes
-
-    if File.new(rebase_dir_path).mtime < expiration_time
+    if File.mtime(rebase_dir_path) < 15.minutes.ago
       FileUtils.rm_rf(rebase_dir_path)
       true
     end
   end
 
+  def squash_in_progress?
+    # The source project can be deleted
+    return false unless source_project
+
+    File.exist?(squash_dir_path) && !clean_stuck_squash
+  end
+
+  def clean_stuck_squash
+    if File.mtime(squash_dir_path) < 15.minutes.ago
+      FileUtils.rm_rf(squash_dir_path)
+      true
+    end
+  end
+
   def diverged_commits_count
     cache = Rails.cache.read(:"merge_request_#{id}_diverged_commits")
 

app/models/repository.rb

@@ -1027,9 +1027,9 @@ class Repository
     end
   end
 
-  def merge(user, merge_request, options = {})
+  def merge(user, source, merge_request, options = {})
     our_commit = rugged.branches[merge_request.target_branch].target
-    their_commit = rugged.lookup(merge_request.diff_head_sha)
+    their_commit = rugged.lookup(source)
 
     raise "Invalid merge target" if our_commit.nil?
     raise "Invalid merge source" if their_commit.nil?

app/services/merge_requests/ff_merge_service.rb

@@ -10,7 +10,7 @@ module MergeRequests
 
     def commit
       repository.ff_merge(current_user,
-                          merge_request.diff_head_sha,
+                          source,
                           merge_request.target_branch,
                           merge_request: merge_request)
     ensure

app/services/merge_requests/merge_service.rb

@@ -6,7 +6,7 @@ module MergeRequests
   # Executed when you do merge via GitLab UI
   #
   class MergeService < MergeRequests::BaseService
-    attr_reader :merge_request
+    attr_reader :merge_request, :source
 
     def execute(merge_request)
       if project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService)
@@ -24,6 +24,10 @@ module MergeRequests
         return error(message)
       end
 
+      @source = find_merge_source
+
+      return log_merge_error('No source for merge', true) unless @source
+
       merge_request.in_locked_state do
         if commit
           after_merge
@@ -64,7 +68,7 @@ module MergeRequests
         committer: committer
       }
 
-      commit_id = repository.merge(current_user, merge_request, options)
+      commit_id = repository.merge(current_user, source, merge_request, options)
 
       if commit_id
         merge_request.update(merge_commit_sha: commit_id)
@@ -103,9 +107,21 @@ module MergeRequests
     end
 
     def merge_request_info
-      project = merge_request.project
+      merge_request.to_reference(full: true)
+    end
+
+    def find_merge_source
+      return merge_request.diff_head_sha unless merge_request.squash
 
-      "#{project.to_reference}#{merge_request.to_reference}"
+      squash_result = SquashService.new(project, current_user, params).execute(merge_request)
+
+      if squash_result[:status] == :success
+        squash_result[:squash_sha]
+      else
+        log_merge_error("Squashing #{merge_request_info} failed")
+
+        nil
+      end
     end
   end
 end

app/services/merge_requests/rebase_service.rb

 module MergeRequests
-  # MergeService class
-  #
-  # Do git 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
-  #
-  class RebaseService < MergeRequests::BaseService
-    include Gitlab::Popen
-
-    attr_reader :merge_request
-
+  class RebaseService < MergeRequests::WorkingCopyBaseService
     def execute(merge_request)
       @merge_request = merge_request
 
@@ -22,92 +12,53 @@ module MergeRequests
 
     def rebase
       if merge_request.rebase_in_progress?
-        log('Rebase task canceled: Another rebase is already in progress')
+        log_error('Rebase task canceled: Another rebase is already in progress')
         return false
       end
 
-      # Clone
-      output, status = popen(
-        %W(git clone -b #{merge_request.source_branch} -- #{source_project.repository.path_to_repo} #{tree_path}),
+      run_git_command(
+        %W(clone -b #{merge_request.source_branch} -- #{source_project.repository.path_to_repo} #{tree_path}),
         nil,
-        git_env
+        git_env,
+        'clone repository for rebase'
       )
 
-      unless status.zero?
-        log('Failed to clone repository for rebase:')
-        log(output)
-        return false
-      end
-
-      # Rebase
-      output, status = popen(
-        %W(git pull --rebase #{target_project.repository.path_to_repo} #{merge_request.target_branch}),
+      run_git_command(
+        %W(pull --rebase #{target_project.repository.path_to_repo} #{merge_request.target_branch}),
         tree_path,
-        git_env
+        git_env,
+        'rebase branch'
       )
 
-      unless status.zero?
-        log('Failed to rebase branch:')
-        log(output)
-        return false
-      end
-
-      output, status = popen(
-        %W(git rev-parse #{merge_request.source_branch}),
+      rebase_sha = run_git_command(
+        %W(rev-parse #{merge_request.source_branch}),
         tree_path,
-        git_env
+        git_env,
+        'get SHA of rebased branch'
       )
 
-      unless status.zero?
-        log('Failed to get SHA of rebased branch:')
-        log(output)
-        return false
-      end
+      merge_request.update_attributes(rebase_commit_sha: rebase_sha)
 
-      merge_request.update_attributes(rebase_commit_sha: output.chomp)
-      # Push
-      output, status = popen(
-        %W(git push -f origin #{merge_request.source_branch}),
+      run_git_command(
+        %W(push -f origin #{merge_request.source_branch}),
         tree_path,
-        git_env
+        git_env,
+        'push rebased branch'
       )
 
-      unless status.zero?
-        log('Failed to push rebased branch:')
-        log(output)
-        return false
-      end
-
       true
-    rescue => ex
-      log('Failed to rebase branch:')
-      log(ex.message)
+    rescue GitCommandError
+      false
+    rescue => e
+      log_error('Failed to rebase branch:')
+      log_error(e)
+      false
     ensure
       clean_dir
     end
 
-    def source_project
-      @source_project ||= merge_request.source_project
-    end
-
-    def target_project
-      @target_project ||= merge_request.target_project
-    end
-
     def tree_path
       @tree_path ||= merge_request.rebase_dir_path
     end
-
-    def log(message)
-      Gitlab::GitLogger.error(message)
-    end
-
-    def clean_dir
-      FileUtils.rm_rf(tree_path) if File.exist?(tree_path)
-    end
-
-    def git_env
-      { 'GL_ID' => Gitlab::GlId.gl_id(current_user), 'GL_PROTOCOL' => 'web' }
-    end
   end
 end

app/services/merge_requests/squash_service.rb

+require 'securerandom'
+
+module MergeRequests
+  class SquashService < MergeRequests::WorkingCopyBaseService
+    attr_reader :repository, :rugged
+
+    def execute(merge_request)
+      @merge_request = merge_request
+      @repository = target_project.repository
+      @rugged = repository.rugged
+
+      squash || error('Failed to squash. Should be done manually')
+    end
+
+    def squash
+      if merge_request.commits_count <= 1
+        return success(squash_sha: merge_request.diff_head_sha)
+      end
+
+      if merge_request.squash_in_progress?
+        log_error('Squash task canceled: Another squash is already in progress')
+        return false
+      end
+
+      run_git_command(
+        %W(worktree add #{tree_path} #{merge_request.target_branch} --detach),
+        repository.path_to_repo,
+        git_env,
+        'add worktree for squash'
+      )
+
+      run_git_command(%w(apply --cached), tree_path, git_env, 'apply patch') do |stdin|
+        stdin.puts(merge_request_to_patch)
+      end
+
+      run_git_command(
+        %W(commit -C #{merge_request.diff_head_sha}),
+        tree_path,
+        git_env.merge('GIT_COMMITTER_NAME' => current_user.name, 'GIT_COMMITTER_EMAIL' => current_user.email),
+        'commit squashed changes'
+      )
+
+      squash_sha = run_git_command(
+        %w(rev-parse HEAD),
+        tree_path,
+        git_env,
+        'get SHA of squashed commit'
+      )
+
+      success(squash_sha: squash_sha)
+    rescue GitCommandError
+      false
+    rescue => e
+      log_error("Failed to squash merge request #{merge_request.to_reference(full: true)}:")
+      log_error(e.message)
+      false
+    ensure
+      clean_dir
+    end
+
+    def tree_path
+      @tree_path ||= merge_request.squash_dir_path
+    end
+
+    def merge_request_to_patch
+      @merge_request_to_patch ||= rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha).patch
+    end
+  end
+end

app/services/merge_requests/working_copy_base_service.rb

+module MergeRequests
+  class WorkingCopyBaseService < MergeRequests::BaseService
+    class GitCommandError < StandardError; end
+
+    include Gitlab::Popen
+
+    attr_reader :merge_request
+
+    def run_git_command(command, path, env, message = nil, &block)
+      git_command = [Gitlab.config.git.bin_path] + command
+      output, status = popen(git_command, path, env, &block)
+
+      unless status.zero?
+        if message
+          log_error("Failed to #{message} with `#{git_command.join(' ')}`:")
+        else
+          log_error("`#{git_command.join(' ')}` failed:")
+        end
+
+        log_error(output)
+
+        raise GitCommandError
+      end
+
+      output.chomp
+    end
+
+    def source_project
+      @source_project ||= merge_request.source_project
+    end
+
+    def target_project
+      @target_project ||= merge_request.target_project
+    end
+
+    def log_error(message)
+      Gitlab::GitLogger.error(message)
+    end
+
+    def clean_dir
+      FileUtils.rm_rf(tree_path) if File.exist?(tree_path)
+    end
+
+    def git_env
+      { 'GL_ID' => Gitlab::GlId.gl_id(current_user), 'GL_PROTOCOL' => 'web' }
+    end
+
+    # Don't try to print expensive instance variables.
+    def inspect
+      "#<#{self.class} #{merge_request.to_reference(full: true)}>"
+    end
+  end
+end

app/views/projects/merge_requests/widget/open/_accept.html.haml

@@ -35,11 +35,17 @@
           The source branch will be removed.
       - elsif @merge_request.can_remove_source_branch?(current_user)
         .accept-control.checkbox
-          = label_tag :should_remove_source_branch, class: "remove_source_checkbox" do
+          = label_tag :should_remove_source_branch, class: "merge-param-checkbox" do
             = check_box_tag :should_remove_source_branch
             Remove source branch
+      .accept-control.checkbox
+        = label_tag :squash, class: 'merge-param-checkbox' do
+          = hidden_field_tag :squash, '0', id: nil
+          = check_box_tag :squash, '1', @merge_request.squash
+          Squash commits
+          = link_to icon('question-circle'), help_page_path('user/project/merge_requests/squash_and_merge'), title: 'About this feature', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'}
 
-      .accept-control.right
+      .accept-control
         - if @project.merge_requests_ff_only_enabled
           Fast-forward merge without a merge commit
         - else

app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml

@@ -7,10 +7,12 @@
 %div
   %p
     = succeed '.' do
+      The changes will be
+      - if @merge_request.squash
+        squashed and
       - if @project.merge_requests_ff_only_enabled
-        The changes will be fast-forward merged into
-      - else
-        The changes will be merged into
+        fast-forward
+      merged into
       %span.label-branch= @merge_request.target_branch
     - if @merge_request.remove_source_branch?
       The source branch will be removed.
@@ -22,7 +24,7 @@
   - if remove_source_branch_button || user_can_cancel_automatic_merge
     .clearfix.prepend-top-10
       - if remove_source_branch_button
-        = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true, sha: @merge_request.diff_head_sha), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
+        = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_params(@merge_request)), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
           = icon('times')
           Remove Source Branch When Merged
 

app/views/shared/issuable/_form.html.haml

@@ -45,6 +45,8 @@
 
 = render 'shared/issuable/form/branch_chooser', issuable: issuable, form: form
 
+= render 'shared/issuable/form/merge_params', issuable: issuable
+
 - if @merge_request_for_resolving_discussions
   .form-group
     .col-sm-10.col-sm-offset-2

app/views/shared/issuable/form/_branch_chooser.html.haml

@@ -19,12 +19,3 @@
     - if issuable.new_record?
        
       = link_to 'Change branches', mr_change_branches_path(issuable)
-
-- if issuable.can_remove_source_branch?(current_user)
-  .form-group
-    .col-sm-10.col-sm-offset-2
-      .checkbox
-        = label_tag 'merge_request[force_remove_source_branch]' do
-          = 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.

app/views/shared/issuable/form/_merge_params.html.haml

+- issuable = local_assigns.fetch(:issuable)
+
+- return unless issuable.is_a?(MergeRequest)
+- return if issuable.closed_without_fork?
+
+.form-group
+  .col-sm-10.col-sm-offset-2
+    - if issuable.can_remove_source_branch?(current_user)
+      .checkbox
+        = label_tag 'merge_request[force_remove_source_branch]' do
+          = 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.
+
+    .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')

changelogs/unreleased-ee/squash.yml

+---
+title: Allow squashing merge requests into a single commit
+merge_request:
+author:

db/migrate/20161230123835_add_squash_to_merge_requests.rb

+class AddSquashToMergeRequests < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
+    add_column_with_default :merge_requests, :squash, :boolean, default: false, allow_null: false
+  end
+
+  def down
+    remove_column :merge_requests, :squash
+  end
+end

db/schema.rb

@@ -782,6 +782,7 @@ ActiveRecord::Schema.define(version: 20170121130655) do
     t.text "title_html"
     t.text "description_html"
     t.integer "time_estimate"
+    t.boolean "squash", default: false, null: false
   end
 
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree

doc/api/merge_requests.md

@@ -75,6 +75,7 @@ Parameters:
     "approvals_before_merge": null
     "should_remove_source_branch": true,
     "force_remove_source_branch": false,
+    "squash": false,
     "web_url": "http://example.com/example/example/merge_requests/1"
   }
 ]
@@ -145,6 +146,7 @@ Parameters:
   "approvals_before_merge": null
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
+  "squash": false,
   "web_url": "http://example.com/example/example/merge_requests/1"
 }
 ```
@@ -251,6 +253,7 @@ Parameters:
   "approvals_before_merge": null,
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
+  "squash": false,
   "web_url": "http://example.com/example/example/merge_requests/1",
   "changes": [
     {
@@ -287,6 +290,7 @@ POST /projects/:id/merge_requests
 | `milestone_id` | integer  | no | The ID of a milestone |
 | `remove_source_branch` | boolean  | no | Flag indicating if a merge request should remove the source branch when merging |
 | `approvals_before_merge` | integer| no | Number of approvals required before this can be merged (see below) |
+| `squash` | boolean| no | Squash commits into a single commit when merging |
 
 If `approvals_before_merge` is not provided, it inherits the value from the
 target project. If it is provided, then the following conditions must hold in
@@ -349,6 +353,7 @@ order for it to take effect:
   "approvals_before_merge": null
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
+  "squash": false,
   "web_url": "http://example.com/example/example/merge_requests/1"
 }
 ```
@@ -374,6 +379,7 @@ PUT /projects/:id/merge_requests/:merge_request_id
 | `labels` | string  | no | Labels for MR as a comma-separated list |
 | `milestone_id` | integer  | no | The ID of a milestone |
 | `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 |
 
 ```json
 {
@@ -426,6 +432,7 @@ PUT /projects/:id/merge_requests/:merge_request_id
   "approvals_before_merge": null
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
+  "squash": false,
   "web_url": "http://example.com/example/example/merge_requests/1"
 }
 ```
@@ -471,7 +478,19 @@ Parameters:
 - `merge_commit_message` (optional)         - Custom merge commit message
 - `should_remove_source_branch` (optional)  - if `true` removes the source branch
 - `merge_when_build_succeeds` (optional)    - if `true` the MR is merged when the build succeeds
-- `sha` (optional)                          - if present, then this SHA must match the HEAD of the source branch, otherwise the merge will fail
+- `sha` (optional)                          - if present, then this SHA must
+
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | string | yes | The ID of a project |
+| `merge_request_id` | integer | yes | The ID of the merge request |
+| `merge_commit_message` | string | no | Custom merge commit message |
+| `should_remove_source_branch` | boolean | no | Remove the source branch after merge |
+| `merge_when_build_succeeds` | boolean | no | Merge when build succeeds, rather than immediately |
+| `sha` | string | no  | If present, then this SHA must match the HEAD of the source branch, otherwise the merge will fail |
+| `squash` | boolean | no | Squash the merge request into a single commit |
+
 
 ```json
 {
@@ -525,6 +544,7 @@ Parameters:
   "approvals_before_merge": null
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
+  "squash": false,
   "web_url": "http://example.com/example/example/merge_requests/1"
 }
 ```
@@ -698,6 +718,7 @@ Parameters:
   "approvals_before_merge": null
   "should_remove_source_branch": true,
   "force_remove_source_branch": false,
+  "squash": false,
   "web_url": "http://example.com/example/example/merge_requests/1"
 }
 ```
@@ -1021,6 +1042,7 @@ Example response:
     "user_notes_count": 7,
     "should_remove_source_branch": true,
     "force_remove_source_branch": false,
+    "squash": true,
     "web_url": "http://example.com/example/example/merge_requests/1"
   },
   "target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/merge_requests/7",

doc/user/project/merge_requests.md

@@ -36,6 +36,13 @@ you hide discussions that are no longer relevant.
 
 [Read more about resolving discussion comments in merge requests reviews.](merge_requests/merge_request_discussion_resolution.md)
 
+## Squash and merge
+
+GitLab allows you to squash all changes present in a merge request into a single
+commit when merging, to allow for a neater commit history.
+
+[Learn more about squash and merge.](merge_requests/squash_and_merge)
+
 ## Resolve conflicts
 
 When a merge request has conflicts, GitLab may provide the option to resolve

doc/user/project/merge_requests/squash_and_merge.md

+# Squash and merge
+
+> [Introduced][ee-1024] in GitLab Enterprise Edition 8.16.
+
+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]
+
+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](#squashing-and-fast-forward-merge) for more
+details.
+
+## 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: taken from the last commit in the source branch.
+* Author: taken from the last commit in the source branch.
+* Committer: the user who initiated the squash.
+
+## Squashing and [fast-forward merge][ff-merge]
+
+When a project has the fast-forward merge setting enabled, 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][rebase] before squashing, even though
+squashing can itself be considered equivalent to rebasing.
+
+[ee-1024]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1024
+[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
+[rebase]: ../../../workflow/rebase_before_merge

lib/api/entities.rb

@@ -326,6 +326,7 @@ module API
       expose :approvals_before_merge
       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)

lib/api/merge_requests.rb

@@ -34,6 +34,7 @@ module API
           optional :labels, type: String, desc: 'Comma-separated list of label names'
           optional :approvals_before_merge, type: Integer, desc: 'Number of approvals required before this can be merged'
           optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging'
+          optional :squash, type: Boolean, desc: 'Squash commits when merging'
         end
       end
 
@@ -153,7 +154,7 @@ module API
           use :optional_params
           at_least_one_of :title, :target_branch, :description, :assignee_id,
                           :milestone_id, :labels, :state_event, :approvals_before_merge,
-                          :remove_source_branch
+                          :remove_source_branch, :squash
         end
         put path do
           merge_request = find_merge_request_with_access(params.delete(:merge_request_id), :update_merge_request)
@@ -180,6 +181,7 @@ module API
           optional :merge_when_build_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'
+          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])
@@ -196,6 +198,10 @@ module API
             render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409)
           end
 
+          if params[:squash]
+            merge_request.update(squash: params[:squash])
+          end
+
           merge_params = {
             commit_message: params[:merge_commit_message],
             should_remove_source_branch: params[:should_remove_source_branch]

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -492,6 +492,7 @@ describe Projects::MergeRequestsController do
         namespace_id: project.namespace.path,
         project_id: project.path,
         id: merge_request.iid,
+        squash: false,
         format: 'raw'
       }
     end
@@ -529,8 +530,26 @@ describe Projects::MergeRequestsController do
     end
 
     context 'when the sha parameter matches the source SHA' do
-      def merge_with_sha
-        post :merge, base_params.merge(sha: merge_request.diff_head_sha)
+      def merge_with_sha(params = {})
+        post :merge, base_params.merge(sha: merge_request.diff_head_sha).merge(params)
+      end
+
+      context 'when squash is passed as 1' do
+        it 'updates the squash attribute on the MR to true' do
+          merge_request.update(squash: false)
+          merge_with_sha(squash: '1')
+
+          expect(merge_request.reload.squash).to be_truthy
+        end
+      end
+
+      context 'when squash is passed as 1' do
+        it 'updates the squash attribute on the MR to false' do
+          merge_request.update(squash: true)
+          merge_with_sha(squash: '0')
+
+          expect(merge_request.reload.squash).to be_falsey
+        end
       end
 
       it 'returns :success' do

spec/features/merge_requests/squash_spec.rb

+require 'spec_helper'
+
+feature 'Squashing merge requests', js: true, feature: true do
+  include WaitForAjax
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:source_branch) { 'csv' }
+
+  let!(:original_head) { project.repository.commit('master') }
+
+  shared_examples 'squash' do
+    it 'squashes the commits into a single commit, and adds a merge commit' do
+      latest_master_commits = project.repository.commits_between(original_head.sha, 'master').map(&:raw)
+      last_mr_commit = project.repository.commit(source_branch)
+
+      squash_commit = an_object_having_attributes(sha: a_string_matching(/\h{40}/),
+                                                  message: "#{last_mr_commit.message}\n",
+                                                  author_name: last_mr_commit.author_name,
+                                                  committer_name: user.name)
+
+      merge_commit = an_object_having_attributes(sha: a_string_matching(/\h{40}/),
+                                                 message: a_string_starting_with("Merge branch 'csv' into 'master'"),
+                                                 author_name: user.name,
+                                                 committer_name: user.name)
+
+      expect(project.repository).not_to be_merged_to_root_ref(source_branch)
+      expect(latest_master_commits).to match([squash_commit, merge_commit])
+    end
+  end
+
+  shared_examples 'no squash' do
+    it 'accepts the merge request without squashing' do
+      expect(project.repository).to be_merged_to_root_ref(source_branch)
+    end
+  end
+
+  def accept_mr
+    click_on 'Accept Merge Request'
+    wait_for_ajax
+  end
+
+  before do
+    project.team << [user, :master]
+
+    login_as user
+  end
+
+  context 'when squash is enabled on merge request creation' do
+    before do
+      visit new_namespace_project_merge_request_path(project.namespace, project, merge_request: { source_branch: source_branch })
+      check 'merge_request[squash]'
+      click_on 'Submit merge request'
+      wait_for_ajax
+    end
+
+    it 'shows the squash checkbox as checked' do
+      expect(page).to have_checked_field('squash')
+    end
+
+    context 'when accepting with squash checked' do
+      before do
+        accept_mr
+      end
+
+      include_examples 'squash'
+    end
+
+    context 'when accepting and unchecking squash' do
+      before do
+        uncheck 'squash'
+        accept_mr
+      end
+
+      include_examples 'no squash'
+    end
+  end
+
+  context 'when squash is not enabled on merge request creation' do
+    before do
+      visit new_namespace_project_merge_request_path(project.namespace, project, merge_request: { source_branch: source_branch })
+      click_on 'Submit merge request'
+      wait_for_ajax
+    end
+
+    it 'shows the squash checkbox as unchecked' do
+      expect(page).to have_unchecked_field('squash')
+    end
+
+    context 'when accepting and checking squash' do
+      before do
+        check 'squash'
+        accept_mr
+      end
+
+      include_examples 'squash'
+    end
+
+    context 'when accepting with squash unchecked' do
+      before do
+        accept_mr
+      end
+
+      include_examples 'no squash'
+    end
+  end
+end

spec/lib/gitlab/diff/position_tracer_spec.rb

@@ -1640,7 +1640,9 @@ describe Gitlab::Diff::PositionTracer, lib: true do
         }
 
         merge_request = create(:merge_request, source_branch: second_create_file_commit.sha, target_branch: branch_name, source_project: project)
-        repository.merge(current_user, merge_request, options)
+
+        repository.merge(current_user, merge_request.diff_head_sha, merge_request, options)
+
         project.commit(branch_name)
       end
 

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -154,6 +154,7 @@ MergeRequest:
 - approvals_before_merge
 - rebase_commit_sha
 - time_estimate
+- squash
 MergeRequestDiff:
 - id
 - state

spec/models/merge_request_spec.rb

@@ -788,36 +788,67 @@ describe MergeRequest, models: true do
   end
 
   describe '#rebase_in_progress?' do
-    it 'returns true' do
+    it 'returns true when there is a current rebase directory' do
       allow(File).to receive(:exist?).and_return(true)
-      allow(File).to receive(:new).and_return(double(:file, mtime: Time.now))
+      allow(File).to receive(:mtime).and_return(Time.now)
 
       expect(subject.rebase_in_progress?).to be_truthy
     end
 
-    it 'returns false' do
+    it 'returns false when there is no rebase directory' do
       allow(File).to receive(:exist?).with(subject.rebase_dir_path).and_return(false)
 
       expect(subject.rebase_in_progress?).to be_falsey
     end
 
-    it 'returns false if temporary file exists by is expired' do
+    it 'returns false when the rebase directory has expired' do
       allow(File).to receive(:exist?).and_return(true)
-      allow(File).to receive(:new).and_return(double(:file, mtime: Time.now - 2.hours))
+      allow(File).to receive(:mtime).and_return(20.minutes.ago)
 
       expect(subject.rebase_in_progress?).to be_falsey
     end
 
-    it 'returns false if source_project is removed' do
+    it 'returns false when the source project has been removed' do
       allow(subject).to receive(:source_project).and_return(nil)
       allow(File).to receive(:exist?).and_return(true)
-      allow(File).to receive(:new).and_return(double(:file, mtime: Time.now))
+      allow(File).to receive(:mtime).and_return(Time.now)
 
       expect(File).not_to have_received(:exist?)
       expect(subject.rebase_in_progress?).to be_falsey
     end
   end
 
+  describe '#squash_in_progress?' do
+    it 'returns true when there is a current squash directory' do
+      allow(File).to receive(:exist?).and_return(true)
+      allow(File).to receive(:mtime).and_return(Time.now)
+
+      expect(subject.squash_in_progress?).to be_truthy
+    end
+
+    it 'returns false when there is no squash directory' do
+      allow(File).to receive(:exist?).with(subject.squash_dir_path).and_return(false)
+
+      expect(subject.squash_in_progress?).to be_falsey
+    end
+
+    it 'returns false when the squash directory has expired' do
+      allow(File).to receive(:exist?).and_return(true)
+      allow(File).to receive(:mtime).and_return(20.minutes.ago)
+
+      expect(subject.squash_in_progress?).to be_falsey
+    end
+
+    it 'returns false when the source project has been removed' do
+      allow(subject).to receive(:source_project).and_return(nil)
+      allow(File).to receive(:exist?).and_return(true)
+      allow(File).to receive(:mtime).and_return(Time.now)
+
+      expect(File).not_to have_received(:exist?)
+      expect(subject.squash_in_progress?).to be_falsey
+    end
+  end
+
   describe '#commits_sha' do
     before do
       allow(subject.merge_request_diff).to receive(:commits_sha).

spec/models/repository_spec.rb

@@ -15,7 +15,12 @@ describe Repository, models: true do
 
   let(:merge_commit) do
     merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project)
-    merge_commit_id = repository.merge(user, merge_request, commit_options)
+
+    merge_commit_id = repository.merge(user,
+                                       merge_request.diff_head_sha,
+                                       merge_request,
+                                       commit_options)
+
     repository.commit(merge_commit_id)
   end
 
@@ -1009,8 +1014,11 @@ describe Repository, models: true do
 
     it 'sets the `in_progress_merge_commit_sha` flag for the given merge request' do
       merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project)
-      merge_commit_id = repository.merge(user, merge_request, commit_options)
-      repository.commit(merge_commit_id)
+
+      merge_commit_id = repository.merge(user,
+                                         merge_request.diff_head_sha,
+                                         merge_request,
+                                         commit_options)
 
       expect(merge_request.in_progress_merge_commit_sha).to eq(merge_commit_id)
     end

spec/requests/api/merge_requests_spec.rb

@@ -39,6 +39,7 @@ describe API::MergeRequests, api: true  do
         expect(json_response.first['sha']).to eq(merge_request_merged.diff_head_sha)
         expect(json_response.first['merge_commit_sha']).not_to be_nil
         expect(json_response.first['merge_commit_sha']).to eq(merge_request_merged.merge_commit_sha)
+        expect(json_response.first['squash']).to eq(merge_request_merged.squash)
       end
 
       it "returns an array of all merge_requests" do
@@ -233,13 +234,15 @@ describe API::MergeRequests, api: true  do
              author: user,
              labels: 'label, label2',
              milestone_id: milestone.id,
-             remove_source_branch: true
+             remove_source_branch: true,
+             squash: true
 
         expect(response).to have_http_status(201)
         expect(json_response['title']).to eq('Test merge_request')
         expect(json_response['labels']).to eq(['label', 'label2'])
         expect(json_response['milestone']['id']).to eq(milestone.id)
         expect(json_response['force_remove_source_branch']).to be_truthy
+        expect(json_response['squash']).to be_truthy
       end
 
       it "returns 422 when source_branch equals target_branch" do
@@ -526,6 +529,14 @@ describe API::MergeRequests, api: true  do
       expect(response).to have_http_status(200)
     end
 
+    it "updates the MR's squash attribute" do
+      expect do
+        put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), squash: true
+      end.to change { merge_request.reload.squash }
+
+      expect(response).to have_http_status(200)
+    end
+
     it "enables merge when pipeline succeeds if the pipeline is active" do
       allow_any_instance_of(MergeRequest).to receive(:head_pipeline).and_return(pipeline)
       allow(pipeline).to receive(:active?).and_return(true)
@@ -566,6 +577,13 @@ describe API::MergeRequests, api: true  do
       expect(json_response['milestone']['id']).to eq(milestone.id)
     end
 
+    it "updates squash and returns merge_request" do
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), squash: true
+
+      expect(response).to have_http_status(200)
+      expect(json_response['squash']).to be_truthy
+    end
+
     it "returns merge_request with renamed target_branch" do
       put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), target_branch: "wiki"
       expect(response).to have_http_status(200)

spec/services/merge_requests/refresh_service_spec.rb

@@ -101,7 +101,8 @@ describe MergeRequests::RefreshService, services: true do
         # Merge master -> feature branch
         author = { email: 'test@gitlab.com', time: Time.now, name: "Me" }
         commit_options = { message: 'Test message', committer: author, author: author }
-        @project.repository.merge(@user, @merge_request, commit_options)
+        @project.repository.merge(@user, @merge_request.diff_head_sha, @merge_request, commit_options)
+
         commit = @project.repository.commit('feature')
         service.new(@project, @user).execute(@oldrev, commit.id, 'refs/heads/feature')
         reload_mrs

spec/services/merge_requests/squash_service_spec.rb

+require 'spec_helper'
+
+describe MergeRequests::SquashService do
+  let(:service) { described_class.new(project, user, {}) }
+  let(:user) { project.owner }
+  let(:project) { create(:project) }
+
+  let(:merge_request) do
+    create(:merge_request,
+           source_branch: 'fix', source_project: project,
+           target_branch: 'master', target_project: project)
+  end
+
+  let(:merge_request_with_one_commit) do
+    create(:merge_request,
+           source_branch: 'feature.custom-highlighting', source_project: project,
+           target_branch: 'master', target_project: project)
+  end
+
+  def git_command(command)
+    a_collection_starting_with([Gitlab.config.git.bin_path, command])
+  end
+
+  shared_examples 'the squashed commit' do
+  end
+
+  describe '#execute' do
+    context 'when there is only one commit in the merge request' do
+      it 'returns that commit SHA' do
+        result = service.execute(merge_request_with_one_commit)
+
+        expect(result).to match(status: :success, squash_sha: merge_request_with_one_commit.diff_head_sha)
+      end
+
+      it 'does not perform any git actions' do
+        expect(service).not_to receive(:run_git_command)
+
+        service.execute(merge_request_with_one_commit)
+      end
+    end
+
+    context 'when the squash succeeds' do
+      it 'returns the squashed commit SHA' do
+        result = service.execute(merge_request)
+
+        expect(result).to match(status: :success, squash_sha: a_string_matching(/\h{40}/))
+        expect(result[:squash_sha]).not_to eq(merge_request.diff_head_sha)
+      end
+
+      it 'cleans up the temporary directory' do
+        expect(service).to receive(:clean_dir).and_call_original
+
+        service.execute(merge_request)
+      end
+
+      it 'does not keep the branch push event' do
+        expect { service.execute(merge_request) }.not_to change { Event.count }
+      end
+
+      context 'the squashed commit' do
+        let(:squash_sha) { service.execute(merge_request)[:squash_sha] }
+        let(:squash_commit) { project.repository.commit(squash_sha) }
+
+        it 'copies the author info and message from the last commit in the source branch' do
+          diff_head_commit = merge_request.diff_head_commit
+
+          expect(squash_commit.author_name).to eq(diff_head_commit.author_name)
+          expect(squash_commit.author_email).to eq(diff_head_commit.author_email)
+          expect(squash_commit.message).to eq(diff_head_commit.message)
+        end
+
+        it 'sets the current user as the committer' do
+          expect(squash_commit.committer_name).to eq(user.name.chomp('.'))
+          expect(squash_commit.committer_email).to eq(user.email)
+        end
+
+        it 'has the same diff as the merge request, but a different SHA' do
+          rugged = project.repository.rugged
+          mr_diff = rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha)
+          squash_diff = rugged.diff(merge_request.diff_start_sha, squash_sha)
+
+          expect(squash_diff.patch).to eq(mr_diff.patch)
+          expect(squash_commit.sha).not_to eq(merge_request.diff_head_sha)
+        end
+      end
+    end
+
+    stages = {
+      'add worktree for squash' => 'worktree',
+      'apply patch' => 'apply',
+      'commit squashed changes' => 'commit',
+      'get SHA of squashed commit' => 'rev-parse'
+    }
+
+    stages.each do |stage, command|
+      context "when the #{stage} stage fails" do
+        let(:error) { 'A test error' }
+
+        before do
+          allow(service).to receive(:popen).and_return(['', 0])
+          allow(service).to receive(:popen).with(git_command(command), anything, anything).and_return([error, 1])
+        end
+
+        it 'logs the stage and output' do
+          expect(service).to receive(:log_error).with(a_string_including(stage))
+          expect(service).to receive(:log_error).with(error)
+
+          service.execute(merge_request)
+        end
+
+        it 'returns an error' do
+          expect(service.execute(merge_request)).to match(status: :error,
+                                                          message: a_string_including('squash'))
+        end
+
+        it 'cleans up the temporary directory' do
+          expect(service).to receive(:clean_dir).and_call_original
+
+          service.execute(merge_request)
+        end
+      end
+    end
+
+    context 'when any other exception is thrown' do
+      let(:error) { 'A test error' }
+
+      before do
+        allow(merge_request).to receive(:commits_count).and_raise(error)
+      end
+
+      it 'logs the MR reference and exception' do
+        expect(service).to receive(:log_error).with(a_string_including("#{project.path_with_namespace}#{merge_request.to_reference}"))
+        expect(service).to receive(:log_error).with(error)
+
+        service.execute(merge_request)
+      end
+
+      it 'returns an error' do
+        expect(service.execute(merge_request)).to match(status: :error,
+                                                        message: a_string_including('squash'))
+      end
+
+      it 'cleans up the temporary directory' do
+        expect(service).to receive(:clean_dir).and_call_original
+
+        service.execute(merge_request)
+      end
+    end
+  end
+end

spec/support/test_env.rb

@@ -37,7 +37,8 @@ module TestEnv
     'conflict-non-utf8'                  => 'd0a293c',
     'conflict-too-large'                 => '39fa04f',
     'deleted-image-test'                 => '6c17798',
-    'wip'                                => 'b9238ee'
+    'wip'                                => 'b9238ee',
+    'csv'                                => '3dd0896'
   }
 
   # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily