From 2d58b7f2d75589c17dd96af835e918e8cbe97d84 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Sun, 10 Dec 2017 14:59:53 +0900 Subject: [PATCH 1/7] Add a pulldown menu to choose the merge strategy --- .../controller/PullRequestsController.scala | 6 ++-- .../core/pulls/mergeguide.scala.html | 35 +++++++++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index 2f913c5ca..e43168cb3 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -50,7 +50,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 +70,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") @@ -259,6 +260,7 @@ trait PullRequestsControllerBase extends ControllerBase { recordMergeActivity(owner, name, loginAccount.userName, issueId, form.message) // merge git repository + // TODO Implement merge strategy! 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)) diff --git a/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html b/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html index 1eb4c220e..472ffc533 100644 --- a/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html +++ b/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html @@ -139,8 +139,34 @@
- - +
+ + +
+
+ + + +
@@ -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()); + $('hidden[name=strategy]').val($(this).data('value')); + }); }); From a03d1c97c23a584bddb241ea8357f084c64c8782 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Sun, 10 Dec 2017 17:39:45 +0900 Subject: [PATCH 2/7] Format code --- .../gitbucket/core/service/MergeService.scala | 71 ++++++++++++------- .../scala/gitbucket/core/util/JGitUtil.scala | 2 +- 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/src/main/scala/gitbucket/core/service/MergeService.scala b/src/main/scala/gitbucket/core/service/MergeService.scala index c67f25541..6ad30a63f 100644 --- a/src/main/scala/gitbucket/core/service/MergeService.scala +++ b/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.revwalk.RevWalk +import org.eclipse.jgit.lib.{CommitBuilder, ObjectId, PersonIdent, Repository} +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,15 @@ 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 = { - 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) } + /** 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 +49,7 @@ trait MergeService { .call } } + /** * Checks whether conflict will be caused in merging. Returns true if conflict will be caused. */ @@ -81,6 +85,7 @@ trait MergeService { } } } + /** * Checks whether conflict will be caused in merging. Returns true if conflict will be caused. */ @@ -102,7 +107,9 @@ trait MergeService { } } + object MergeService{ + object Util{ // return treeId def createMergeCommit(repository: Repository, treeId: ObjectId, committer: PersonIdent, message: String, parents: Seq[ObjectId]): ObjectId = { @@ -119,7 +126,8 @@ object MergeService{ inserter.close() mergeCommitId } - def updateRefs(repository: Repository, ref: String, newObjectId: ObjectId, force: Boolean, committer: PersonIdent, refLogMessage: Option[String] = None):Unit = { + + def updateRefs(repository: Repository, ref: String, newObjectId: ObjectId, force: Boolean, committer: PersonIdent, refLogMessage: Option[String] = None): Unit = { // update refs val refUpdate = repository.updateRef(ref) refUpdate.setNewObjectId(newObjectId) @@ -129,21 +137,25 @@ object MergeService{ refUpdate.update() } } - case class MergeCacheInfo(git:Git, branch:String, issueId:Int){ - val repository = git.getRepository - val mergedBranchName = s"refs/pull/${issueId}/merge" - val conflictedBranchName = s"refs/pull/${issueId}/conflict" + + class MergeCacheInfo(git: Git, branch: String, issueId: Int){ + + 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 => - if(parseCommit(merged).getParents().toSet == Set( mergeBaseTip, mergeTip )){ - // merged branch exists - Some(false) - } else { - None - } - }.orElse(Option(repository.resolve(conflictedBranchName)).flatMap{ conflicted => + 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 => if(parseCommit(conflicted).getParents().toSet == Set( mergeBaseTip, mergeTip )){ // conflict branch exists Some(true) @@ -152,10 +164,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,11 +178,13 @@ 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) git.branchDelete().setForce(true).setBranchNames(conflictedBranchName).call() @@ -178,21 +194,26 @@ object MergeService{ } conflicted } + // update branch from cache - def merge(message:String, committer:PersonIdent) = { + def merge(message: String, committer: PersonIdent) = { 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")) } + // 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)) + } } diff --git a/src/main/scala/gitbucket/core/util/JGitUtil.scala b/src/main/scala/gitbucket/core/util/JGitUtil.scala index fd70e67bb..e34627eac 100644 --- a/src/main/scala/gitbucket/core/util/JGitUtil.scala +++ b/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( From fcb374c5c29fb873ec426e63afbf1b0b473fcd1c Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Sun, 10 Dec 2017 18:04:45 +0900 Subject: [PATCH 3/7] Implemented rebase strategy --- .../controller/PullRequestsController.scala | 15 ++++++++++++--- .../gitbucket/core/service/MergeService.scala | 17 +++++++++++++++-- .../gitbucket/core/pulls/mergeguide.scala.html | 2 +- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index e43168cb3..4a8d39f76 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -261,9 +261,18 @@ trait PullRequestsControllerBase extends ControllerBase { // merge git repository // TODO Implement merge strategy! - 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)) + println(form.strategy) + form.strategy match { + case "merge-commit" => + println("** 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" => + println("** rebase **") + rebasePullRequest(git, pullreq.branch, issueId, + new PersonIdent(loginAccount.fullName, loginAccount.mailAddress)) + } val (commits, _) = getRequestCompareInfo(owner, name, pullreq.commitIdFrom, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo) diff --git a/src/main/scala/gitbucket/core/service/MergeService.scala b/src/main/scala/gitbucket/core/service/MergeService.scala index 6ad30a63f..770fef237 100644 --- a/src/main/scala/gitbucket/core/service/MergeService.scala +++ b/src/main/scala/gitbucket/core/service/MergeService.scala @@ -35,11 +35,16 @@ trait MergeService { } } - /** merge pull request */ - def mergePullRequest(git:Git, branch: String, issueId: Int, message:String, committer: PersonIdent): Unit = { + /** merge the pull request with a merge commit */ + def mergePullRequest(git: Git, branch: String, issueId: Int, message: String, committer: PersonIdent): Unit = { new MergeCacheInfo(git, branch, issueId).merge(message, committer) } + /** rebase to the pull request branch */ + def rebasePullRequest(git: Git, branch: String, issueId: Int, committer: PersonIdent): Unit = { + new MergeCacheInfo(git, branch, issueId).rebase(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 => @@ -209,6 +214,14 @@ object MergeService{ Util.updateRefs(repository, s"refs/heads/${branch}", mergeCommitId, false, committer, Some("merged")) } + def rebase(committer: PersonIdent) = { + if(checkConflict()){ + throw new RuntimeException("This pull request can't merge automatically.") + } + val mergeTipCommit = using(new RevWalk( repository ))(_.parseCommit( mergeTip )) + Util.updateRefs(repository, s"refs/heads/${branch}", mergeTipCommit.getId, false, committer, Some("merged")) + } + // return treeId private def createMergeCommit(treeId: ObjectId, committer: PersonIdent, message: String) = Util.createMergeCommit(repository, treeId, committer, message, Seq[ObjectId](mergeBaseTip, mergeTip)) diff --git a/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html b/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html index 472ffc533..0dcf106d7 100644 --- a/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html +++ b/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html @@ -223,7 +223,7 @@ $(function(){ $('.merge-strategy').click(function(){ $('button#merge-strategy-btn > span.strong').text($(this).find('strong').text()); - $('hidden[name=strategy]').val($(this).data('value')); + $('input[name=strategy]').val($(this).data('value')); }); }); From eb61bc50d6d2b1d88f8827d973eee9880e1db4b9 Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Mon, 11 Dec 2017 12:32:52 +0900 Subject: [PATCH 4/7] Implemented squash merge strategy --- .../controller/PullRequestsController.scala | 7 ++- .../gitbucket/core/service/MergeService.scala | 63 ++++++++++++++----- 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index 4a8d39f76..8819c412d 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -260,18 +260,19 @@ trait PullRequestsControllerBase extends ControllerBase { recordMergeActivity(owner, name, loginAccount.userName, issueId, form.message) // merge git repository - // TODO Implement merge strategy! println(form.strategy) form.strategy match { case "merge-commit" => - println("** 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" => - println("** rebase **") rebasePullRequest(git, pullreq.branch, issueId, new PersonIdent(loginAccount.fullName, loginAccount.mailAddress)) + case "squash" => + squashPullRequest(git, pullreq.branch, issueId, + s"Merge pull request #${issueId} from ${pullreq.requestUserName}/${pullreq.requestBranch}\n\n" + form.message, + new PersonIdent(loginAccount.fullName, loginAccount.mailAddress)) } val (commits, _) = getRequestCompareInfo(owner, name, pullreq.commitIdFrom, diff --git a/src/main/scala/gitbucket/core/service/MergeService.scala b/src/main/scala/gitbucket/core/service/MergeService.scala index 770fef237..046b54fac 100644 --- a/src/main/scala/gitbucket/core/service/MergeService.scala +++ b/src/main/scala/gitbucket/core/service/MergeService.scala @@ -8,7 +8,7 @@ import org.eclipse.jgit.api.Git import org.eclipse.jgit.transport.RefSpec import org.eclipse.jgit.errors.NoMergeBaseException import org.eclipse.jgit.lib.{CommitBuilder, ObjectId, PersonIdent, Repository} -import org.eclipse.jgit.revwalk.{RevCommit, RevWalk} +import org.eclipse.jgit.revwalk.RevWalk trait MergeService { import MergeService._ @@ -45,6 +45,10 @@ trait MergeService { new MergeCacheInfo(git, branch, issueId).rebase(committer) } + 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 => @@ -101,14 +105,15 @@ 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) => - 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)) - Util.updateRefs(git.getRepository, s"refs/heads/${localBranch}", newCommit, false, committer, Some("merge")) + 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)) + Util.updateRefs(git.getRepository, s"refs/heads/${localBranch}", newCommit, false, committer, Some("merge")) + } + oldBaseId } - oldBaseId - } } } @@ -184,24 +189,24 @@ 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.") } @@ -211,15 +216,43 @@ object MergeService{ // 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")) + Util.updateRefs(repository, s"refs/heads/${branch}", mergeCommitId, false, committer, Some("merged")) // TODO reflog message } - def rebase(committer: PersonIdent) = { + def rebase(committer: PersonIdent): Unit = { if(checkConflict()){ throw new RuntimeException("This pull request can't merge automatically.") } val mergeTipCommit = using(new RevWalk( repository ))(_.parseCommit( mergeTip )) - Util.updateRefs(repository, s"refs/heads/${branch}", mergeTipCommit.getId, false, committer, Some("merged")) + Util.updateRefs(repository, s"refs/heads/${branch}", mergeTipCommit.getId, false, committer, Some("merged")) // TODO reflog message + } + + def squash(message: String, committer: PersonIdent): Unit = { + if(checkConflict()){ + throw new RuntimeException("This pull request can't merge automatically.") + } + + val baseCommit = 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(baseCommit) + mergeCommit.setAuthor(mergeBranchHeadCommit.getAuthorIdent) + mergeCommit.setCommitter(committer) + mergeCommit.setMessage(message) + + // insertObject and got squash commit Object Id + val inserter = repository.newObjectInserter + val newCommitId = inserter.insert(mergeCommit) + inserter.flush() + inserter.close() + + Util.updateRefs(repository, mergedBranchName, newCommitId, true, committer) + + // rebase to squash commit + Util.updateRefs(repository, s"refs/heads/${branch}", repository.resolve(mergedBranchName), false, committer, Some("merged")) // TODO reflog message } // return treeId From 0309496df6e9b3d564c2287dc76f2106e4f2b6bf Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Mon, 11 Dec 2017 20:08:03 +0900 Subject: [PATCH 5/7] Fix rebase process --- .../controller/PullRequestsController.scala | 18 ++++--- .../gitbucket/core/service/MergeService.scala | 47 ++++++++++++++----- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index 8819c412d..5e6bd37d9 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/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._ @@ -259,25 +260,30 @@ 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 - println(form.strategy) 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" => - rebasePullRequest(git, pullreq.branch, issueId, + rebasePullRequest(git, pullreq.branch, issueId, revCommits, new PersonIdent(loginAccount.fullName, loginAccount.mailAddress)) case "squash" => squashPullRequest(git, pullreq.branch, issueId, - s"Merge pull request #${issueId} from ${pullreq.requestUserName}/${pullreq.requestBranch}\n\n" + form.message, + s"${issue.title} (#${issueId})\n\n" + form.message, new PersonIdent(loginAccount.fullName, loginAccount.mailAddress)) } - val (commits, _) = getRequestCompareInfo(owner, name, pullreq.commitIdFrom, - pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo) - // close issue by content of pull request val defaultBranch = getRepository(owner, name).get.repository.defaultBranch if(pullreq.branch == defaultBranch){ diff --git a/src/main/scala/gitbucket/core/service/MergeService.scala b/src/main/scala/gitbucket/core/service/MergeService.scala index 046b54fac..1a80bfa1d 100644 --- a/src/main/scala/gitbucket/core/service/MergeService.scala +++ b/src/main/scala/gitbucket/core/service/MergeService.scala @@ -8,7 +8,7 @@ import org.eclipse.jgit.api.Git import org.eclipse.jgit.transport.RefSpec import org.eclipse.jgit.errors.NoMergeBaseException 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._ @@ -40,11 +40,12 @@ trait MergeService { new MergeCacheInfo(git, branch, issueId).merge(message, committer) } - /** rebase to the pull request branch */ - def rebasePullRequest(git: Git, branch: String, issueId: Int, committer: PersonIdent): Unit = { - new MergeCacheInfo(git, branch, issueId).rebase(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) } @@ -205,7 +206,6 @@ object MergeService{ conflicted } - // update branch from cache def merge(message: String, committer: PersonIdent): Unit = { if(checkConflict()){ throw new RuntimeException("This pull request can't merge automatically.") @@ -219,12 +219,37 @@ object MergeService{ Util.updateRefs(repository, s"refs/heads/${branch}", mergeCommitId, false, committer, Some("merged")) // TODO reflog message } - def rebase(committer: PersonIdent): Unit = { + def rebase(committer: PersonIdent, commits: Seq[RevCommit]): Unit = { if(checkConflict()){ throw new RuntimeException("This pull request can't merge automatically.") } - val mergeTipCommit = using(new RevWalk( repository ))(_.parseCommit( mergeTip )) - Util.updateRefs(repository, s"refs/heads/${branch}", mergeTipCommit.getId, false, committer, Some("merged")) // TODO reflog message + + 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 + + val inserter = repository.newObjectInserter + + commits.foreach { commit => + val nextCommit = _cloneCommit(commit, Array(previousId)) + previousId = inserter.insert(nextCommit) + } + + inserter.flush() + inserter.close() + + Util.updateRefs(repository, s"refs/heads/${branch}", previousId, false, committer, Some("merged")) // TODO reflog message } def squash(message: String, committer: PersonIdent): Unit = { @@ -232,13 +257,13 @@ object MergeService{ throw new RuntimeException("This pull request can't merge automatically.") } - val baseCommit = using(new RevWalk( repository ))(_.parseCommit(mergeBaseTip)) + 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(baseCommit) + mergeCommit.setParentId(mergeBaseTipCommit) mergeCommit.setAuthor(mergeBranchHeadCommit.getAuthorIdent) mergeCommit.setCommitter(committer) mergeCommit.setMessage(message) @@ -259,7 +284,7 @@ object MergeService{ 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)) } } From 1adc9b3223e547f84e6514a98b7ded95c9ac68bf Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Mon, 11 Dec 2017 20:20:58 +0900 Subject: [PATCH 6/7] Refactoring --- .../gitbucket/core/service/MergeService.scala | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/main/scala/gitbucket/core/service/MergeService.scala b/src/main/scala/gitbucket/core/service/MergeService.scala index 1a80bfa1d..afefae97d 100644 --- a/src/main/scala/gitbucket/core/service/MergeService.scala +++ b/src/main/scala/gitbucket/core/service/MergeService.scala @@ -122,7 +122,7 @@ 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) @@ -131,15 +131,14 @@ object MergeService{ mergeCommit.setCommitter(committer) mergeCommit.setMessage(message) // insertObject and got mergeCommit Object Id - val inserter = repository.newObjectInserter - val mergeCommitId = inserter.insert(mergeCommit) - inserter.flush() - inserter.close() - mergeCommitId + using(repository.newObjectInserter){ inserter => + val mergeCommitId = inserter.insert(mergeCommit) + inserter.flush() + mergeCommitId + } } def updateRefs(repository: Repository, ref: String, newObjectId: ObjectId, force: Boolean, committer: PersonIdent, refLogMessage: Option[String] = None): Unit = { - // update refs val refUpdate = repository.updateRef(ref) refUpdate.setNewObjectId(newObjectId) refUpdate.setForceUpdate(force) @@ -239,16 +238,14 @@ object MergeService{ val mergeBaseTipCommit = using(new RevWalk( repository ))(_.parseCommit( mergeBaseTip )) var previousId = mergeBaseTipCommit.getId - val inserter = repository.newObjectInserter - - commits.foreach { commit => - val nextCommit = _cloneCommit(commit, Array(previousId)) - previousId = inserter.insert(nextCommit) + using(repository.newObjectInserter){ inserter => + commits.foreach { commit => + val nextCommit = _cloneCommit(commit, Array(previousId)) + previousId = inserter.insert(nextCommit) + } + inserter.flush() } - inserter.flush() - inserter.close() - Util.updateRefs(repository, s"refs/heads/${branch}", previousId, false, committer, Some("merged")) // TODO reflog message } @@ -269,10 +266,11 @@ object MergeService{ mergeCommit.setMessage(message) // insertObject and got squash commit Object Id - val inserter = repository.newObjectInserter - val newCommitId = inserter.insert(mergeCommit) - inserter.flush() - inserter.close() + val newCommitId = using(repository.newObjectInserter){ inserter => + val newCommitId = inserter.insert(mergeCommit) + inserter.flush() + newCommitId + } Util.updateRefs(repository, mergedBranchName, newCommitId, true, committer) From 9a127256f33fbb26a937df5609e90be2def261ca Mon Sep 17 00:00:00 2001 From: Naoki Takezoe Date: Tue, 12 Dec 2017 11:39:00 +0900 Subject: [PATCH 7/7] Update reflog message --- src/main/scala/gitbucket/core/service/MergeService.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/scala/gitbucket/core/service/MergeService.scala b/src/main/scala/gitbucket/core/service/MergeService.scala index afefae97d..2f609bd99 100644 --- a/src/main/scala/gitbucket/core/service/MergeService.scala +++ b/src/main/scala/gitbucket/core/service/MergeService.scala @@ -215,7 +215,7 @@ object MergeService{ // 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")) // TODO reflog message + Util.updateRefs(repository, s"refs/heads/${branch}", mergeCommitId, false, committer, Some("merged")) } def rebase(committer: PersonIdent, commits: Seq[RevCommit]): Unit = { @@ -246,7 +246,7 @@ object MergeService{ inserter.flush() } - Util.updateRefs(repository, s"refs/heads/${branch}", previousId, false, committer, Some("merged")) // TODO reflog message + Util.updateRefs(repository, s"refs/heads/${branch}", previousId, false, committer, Some("rebased")) } def squash(message: String, committer: PersonIdent): Unit = { @@ -275,7 +275,7 @@ object MergeService{ Util.updateRefs(repository, mergedBranchName, newCommitId, true, committer) // rebase to squash commit - Util.updateRefs(repository, s"refs/heads/${branch}", repository.resolve(mergedBranchName), false, committer, Some("merged")) // TODO reflog message + Util.updateRefs(repository, s"refs/heads/${branch}", repository.resolve(mergedBranchName), false, committer, Some("squashed")) } // return treeId