Improve handling of checkout conflicts

This converts a checkout conflict exception into a RebaseResult /
MergeResult containing the conflicting paths, which enables EGit (or
others) to handle the situation in a user-friendly way

Change-Id: I48d9bdcc1e98095576513a54a225a42409f301f3

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
index ba97e905b3226f63daf4b8ea39fcb62658158161..fbaef76e1c2bc7d67a5ba8f2c35c4c39b10c6385 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
@@ -1059,7 +1059,6 @@ public class RebaseCommandTest extends RepositoryTestCase {
        }
 
        @Test
-       @SuppressWarnings("null")
        public void testRebaseWithUnstagedTopicChange() throws Exception {
                // create file1, add and commit
                writeTrashFile(FILE1, "file1");
@@ -1084,19 +1083,14 @@ public class RebaseCommandTest extends RepositoryTestCase {
                writeTrashFile("file2", "unstaged file2");
 
                // rebase
-               JGitInternalException exception = null;
-               try {
-                       git.rebase().setUpstream("refs/heads/master").call();
-               } catch (JGitInternalException e) {
-                       exception = e;
-               }
-               assertNotNull(exception);
-               assertEquals("Checkout conflict with files: \nfile2",
-                               exception.getMessage());
+               RebaseResult result = git.rebase().setUpstream("refs/heads/master")
+                               .call();
+               assertEquals(Status.CONFLICTS, result.getStatus());
+               assertEquals(1, result.getConflicts().size());
+               assertEquals("file2", result.getConflicts().get(0));
        }
 
        @Test
-       @SuppressWarnings("null")
        public void testRebaseWithUncommittedTopicChange() throws Exception {
                // create file1, add and commit
                writeTrashFile(FILE1, "file1");
@@ -1122,23 +1116,17 @@ public class RebaseCommandTest extends RepositoryTestCase {
                git.add().addFilepattern("file2").call();
                // do not commit
 
-               // rebase
-               JGitInternalException exception = null;
-               try {
-                       git.rebase().setUpstream("refs/heads/master").call();
-               } catch (JGitInternalException e) {
-                       exception = e;
-               }
-               assertNotNull(exception);
-               assertEquals("Checkout conflict with files: \nfile2",
-                               exception.getMessage());
+               RebaseResult result = git.rebase().setUpstream("refs/heads/master")
+                               .call();
+               assertEquals(Status.CONFLICTS, result.getStatus());
+               assertEquals(1, result.getConflicts().size());
+               assertEquals("file2", result.getConflicts().get(0));
 
                checkFile(uncommittedFile, "uncommitted file2");
                assertEquals(RepositoryState.SAFE, git.getRepository().getRepositoryState());
        }
 
        @Test
-       @SuppressWarnings("null")
        public void testRebaseWithUnstagedMasterChange() throws Exception {
                // create file1, add and commit
                writeTrashFile(FILE1, "file1");
@@ -1163,19 +1151,14 @@ public class RebaseCommandTest extends RepositoryTestCase {
                writeTrashFile(FILE1, "unstaged modified file1");
 
                // rebase
-               JGitInternalException exception = null;
-               try {
-                       git.rebase().setUpstream("refs/heads/master").call();
-               } catch (JGitInternalException e) {
-                       exception = e;
-               }
-               assertNotNull(exception);
-               assertEquals("Checkout conflict with files: \nfile1",
-                               exception.getMessage());
+               RebaseResult result = git.rebase().setUpstream("refs/heads/master")
+                               .call();
+               assertEquals(Status.CONFLICTS, result.getStatus());
+               assertEquals(1, result.getConflicts().size());
+               assertEquals(FILE1, result.getConflicts().get(0));
        }
 
        @Test
-       @SuppressWarnings("null")
        public void testRebaseWithUncommittedMasterChange() throws Exception {
                // create file1, add and commit
                writeTrashFile(FILE1, "file1");
@@ -1202,15 +1185,11 @@ public class RebaseCommandTest extends RepositoryTestCase {
                // do not commit
 
                // rebase
-               JGitInternalException exception = null;
-               try {
-                       git.rebase().setUpstream("refs/heads/master").call();
-               } catch (JGitInternalException e) {
-                       exception = e;
-               }
-               assertNotNull(exception);
-               assertEquals("Checkout conflict with files: \nfile1",
-                               exception.getMessage());
+               RebaseResult result = git.rebase().setUpstream("refs/heads/master")
+                               .call();
+               assertEquals(Status.CONFLICTS, result.getStatus());
+               assertEquals(1, result.getConflicts().size());
+               assertEquals(FILE1, result.getConflicts().get(0));
        }
 
        @Test
@@ -1496,14 +1475,13 @@ public class RebaseCommandTest extends RepositoryTestCase {
                File theFile = writeTrashFile(FILE1, "dirty the file");
 
                // and attempt to rebase
-               try {
-                       RebaseResult rebaseResult = git.rebase()
+               RebaseResult rebaseResult = git.rebase()
                                        .setUpstream("refs/heads/master").call();
-                       fail("Checkout with conflict should have occured, not "
-                                       + rebaseResult.getStatus());
-               } catch (JGitInternalException e) {
-                       checkFile(theFile, "dirty the file");
-               }
+               assertEquals(Status.CONFLICTS, rebaseResult.getStatus());
+               assertEquals(1, rebaseResult.getConflicts().size());
+               assertEquals(FILE1, rebaseResult.getConflicts().get(0));
+
+               checkFile(theFile, "dirty the file");
 
                assertEquals(RepositoryState.SAFE, git.getRepository()
                                .getRepositoryState());

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeResult.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeResult.java
index 1c930ebeb33aeea24ff7f98b8939d06b8731b240..82272168aa71fbaf9980f73e6972c55979d7b2e4 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeResult.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/MergeResult.java
@@ -45,6 +45,7 @@ package org.eclipse.jgit.api;
 
 import java.text.MessageFormat;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import org.eclipse.jgit.internal.JGitText;
@@ -173,6 +174,21 @@ public class MergeResult {
                                return "Not-yet-supported";
                        }
 
+                       @Override
+                       public boolean isSuccessful() {
+                               return false;
+                       }
+               },
+               /**
+                * Status representing a checkout conflict, meaning that nothing could
+                * be merged, as the pre-scan for the trees already failed for certain
+                * files (i.e. local modifications prevent checkout of files).
+                */
+               CHECKOUT_CONFLICT {
+                       public String toString() {
+                               return "Checkout Conflict";
+                       }
+
                        @Override
                        public boolean isSuccessful() {
                                return false;
@@ -201,6 +217,8 @@ public class MergeResult {
 
        private Map<String, MergeFailureReason> failingPaths;
 
+       private List<String> checkoutConflicts;
+
        /**
         * @param newHead
         *            the object the head points at after the merge
@@ -294,6 +312,18 @@ public class MergeResult {
                                addConflict(result.getKey(), result.getValue());
        }
 
+       /**
+        * Creates a new result that represents a checkout conflict before the
+        * operation even started for real.
+        *
+        * @param checkoutConflicts
+        *            the conflicting files
+        */
+       public MergeResult(List<String> checkoutConflicts) {
+               this.checkoutConflicts = checkoutConflicts;
+               this.mergeStatus = MergeStatus.CHECKOUT_CONFLICT;
+       }
+
        /**
         * @return the object the head points at after the merge
         */
@@ -450,4 +480,14 @@ public class MergeResult {
        public Map<String, MergeFailureReason> getFailingPaths() {
                return failingPaths;
        }
+
+       /**
+        * Returns a list of paths that cause a checkout conflict. These paths
+        * prevent the operation from even starting.
+        *
+        * @return the list of files that caused the checkout conflict.
+        */
+       public List<String> getCheckoutConflicts() {
+               return checkoutConflicts;
+       }
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
index 3f7feb936f4fbba37c3ff2c4253d1a0755e7c763..5158c8533bcbb4a6dff37bee907c17a4157f69f1 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
@@ -355,6 +355,8 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
                                return RebaseResult.OK_RESULT;
                        }
                        return RebaseResult.FAST_FORWARD_RESULT;
+               } catch (CheckoutConflictException cce) {
+                       return new RebaseResult(cce.getConflictingPaths());
                } catch (IOException ioe) {
                        throw new JGitInternalException(ioe.getMessage(), ioe);
                }
@@ -880,13 +882,18 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
                return RawParseUtils.decode(content, 0, end);
        }
 
-       private boolean checkoutCommit(RevCommit commit) throws IOException {
+       private boolean checkoutCommit(RevCommit commit) throws IOException,
+                       CheckoutConflictException {
                try {
                        RevCommit head = walk.parseCommit(repo.resolve(Constants.HEAD));
                        DirCacheCheckout dco = new DirCacheCheckout(repo, head.getTree(),
                                        repo.lockDirCache(), commit.getTree());
                        dco.setFailOnConflict(true);
-                       dco.checkout();
+                       try {
+                               dco.checkout();
+                       } catch (org.eclipse.jgit.errors.CheckoutConflictException cce) {
+                               throw new CheckoutConflictException(dco.getConflicts(), cce);
+                       }
                        // update the HEAD
                        RefUpdate refUpdate = repo.updateRef(Constants.HEAD, true);
                        refUpdate.setExpectedOldObjectId(head);

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseResult.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseResult.java
index a09f8c28afe001f66111f969cec6bc331ad4dc74..59655570c0a23323027327a9beb6a45f33ee6a08 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseResult.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseResult.java
@@ -42,6 +42,7 @@
  */
 package org.eclipse.jgit.api;
 
+import java.util.List;
 import java.util.Map;
 
 import org.eclipse.jgit.merge.ResolveMerger;
@@ -92,6 +93,15 @@ public class RebaseResult {
                                return false;
                        }
                },
+               /**
+                * Conflicts: checkout of target HEAD failed
+                */
+               CONFLICTS {
+                       @Override
+                       public boolean isSuccessful() {
+                               return false;
+                       }
+               },
                /**
                 * Already up-to-date
                 */
@@ -148,6 +158,8 @@ public class RebaseResult {
 
        private Map<String, MergeFailureReason> failingPaths;
 
+       private List<String> conflicts;
+
        private RebaseResult(Status status) {
                this.status = status;
                currentCommit = null;
@@ -176,6 +188,18 @@ public class RebaseResult {
                this.failingPaths = failingPaths;
        }
 
+       /**
+        * Create <code>RebaseResult</code> with status {@link Status#CONFLICTS}
+        *
+        * @param conflicts
+        *            the list of conflicting paths
+        */
+       RebaseResult(List<String> conflicts) {
+               status = Status.CONFLICTS;
+               currentCommit = null;
+               this.conflicts = conflicts;
+       }
+
        /**
         * @return the overall status
         */
@@ -199,4 +223,11 @@ public class RebaseResult {
        public Map<String, MergeFailureReason> getFailingPaths() {
                return failingPaths;
        }
+
+       /**
+        * @return the list of conflicts if status is {@link Status#CONFLICTS}
+        */
+       public List<String> getConflicts() {
+               return conflicts;
+       }
 }