Merge pull request #1802 from gitbucket/merge-strategy

Add a pulldown menu to choose the merge strategy

src/main/scala/gitbucket/core/controller/PullRequestsController.scala

@@ -16,6 +16,7 @@ import gitbucket.core.util._
 import org.scalatra.forms._
 import org.eclipse.jgit.api.Git
 import org.eclipse.jgit.lib.PersonIdent
+import org.eclipse.jgit.revwalk.RevWalk
 
 import scala.collection.JavaConverters._
 
@@ -50,7 +51,8 @@ trait PullRequestsControllerBase extends ControllerBase {
   )(PullRequestForm.apply)
 
   val mergeForm = mapping(
-    "message" -> trim(label("Message", text(required)))
+    "message"  -> trim(label("Message", text(required))),
+    "strategy" -> trim(label("Strategy", text(required)))
   )(MergeForm.apply)
 
   case class PullRequestForm(
@@ -69,7 +71,7 @@ trait PullRequestsControllerBase extends ControllerBase {
     labelNames: Option[String]
   )
 
-  case class MergeForm(message: String)
+  case class MergeForm(message: String, strategy: String)
 
   get("/:owner/:repository/pulls")(referrersOnly { repository =>
     val q = request.getParameter("q")
@@ -258,13 +260,29 @@ trait PullRequestsControllerBase extends ControllerBase {
             // record activity
             recordMergeActivity(owner, name, loginAccount.userName, issueId, form.message)
 
+            val (commits, _) = getRequestCompareInfo(owner, name, pullreq.commitIdFrom,
+              pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo)
+
+            val revCommits = using(new RevWalk( git.getRepository )){ revWalk =>
+              commits.flatten.map { commit =>
+                revWalk.parseCommit(git.getRepository.resolve(commit.id))
+              }
+            }.reverse
+
             // merge git repository
+            form.strategy match {
+              case "merge-commit" =>
                 mergePullRequest(git, pullreq.branch, issueId,
                   s"Merge pull request #${issueId} from ${pullreq.requestUserName}/${pullreq.requestBranch}\n\n" + form.message,
                   new PersonIdent(loginAccount.fullName, loginAccount.mailAddress))
-
+              case "rebase" =>
-            val (commits, _) = getRequestCompareInfo(owner, name, pullreq.commitIdFrom,
+                rebasePullRequest(git, pullreq.branch, issueId, revCommits,
-              pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo)
+                  new PersonIdent(loginAccount.fullName, loginAccount.mailAddress))
+              case "squash" =>
+                squashPullRequest(git, pullreq.branch, issueId,
+                  s"${issue.title} (#${issueId})\n\n" + form.message,
+                  new PersonIdent(loginAccount.fullName, loginAccount.mailAddress))
+            }
 
             // close issue by content of pull request
             val defaultBranch = getRepository(owner, name).get.repository.defaultBranch

src/main/scala/gitbucket/core/service/MergeService.scala

@@ -3,25 +3,26 @@ package gitbucket.core.service
 import gitbucket.core.model.Account
 import gitbucket.core.util.Directory._
 import gitbucket.core.util.SyntaxSugars._
-
 import org.eclipse.jgit.merge.MergeStrategy
 import org.eclipse.jgit.api.Git
 import org.eclipse.jgit.transport.RefSpec
 import org.eclipse.jgit.errors.NoMergeBaseException
-import org.eclipse.jgit.lib.{ObjectId, CommitBuilder, PersonIdent, Repository}
+import org.eclipse.jgit.lib.{CommitBuilder, ObjectId, PersonIdent, Repository}
-import org.eclipse.jgit.revwalk.RevWalk
+import org.eclipse.jgit.revwalk.{RevCommit, RevWalk}
 
 trait MergeService {
   import MergeService._
+
   /**
    * Checks whether conflict will be caused in merging within pull request.
    * Returns true if conflict will be caused.
    */
   def checkConflict(userName: String, repositoryName: String, branch: String, issueId: Int): Boolean = {
     using(Git.open(getRepositoryDir(userName, repositoryName))) { git =>
-      MergeCacheInfo(git, branch, issueId).checkConflict()
+      new MergeCacheInfo(git, branch, issueId).checkConflict()
     }
   }
+
   /**
    * Checks whether conflict will be caused in merging within pull request.
    * only cache check.
@@ -30,13 +31,25 @@ trait MergeService {
    */
   def checkConflictCache(userName: String, repositoryName: String, branch: String, issueId: Int): Option[Boolean] = {
     using(Git.open(getRepositoryDir(userName, repositoryName))) { git =>
-      MergeCacheInfo(git, branch, issueId).checkConflictCache()
+      new MergeCacheInfo(git, branch, issueId).checkConflictCache()
     }
   }
-  /** merge pull request */
+
-  def mergePullRequest(git:Git, branch: String, issueId: Int, message:String, committer:PersonIdent): Unit = {
+  /** merge the pull request with a merge commit */
-    MergeCacheInfo(git, branch, issueId).merge(message, committer)
+  def mergePullRequest(git: Git, branch: String, issueId: Int, message: String, committer: PersonIdent): Unit = {
+    new MergeCacheInfo(git, branch, issueId).merge(message, committer)
   }
+
+  /** rebase to the head of the pull request branch */
+  def rebasePullRequest(git: Git, branch: String, issueId: Int, commits: Seq[RevCommit], committer: PersonIdent): Unit = {
+    new MergeCacheInfo(git, branch, issueId).rebase(committer, commits)
+  }
+
+  /** squash commits in the pull request and append it */
+  def squashPullRequest(git: Git, branch: String, issueId: Int, message: String, committer: PersonIdent): Unit = {
+    new MergeCacheInfo(git, branch, issueId).squash(message, committer)
+  }
+
   /** fetch remote branch to my repository refs/pull/{issueId}/head */
   def fetchAsPullRequest(userName: String, repositoryName: String, requestUserName: String, requestRepositoryName: String, requestBranch:String, issueId:Int){
     using(Git.open(getRepositoryDir(userName, repositoryName))){ git =>
@@ -46,6 +59,7 @@ trait MergeService {
         .call
     }
   }
+
   /**
    * Checks whether conflict will be caused in merging. Returns true if conflict will be caused.
    */
@@ -81,6 +95,7 @@ trait MergeService {
       }
     }
   }
+
   /**
    * Checks whether conflict will be caused in merging. Returns true if conflict will be caused.
    */
@@ -91,7 +106,8 @@ trait MergeService {
   def pullRemote(localUserName: String, localRepositoryName: String, localBranch: String,
                       remoteUserName: String, remoteRepositoryName: String, remoteBranch: String,
                       loginAccount: Account, message: String): Option[ObjectId] = {
-    tryMergeRemote(localUserName, localRepositoryName, localBranch, remoteUserName, remoteRepositoryName, remoteBranch).map{ case (newTreeId, oldBaseId, oldHeadId) =>
+    tryMergeRemote(localUserName, localRepositoryName, localBranch, remoteUserName, remoteRepositoryName, remoteBranch)
+      .map { case (newTreeId, oldBaseId, oldHeadId) =>
         using(Git.open(getRepositoryDir(localUserName, localRepositoryName))) { git =>
           val committer = new PersonIdent(loginAccount.fullName, loginAccount.mailAddress)
           val newCommit = Util.createMergeCommit(git.getRepository, newTreeId, committer, message, Seq(oldBaseId, oldHeadId))
@@ -102,9 +118,11 @@ trait MergeService {
   }
 
 }
+
 object MergeService{
+
   object Util{
-    // return treeId
+    // return merge commit id
     def createMergeCommit(repository: Repository, treeId: ObjectId, committer: PersonIdent, message: String, parents: Seq[ObjectId]): ObjectId = {
       val mergeCommit = new CommitBuilder()
       mergeCommit.setTreeId(treeId)
@@ -113,14 +131,14 @@ object MergeService{
       mergeCommit.setCommitter(committer)
       mergeCommit.setMessage(message)
       // insertObject and got mergeCommit Object Id
-      val inserter = repository.newObjectInserter
+      using(repository.newObjectInserter){ inserter =>
         val mergeCommitId = inserter.insert(mergeCommit)
         inserter.flush()
-      inserter.close()
         mergeCommitId
       }
-    def updateRefs(repository: Repository, ref: String, newObjectId: ObjectId, force: Boolean, committer: PersonIdent, refLogMessage: Option[String] = None):Unit = {
+    }
-      // update refs
+
+    def updateRefs(repository: Repository, ref: String, newObjectId: ObjectId, force: Boolean, committer: PersonIdent, refLogMessage: Option[String] = None): Unit = {
       val refUpdate = repository.updateRef(ref)
       refUpdate.setNewObjectId(newObjectId)
       refUpdate.setForceUpdate(force)
@@ -129,21 +147,25 @@ object MergeService{
       refUpdate.update()
     }
   }
-  case class MergeCacheInfo(git:Git, branch:String, issueId:Int){
+
-    val repository = git.getRepository
+  class MergeCacheInfo(git: Git, branch: String, issueId: Int){
-    val mergedBranchName = s"refs/pull/${issueId}/merge"
+
-    val conflictedBranchName = s"refs/pull/${issueId}/conflict"
+    private val repository = git.getRepository
+    private val mergedBranchName = s"refs/pull/${issueId}/merge"
+    private val conflictedBranchName = s"refs/pull/${issueId}/conflict"
+
     lazy val mergeBaseTip = repository.resolve(s"refs/heads/${branch}")
     lazy val mergeTip = repository.resolve(s"refs/pull/${issueId}/head")
+
     def checkConflictCache(): Option[Boolean] = {
-      Option(repository.resolve(mergedBranchName)).flatMap{ merged =>
+      Option(repository.resolve(mergedBranchName)).flatMap { merged =>
         if(parseCommit(merged).getParents().toSet == Set( mergeBaseTip, mergeTip )){
           // merged branch exists
           Some(false)
         } else {
           None
         }
-      }.orElse(Option(repository.resolve(conflictedBranchName)).flatMap{ conflicted =>
+      }.orElse(Option(repository.resolve(conflictedBranchName)).flatMap { conflicted =>
         if(parseCommit(conflicted).getParents().toSet == Set( mergeBaseTip, mergeTip )){
           // conflict branch exists
           Some(true)
@@ -152,10 +174,12 @@ object MergeService{
         }
       })
     }
-    def checkConflict():Boolean ={
+
+    def checkConflict(): Boolean = {
       checkConflictCache.getOrElse(checkConflictForce)
     }
-    def checkConflictForce():Boolean ={
+
+    def checkConflictForce(): Boolean = {
       val merger = MergeStrategy.RECURSIVE.newMerger(repository, true)
       val conflicted = try {
         !merger.merge(mergeBaseTip, mergeTip)
@@ -164,35 +188,101 @@ object MergeService{
       }
       val mergeTipCommit = using(new RevWalk( repository ))(_.parseCommit( mergeTip ))
       val committer = mergeTipCommit.getCommitterIdent
-      def updateBranch(treeId:ObjectId, message:String, branchName:String){
+
+      def _updateBranch(treeId: ObjectId, message: String, branchName: String){
         // creates merge commit
         val mergeCommitId = createMergeCommit(treeId, committer, message)
         Util.updateRefs(repository, branchName, mergeCommitId, true, committer)
       }
+
       if(!conflicted){
-        updateBranch(merger.getResultTreeId, s"Merge ${mergeTip.name} into ${mergeBaseTip.name}", mergedBranchName)
+        _updateBranch(merger.getResultTreeId, s"Merge ${mergeTip.name} into ${mergeBaseTip.name}", mergedBranchName)
         git.branchDelete().setForce(true).setBranchNames(conflictedBranchName).call()
       } else {
-        updateBranch(mergeTipCommit.getTree().getId(), s"can't merge ${mergeTip.name} into ${mergeBaseTip.name}", conflictedBranchName)
+        _updateBranch(mergeTipCommit.getTree().getId(), s"can't merge ${mergeTip.name} into ${mergeBaseTip.name}", conflictedBranchName)
         git.branchDelete().setForce(true).setBranchNames(mergedBranchName).call()
       }
       conflicted
     }
-    // update branch from cache
+
-    def merge(message:String, committer:PersonIdent) = {
+    def merge(message: String, committer: PersonIdent): Unit = {
       if(checkConflict()){
         throw new RuntimeException("This pull request can't merge automatically.")
       }
-      val mergeResultCommit = parseCommit( Option(repository.resolve(mergedBranchName)).getOrElse(throw new RuntimeException(s"not found branch ${mergedBranchName}")) )
+      val mergeResultCommit = parseCommit(Option(repository.resolve(mergedBranchName)).getOrElse {
+        throw new RuntimeException(s"not found branch ${mergedBranchName}")
+      })
       // creates merge commit
       val mergeCommitId = createMergeCommit(mergeResultCommit.getTree().getId(), committer, message)
       // update refs
       Util.updateRefs(repository, s"refs/heads/${branch}", mergeCommitId, false, committer, Some("merged"))
     }
+
+    def rebase(committer: PersonIdent, commits: Seq[RevCommit]): Unit = {
+      if(checkConflict()){
+        throw new RuntimeException("This pull request can't merge automatically.")
+      }
+
+      def _cloneCommit(commit: RevCommit, parents: Array[ObjectId]): CommitBuilder = {
+        val newCommit = new CommitBuilder()
+        newCommit.setTreeId(commit.getTree.getId)
+        parents.foreach { parentId =>
+          newCommit.addParentId(parentId)
+        }
+        newCommit.setAuthor(commit.getAuthorIdent)
+        newCommit.setCommitter(committer)
+        newCommit.setMessage(commit.getFullMessage)
+        newCommit
+      }
+
+      val mergeBaseTipCommit = using(new RevWalk( repository ))(_.parseCommit( mergeBaseTip ))
+      var previousId = mergeBaseTipCommit.getId
+
+      using(repository.newObjectInserter){ inserter =>
+        commits.foreach { commit =>
+          val nextCommit = _cloneCommit(commit, Array(previousId))
+          previousId = inserter.insert(nextCommit)
+        }
+        inserter.flush()
+      }
+
+      Util.updateRefs(repository, s"refs/heads/${branch}", previousId, false, committer, Some("rebased"))
+    }
+
+    def squash(message: String, committer: PersonIdent): Unit = {
+      if(checkConflict()){
+        throw new RuntimeException("This pull request can't merge automatically.")
+      }
+
+      val mergeBaseTipCommit = using(new RevWalk( repository ))(_.parseCommit(mergeBaseTip))
+      val mergeBranchHeadCommit = using(new RevWalk( repository ))(_.parseCommit(repository.resolve(mergedBranchName)))
+
+      // Create squash commit
+      val mergeCommit = new CommitBuilder()
+      mergeCommit.setTreeId(mergeBranchHeadCommit.getTree.getId)
+      mergeCommit.setParentId(mergeBaseTipCommit)
+      mergeCommit.setAuthor(mergeBranchHeadCommit.getAuthorIdent)
+      mergeCommit.setCommitter(committer)
+      mergeCommit.setMessage(message)
+
+      // insertObject and got squash commit Object Id
+      val newCommitId = using(repository.newObjectInserter){ inserter =>
+        val newCommitId = inserter.insert(mergeCommit)
+        inserter.flush()
+        newCommitId
+      }
+
+      Util.updateRefs(repository, mergedBranchName, newCommitId, true, committer)
+
+      // rebase to squash commit
+      Util.updateRefs(repository, s"refs/heads/${branch}", repository.resolve(mergedBranchName), false, committer, Some("squashed"))
+    }
+
     // return treeId
     private def createMergeCommit(treeId: ObjectId, committer: PersonIdent, message: String) =
       Util.createMergeCommit(repository, treeId, committer, message, Seq[ObjectId](mergeBaseTip, mergeTip))
 
-    private def parseCommit(id:ObjectId) = using(new RevWalk( repository ))(_.parseCommit(id))
+    private def parseCommit(id: ObjectId) = using(new RevWalk( repository ))(_.parseCommit(id))
+
   }
 }

src/main/scala/gitbucket/core/util/JGitUtil.scala

@@ -981,7 +981,7 @@ object JGitUtil {
       val blame = blamer.call()
       var blameMap = Map[String, JGitUtil.BlameInfo]()
       var idLine = List[(String, Int)]()
-      val commits = 0.to(blame.getResultContents().size() - 1).map{ i =>
+      val commits = 0.to(blame.getResultContents().size() - 1).map { i =>
         val c = blame.getSourceCommit(i)
         if(!blameMap.contains(c.name)){
           blameMap += c.name -> JGitUtil.BlameInfo(

src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html

@@ -139,8 +139,34 @@
         <span id="error-message" class="error"></span>
         <textarea name="message" style="height: 80px; margin-top: 8px; margin-bottom: 8px;" class="form-control">@issue.title</textarea>
         <div>
+          <div class="btn-group">
+            <button id="merge-strategy-btn" class="dropdown-toggle btn btn-default" data-toggle="dropdown">
+              <span class="strong">Merge commit</span>
+              <span class="caret"></span>
+            </button>
+            <ul class="dropdown-menu">
+              <li>
+                <a href="javascript:void(0);" class="merge-strategy" data-value="merge-commit">
+                  <strong>Merge commit</strong><br>These commits will be added to the base branch via a merge commit.
+                </a>
+              </li>
+              <li>
+                <a href="javascript:void(0);" class="merge-strategy" data-value="squash">
+                  <strong>Squash</strong><br>These commits will be combined into one commit in the base branch.
+                </a>
+              </li>
+              <li>
+                <a href="javascript:void(0);" class="merge-strategy" data-value="rebase">
+                  <strong>Rebase</strong><br>These commits will be rebased and added to the base branch.
+                </a>
+              </li>
+            </ul>
+          </div>
+          <div class="pull-right">
             <input type="button" class="btn btn-default" value="Cancel" id="cancel-merge-pull-request"/>
             <input type="submit" class="btn btn-success" value="Confirm merge"/>
+            <input type="hidden" name="strategy" value="merge-commit"/>
+          </div>
         </div>
       </form>
     </div>
@@ -194,5 +220,10 @@ $(function(){
       $('#merge-command-copy-1').attr('data-clipboard-text', $('#merge-command').text());
     });
   }
+
+  $('.merge-strategy').click(function(){
+    $('button#merge-strategy-btn > span.strong').text($(this).find('strong').text());
+    $('input[name=strategy]').val($(this).data('value'));
+  });
 });
 </script>