From baf7ca9cc0d2bdf237a9bb19486d6fde5250a7f5 Mon Sep 17 00:00:00 2001 From: Markus Duft Date: Fri, 13 Jul 2012 08:25:25 +0200 Subject: [PATCH] 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 --- .../eclipse/jgit/api/RebaseCommandTest.java | 74 +++++++------------ .../src/org/eclipse/jgit/api/MergeResult.java | 40 ++++++++++ .../org/eclipse/jgit/api/RebaseCommand.java | 11 ++- .../org/eclipse/jgit/api/RebaseResult.java | 31 ++++++++ 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 ba97e905b..fbaef76e1 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 1c930ebeb..82272168a 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; @@ -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 failingPaths; + private List 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 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 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 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 3f7feb936..5158c8533 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 { 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 { 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 a09f8c28a..59655570c 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; @@ -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 failingPaths; + private List conflicts; + private RebaseResult(Status status) { this.status = status; currentCommit = null; @@ -176,6 +188,18 @@ public class RebaseResult { this.failingPaths = failingPaths; } + /** + * Create RebaseResult with status {@link Status#CONFLICTS} + * + * @param conflicts + * the list of conflicting paths + */ + RebaseResult(List conflicts) { + status = Status.CONFLICTS; + currentCommit = null; + this.conflicts = conflicts; + } + /** * @return the overall status */ @@ -199,4 +223,11 @@ public class RebaseResult { public Map getFailingPaths() { return failingPaths; } + + /** + * @return the list of conflicts if status is {@link Status#CONFLICTS} + */ + public List getConflicts() { + return conflicts; + } }