From 5fe44ed3ee025404dc34966ec996641f47f8490b Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Fri, 17 Jun 2016 16:41:14 +0200 Subject: [PATCH] Fix DirCacheCheckout to return CheckoutConflictException MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem occurs when the checkout wants to create a file 'd/f' but the workingtree contains a dirty file 'd'. In order to create d/f the file 'd' would have to be deleted and since the file is dirty that content would be lost. This should lead to a CheckoutConflictException for d/f when failOnConflict was set to true. This fix also changes jgit checkout semantics to be more like native gits checkout semantics. If during a checkout jgit wants to delete a folder but finds that the working tree contains a dirty file at this path then JGit will now throw an exception instead of silently keeping the dirty file. Like in this example: git init touch b git add b git commit -m addB mkdir a touch a/c git add a/c git commit -m addAC rm -fr a touch a git checkout HEAD~ Change-Id: I9089123179e09dd565285d50b0caa308d290cccd Signed-off-by: Rüdiger Herrmann Also-by: Rüdiger Herrmann --- .../org/eclipse/jgit/pgm/CheckoutTest.java | 19 +++--- .../jgit/lib/DirCacheCheckoutTest.java | 58 +++++++++++++++++++ .../jgit/dircache/DirCacheCheckout.java | 32 ++++++++-- 3 files changed, 98 insertions(+), 11 deletions(-) diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java index e690ad696..3651542fc 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java @@ -47,6 +47,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.File; import java.nio.file.Files; @@ -236,8 +237,8 @@ public void testCheckoutOrphan() throws Exception { *
  • Checkout branch '1' * *

    - * The working tree should contain 'a' with FileMode.REGULAR_FILE after the - * checkout. + * The checkout has to delete folder but the workingtree contains a dirty + * file at this path. The checkout should fail like in native git. * * @throws Exception */ @@ -266,11 +267,15 @@ public void fileModeTestMissingThenFolderWithFileInWorkingTree() db.getFS()); assertEquals(FileMode.REGULAR_FILE, entry.getMode()); - git.checkout().setName(branch_1.getName()).call(); - - entry = new FileTreeIterator.FileEntry(new File(db.getWorkTree(), "a"), - db.getFS()); - assertEquals(FileMode.REGULAR_FILE, entry.getMode()); + try { + git.checkout().setName(branch_1.getName()).call(); + fail("Don't get the expected conflict"); + } catch (CheckoutConflictException e) { + assertEquals("[a]", e.getConflictingPaths().toString()); + entry = new FileTreeIterator.FileEntry( + new File(db.getWorkTree(), "a"), db.getFS()); + assertEquals(FileMode.REGULAR_FILE, entry.getMode()); + } } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java index fbe7dd041..bb553a444 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java @@ -1614,6 +1614,64 @@ public void testFileModeChangeAndContentChangeNoConflict() throws Exception { assertNotNull(git.checkout().setName(Constants.MASTER).call()); } + @Test(expected = CheckoutConflictException.class) + public void testFolderFileConflict() throws Exception { + RevCommit headCommit = commitFile("f/a", "initial content", "master"); + RevCommit checkoutCommit = commitFile("f/a", "side content", "side"); + FileUtils.delete(new File(db.getWorkTree(), "f"), FileUtils.RECURSIVE); + writeTrashFile("f", "file instead of folder"); + new DirCacheCheckout(db, headCommit.getTree(), db.lockDirCache(), + checkoutCommit.getTree()).checkout(); + } + + @Test + public void testMultipleContentConflicts() throws Exception { + commitFile("a", "initial content", "master"); + RevCommit headCommit = commitFile("b", "initial content", "master"); + commitFile("a", "side content", "side"); + RevCommit checkoutCommit = commitFile("b", "side content", "side"); + writeTrashFile("a", "changed content"); + writeTrashFile("b", "changed content"); + + try { + new DirCacheCheckout(db, headCommit.getTree(), db.lockDirCache(), + checkoutCommit.getTree()).checkout(); + fail(); + } catch (CheckoutConflictException expected) { + assertEquals(2, expected.getConflictingFiles().length); + assertTrue(Arrays.asList(expected.getConflictingFiles()) + .contains("a")); + assertTrue(Arrays.asList(expected.getConflictingFiles()) + .contains("b")); + assertEquals("changed content", read("a")); + assertEquals("changed content", read("b")); + } + } + + @Test + public void testFolderFileAndContentConflicts() throws Exception { + RevCommit headCommit = commitFile("f/a", "initial content", "master"); + commitFile("b", "side content", "side"); + RevCommit checkoutCommit = commitFile("f/a", "side content", "side"); + FileUtils.delete(new File(db.getWorkTree(), "f"), FileUtils.RECURSIVE); + writeTrashFile("f", "file instead of a folder"); + writeTrashFile("b", "changed content"); + + try { + new DirCacheCheckout(db, headCommit.getTree(), db.lockDirCache(), + checkoutCommit.getTree()).checkout(); + fail(); + } catch (CheckoutConflictException expected) { + assertEquals(2, expected.getConflictingFiles().length); + assertTrue(Arrays.asList(expected.getConflictingFiles()) + .contains("b")); + assertTrue(Arrays.asList(expected.getConflictingFiles()) + .contains("f")); + assertEquals("file instead of a folder", read("f")); + assertEquals("changed content", read("b")); + } + } + public void assertWorkDir(Map i) throws CorruptObjectException, IOException { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java index 12ceb74ab..ab61c5937 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java @@ -718,10 +718,23 @@ void processEntry(CanonicalTreeParser h, CanonicalTreeParser m, return; } - // if we have no file at all then there is nothing to do - if ((ffMask & 0x222) == 0 - && (f == null || FileMode.TREE.equals(f.getEntryFileMode()))) - return; + if ((ffMask & 0x222) == 0) { + // HEAD, MERGE and index don't contain a file (e.g. all contain a + // folder) + if (f == null || FileMode.TREE.equals(f.getEntryFileMode())) { + // the workingtree entry doesn't exist or also contains a folder + // -> no problem + return; + } else { + // the workingtree entry exists and is not a folder + if (!idEqual(h, m)) { + // Because HEAD and MERGE differ we will try to update the + // workingtree with a folder -> return a conflict + conflict(name, null, null, null); + } + return; + } + } if ((ffMask == 0x00F) && f != null && FileMode.TREE.equals(f.getEntryFileMode())) { // File/Directory conflict case #20 @@ -1004,6 +1017,17 @@ && isModified_IndexTree(name, iId, iMode, mId, mMode, } } + private static boolean idEqual(AbstractTreeIterator a, + AbstractTreeIterator b) { + if (a == b) { + return true; + } + if (a == null || b == null) { + return false; + } + return a.getEntryObjectId().equals(b.getEntryObjectId()); + } + /** * A conflict is detected - add the three different stages to the index * @param path the path of the conflicting entry