Merge "Improve handling of checkout conflicts"

4 files changed, 106 insertions, 50 deletions

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 ba97e905b3..fbaef76e1c 100644
--- 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 1c930ebeb3..82272168aa 100644
--- 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;
@@ -177,6 +178,21 @@ public class MergeResult {
 			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
@@ -295,6 +313,18 @@ public class MergeResult {
 	}
 
 	/**
+	 * 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
 	 */
 	public ObjectId getNewHead() {
@@ -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 3f7feb936f..5158c8533b 100644
--- 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 a09f8c28af..59655570c0 100644
--- 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;
@@ -93,6 +94,15 @@ public class RebaseResult {
 			}
 		},
 		/**
+		 * Conflicts: checkout of target HEAD failed
+		 */
+		CONFLICTS {
+			@Override
+			public boolean isSuccessful() {
+				return false;
+			}
+		},
+		/**
 		 * Already up-to-date
 		 */
 		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;
@@ -177,6 +189,18 @@ public class RebaseResult {
 	}
 
 	/**
+	 * 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
 	 */
 	public Status getStatus() {
@@ -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;
+	}
 }