(refs #2)Add conflict checking.

2025-11-01 11:06:06 +01:00 · 2013-07-16 03:43:26 +09:00
parent e59ae9c6e9
commit f53f71ecf1
4 changed files with 103 additions and 38 deletions
--- a/src/main/scala/app/PullRequestsController.scala
+++ b/src/main/scala/app/PullRequestsController.scala
@@ -3,7 +3,6 @@ package app
 import util.{CollaboratorsAuthenticator, FileUtil, JGitUtil, ReferrerAuthenticator}
 import util.Directory._
 import service._
-import org.eclipse.jgit.treewalk.CanonicalTreeParser
 import org.eclipse.jgit.api.Git
 import jp.sf.amateras.scalatra.forms._
 import util.JGitUtil.{DiffInfo, CommitInfo}
@@ -50,9 +49,9 @@ trait PullRequestsControllerBase extends ControllerBase {
        val requestCommitId = git.getRepository.resolve(pullreq.requestBranch)

        val (commits, diffs) = if(pullreq.mergeStartId.isDefined){
-          getCompareInfo(owner, name, pullreq.mergeStartId.get, owner, name, pullreq.mergeEndId.get)
+          getCompareInfo(owner, name, pullreq.mergeStartId.get, owner, name, pullreq.mergeEndId.get, true)
        } else {
-          getCompareInfo(owner, name, pullreq.branch, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.requestBranch)
+          getCompareInfo(owner, name, pullreq.branch, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.requestBranch, false)
        }

        pulls.html.pullreq(
@@ -63,6 +62,11 @@ trait PullRequestsControllerBase extends ControllerBase {
          commits,
          diffs,
          requestCommitId.getName,
+          if(pullreq.mergeStartId.isDefined){
+            false
+          } else {
+            checkConflict(owner, name, pullreq.branch, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.requestBranch)
+          },
          hasWritePermission(owner, name, context.loginAccount),
          repository,
          s"${baseUrl}${context.path}/git/${pullreq.requestUserName}/${pullreq.requestRepositoryName}.git")
@@ -81,7 +85,7 @@ trait PullRequestsControllerBase extends ControllerBase {

      try {
        val (commits, _) = getCompareInfo(repository.owner, repository.name, pullreq.branch,
-                                  pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.requestBranch)
+                                  pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.requestBranch, false)
        mergePullRequest(repository.owner, repository.name, issueId,
          git.getRepository.resolve("master").getName,
          commits.head.head.id)
@@ -102,7 +106,10 @@ trait PullRequestsControllerBase extends ControllerBase {
          .setRemote(getRepositoryDir(pullreq.requestUserName, pullreq.requestRepositoryName).toURI.toString)
          .setRefSpecs(new RefSpec(s"refs/heads/${pullreq.branch}:refs/heads/${pullreq.requestBranch}")).call

-        git.merge.include(git.getRepository.resolve("FETCH_HEAD")).setCommit(false).call
+        val result = git.merge.include(git.getRepository.resolve("FETCH_HEAD")).setCommit(false).call
+        if(result.getConflicts != null){
+          throw new RuntimeException("This pull request can't merge automatically.")
+        }

        git.commit
          .setCommitter(new PersonIdent(loginAccount.userName, loginAccount.mailAddress))
@@ -110,6 +117,8 @@ trait PullRequestsControllerBase extends ControllerBase {
                     + form.message).call
        git.push.call

+        redirect(s"/${repository.owner}/${repository.name}/pulls/${issueId}")
+
      } finally {
        git.getRepository.close
        FileUtils.deleteDirectory(tmpdir)
@@ -117,6 +126,30 @@ trait PullRequestsControllerBase extends ControllerBase {
    } getOrElse NotFound
  })

+  private def checkConflict(userName: String, repositoryName: String, branch: String,
+                            requestUserName: String, requestRepositoryName: String, requestBranch: String): Boolean = {
+    val remote = getRepositoryDir(userName, repositoryName)
+    val tmpdir = new java.io.File(getTemporaryDir(userName, repositoryName), "merge-check")
+    val git = Git.cloneRepository.setDirectory(tmpdir).setURI(remote.toURI.toString).call
+    try {
+      git.checkout.setName(branch).call
+
+      git.fetch
+        .setRemote(getRepositoryDir(requestUserName, requestRepositoryName).toURI.toString)
+        .setRefSpecs(new RefSpec(s"refs/heads/${branch}:refs/heads/${requestBranch}")).call
+
+      val result = git.merge
+        .include(git.getRepository.resolve("FETCH_HEAD"))
+        .setCommit(false).call
+
+      result.getConflicts != null
+
+    } finally {
+      git.getRepository.close
+      FileUtils.deleteDirectory(tmpdir)
+    }
+  }
+
  get("/:owner/:repository/pulls/compare")(collaboratorsOnly { newRepo =>
    (newRepo.repository.originUserName, newRepo.repository.originRepositoryName) match {
      case (None,_)|(_, None) => NotFound // TODO BadRequest?
@@ -140,22 +173,22 @@ trait PullRequestsControllerBase extends ControllerBase {
    if(repository.repository.originUserName.isEmpty || repository.repository.originRepositoryName.isEmpty){
      NotFound // TODO BadRequest?
    } else {
-      getRepository(
-        repository.repository.originUserName.get,
-        repository.repository.originRepositoryName.get, baseUrl
-      ).map{ originRepository =>
-        val Seq(origin, originId, forkedId) = multiParams("splat")
-        val userName       = params("owner")
-        val repositoryName = params("repository")
+      val originUserName       = repository.repository.originUserName.get
+      val originRepositoryName = repository.repository.originRepositoryName.get

-        JGitUtil.withGit(getRepositoryDir(userName, repositoryName)){ git =>
+      getRepository(originUserName, originRepositoryName, baseUrl).map{ originRepository =>
+        val Seq(origin, originId, forkedId) = multiParams("splat")
+
+        JGitUtil.withGit(getRepositoryDir(repository.owner, repository.name)){ git =>
          val newId = git.getRepository.resolve(forkedId)

-          val pullreq = getCompareInfo(
+          val (commits, diffs) = getCompareInfo(
            origin, repository.repository.originRepositoryName.get, originId,
-            params("owner"), params("repository"), forkedId)
+            repository.owner, repository.name, forkedId, false)

-          pulls.html.compare(pullreq._1, pullreq._2, origin, originId, forkedId, newId.getName, repository, originRepository)
+          pulls.html.compare(commits, diffs, origin, originId, forkedId, newId.getName,
+            checkConflict(originUserName, originRepositoryName, originId, repository.owner, repository.name, forkedId),
+            repository, originRepository)
        }
      } getOrElse NotFound
    }
@@ -201,7 +234,8 @@ trait PullRequestsControllerBase extends ControllerBase {
   * Returns the commits and diffs between specified repository and revision.
   */
  private def getCompareInfo(userName: String, repositoryName: String, branch: String,
-      requestUserName: String, requestRepositoryName: String, requestBranch: String): (Seq[Seq[CommitInfo]], Seq[DiffInfo]) = {
+      requestUserName: String, requestRepositoryName: String, requestBranch: String,
+      containsLastCommit: Boolean): (Seq[Seq[CommitInfo]], Seq[DiffInfo]) = {

    import scala.collection.JavaConverters._
    import util.Implicits._
--- a/src/main/twirl/pulls/compare.scala.html
+++ b/src/main/twirl/pulls/compare.scala.html
@@ -1,5 +1,5 @@
@(commits: Seq[Seq[util.JGitUtil.CommitInfo]], diffs: Seq[util.JGitUtil.DiffInfo],
-  origin: String, originId: String, forkedId: String, commitId: String,
+  origin: String, originId: String, forkedId: String, commitId: String, hasConflict: Boolean,
  repository: service.RepositoryService.RepositoryInfo,
  originRepository: service.RepositoryService.RepositoryInfo)(implicit context: app.Context)
@import context._
@@ -33,16 +33,29 @@
    <div style="margin-bottom: 10px;" id="create-pull-request">
      <a href="#" class="btn" id="show-form">Click to create a pull request for this comparison</a>
    </div>
-    <div id="pull-request-form" style="display: none; width: 620px;">
-      <form method="POST" action="@path/@origin/@repository.name/pulls/new" validate="true">
-        <span class="error" id="error-title"></span>
-        <input type="text" name="title" style="width: 600px" placeholder="Title"/>
-        @helper.html.preview(repository, "", false, true, "width: 600px; height: 200px;")
-        <input type="hidden" name="branch" value="@originId"/>
-        <input type="hidden" name="requestUserName" value="@repository.owner"/>
-        <input type="hidden" name="requestBranch" value="@forkedId"/>
-        <input type="submit" class="btn btn-success" value="Send pull request"/>
-      </form>
+    <div id="pull-request-form" class="box" style="display: none;">
+        <div class="box-content">
+          <form method="POST" action="@path/@origin/@repository.name/pulls/new" validate="true">
+            <div style="width: 260px; position: absolute; margin-left: 635px;">
+              @if(hasConflict){
+                <h4>We can’t automatically merge these branches</h4>
+                <p>Don't worry, you can still submit the pull request.</p>
+              } else {
+                <h4 style="color: #468847;">Able to merge</h4>
+                <p>These branches can be automatically merged.</p>
+              }
+              <input type="submit" class="btn btn-success btn-block" value="Send pull request"/>
+            </div>
+            <div style="width: 620px; border-right: 1px solid #d4d4d4;">
+              <span class="error" id="error-title"></span>
+              <input type="text" name="title" style="width: 600px" placeholder="Title"/>
+              @helper.html.preview(repository, "", false, true, "width: 600px; height: 200px;")
+              <input type="hidden" name="branch" value="@originId"/>
+              <input type="hidden" name="requestUserName" value="@repository.owner"/>
+              <input type="hidden" name="requestBranch" value="@forkedId"/>
+            </div>
+          </form>
+        </div>
    </div>
  }
  @if(commits.isEmpty){
--- a/src/main/twirl/pulls/discussion.scala.html
+++ b/src/main/twirl/pulls/discussion.scala.html
@@ -3,6 +3,7 @@
  comments: List[model.IssueComment],
  collaborators: List[String],
  milestones: List[model.Milestone],
+  hasConflict: Boolean,
  hasWritePermission: Boolean,
  repository: service.RepositoryService.RepositoryInfo,
  requestRepositoryUrl: String)(implicit context: app.Context)
@@ -90,26 +91,42 @@
      </div>
      }
    }
-    @if(hasWritePermission){
+    @if(hasWritePermission && !issue.closed){
      <div class="box issue-comment-box" style="background-color: #d8f5cd;">
        <div class="box-content"class="issue-content" style="border: 1px solid #95c97e; padding: 10px;">
          <div id="merge-pull-request">
            <div class="pull-right">
-              <input type="button" class="btn btn-success" id="merge-pull-request-button" value="Merge pull request"/>
+              <input type="button" class="btn btn-success" id="merge-pull-request-button" value="Merge pull request"@if(hasConflict){ disabled="true"}/>
            </div>
            <div>
-              <strong>This pull request can be automatically merged.</strong>
+              @if(hasConflict){
+                <strong>We can’t automatically merge this pull request.</strong>
+              } else {
+                <strong>This pull request can be automatically merged.</strong>
+              }
            </div>
            <div class="small">
-              You can also merge branches on the <a href="#" id="show-command-line">command line</a>.
+              @if(hasConflict){
+                <a href="#" id="show-command-line">Use the command line</a> to resolve conflicts before continuing.
+              } else {
+                You can also merge branches on the <a href="#" id="show-command-line">command line</a>.
+              }
            </div>
            <div id="command-line" style="display: none;">
              <hr>
-              <strong>Merging via command line</strong>
-              <p>
-                If you do not want to use the merge button or an automatic merge cannot be performed,
-                you can perform a manual merge on the command line.
-              </p>
+              @if(hasConflict){
+                <strong>Checkout via command line</strong>
+                <p>
+                  If you cannot merge a pull request automatically here, you have the option of checking
+                  it out via command line to resolve conflicts and perform a manual merge.
+                </p>
+              } else {
+                <strong>Merging via command line</strong>
+                <p>
+                  If you do not want to use the merge button or an automatic merge cannot be performed,
+                  you can perform a manual merge on the command line.
+                </p>
+              }
              <div class="input-prepend">
                <span class="add-on">HTTP</span>
                <input type="text" value="@requestRepositoryUrl" id="repository-url" readonly>
--- a/src/main/twirl/pulls/pullreq.scala.html
+++ b/src/main/twirl/pulls/pullreq.scala.html
@@ -6,6 +6,7 @@
  commits: Seq[Seq[util.JGitUtil.CommitInfo]],
  diffs: Seq[util.JGitUtil.DiffInfo],
  commitId: String,
+  hasConflict: Boolean,
  hasWritePermission: Boolean,
  repository: service.RepositoryService.RepositoryInfo,
  requestRepositoryUrl: String)(implicit context: app.Context)
@@ -20,7 +21,7 @@
  </ul>
  <div class="tab-content">
    <div class="tab-pane active" id="discussion">
-      @pulls.html.discussion(issue, pullreq, comments, collaborators, milestones, hasWritePermission, repository, requestRepositoryUrl)
+      @pulls.html.discussion(issue, pullreq, comments, collaborators, milestones, hasConflict, hasWritePermission, repository, requestRepositoryUrl)
    </div>
    <div class="tab-pane" id="commits">
      @pulls.html.commits(issue, pullreq, commits, hasWritePermission, repository)