Fix NPE in DirCacheCheckout

If a file exists in head, merge, and the working tree, but not in
the index, and we're doing a force checkout, the checkout must be
an "update", not a "keep".

This is a follow-up on If3a9b9e60064459d187c7db04eb4471a72c6cece.

Bug: 569962
Change-Id: I59a7ac41898ddc1dd90e86b09b621a41fdf45667
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CheckoutCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CheckoutCommandTest.java
index 0a0a88c83821a8aa15bec0eed6a99a6645cb6d54..e520732513a814612303844f51d503f55c27f1a5 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CheckoutCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CheckoutCommandTest.java
@@ -146,6 +146,55 @@ public class CheckoutCommandTest extends RepositoryTestCase {
                assertTrue(testFile.exists());
        }
 
+       @Test
+       public void testCheckoutForcedNoChangeNotInIndex() throws Exception {
+               git.checkout().setCreateBranch(true).setName("test2").call();
+               File f = writeTrashFile("NewFile.txt", "New file");
+               git.add().addFilepattern("NewFile.txt").call();
+               git.commit().setMessage("New file created").call();
+               git.checkout().setName("test").call();
+               assertFalse("NewFile.txt should not exist", f.exists());
+               writeTrashFile("NewFile.txt", "New file");
+               git.add().addFilepattern("NewFile.txt").call();
+               git.commit().setMessage("New file created again with same content")
+                               .call();
+               // Now remove the file from the index only. So it exists in both
+               // commits, and in the working tree, but not in the index.
+               git.rm().addFilepattern("NewFile.txt").setCached(true).call();
+               assertTrue("NewFile.txt should exist", f.isFile());
+               git.checkout().setForced(true).setName("test2").call();
+               assertTrue("NewFile.txt should exist", f.isFile());
+               assertEquals(Constants.R_HEADS + "test2", git.getRepository()
+                               .exactRef(Constants.HEAD).getTarget().getName());
+               assertTrue("Force checkout should have undone git rm --cached",
+                               git.status().call().isClean());
+       }
+
+       @Test
+       public void testCheckoutNoChangeNotInIndex() throws Exception {
+               git.checkout().setCreateBranch(true).setName("test2").call();
+               File f = writeTrashFile("NewFile.txt", "New file");
+               git.add().addFilepattern("NewFile.txt").call();
+               git.commit().setMessage("New file created").call();
+               git.checkout().setName("test").call();
+               assertFalse("NewFile.txt should not exist", f.exists());
+               writeTrashFile("NewFile.txt", "New file");
+               git.add().addFilepattern("NewFile.txt").call();
+               git.commit().setMessage("New file created again with same content")
+                               .call();
+               // Now remove the file from the index only. So it exists in both
+               // commits, and in the working tree, but not in the index.
+               git.rm().addFilepattern("NewFile.txt").setCached(true).call();
+               assertTrue("NewFile.txt should exist", f.isFile());
+               git.checkout().setName("test2").call();
+               assertTrue("NewFile.txt should exist", f.isFile());
+               assertEquals(Constants.R_HEADS + "test2", git.getRepository()
+                               .exactRef(Constants.HEAD).getTarget().getName());
+               org.eclipse.jgit.api.Status status = git.status().call();
+               assertEquals("[NewFile.txt]", status.getRemoved().toString());
+               assertEquals("[NewFile.txt]", status.getUntracked().toString());
+       }
+
        @Test
        public void testCreateBranchOnCheckout() throws Exception {
                git.checkout().setCreateBranch(true).setName("test2").call();

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 9dfceae3458078614bd32b8dca14442d6a93805e..b943486b1b668c09817b5e7901d54a9884eb91cd 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java
@@ -13,7 +13,6 @@
 package org.eclipse.jgit.lib;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static org.eclipse.jgit.dircache.DirCacheCheckout.checkoutEntry;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -48,7 +47,6 @@ import org.eclipse.jgit.errors.CorruptObjectException;
 import org.eclipse.jgit.errors.NoWorkTreeException;
 import org.eclipse.jgit.events.ChangeRecorder;
 import org.eclipse.jgit.events.ListenerHandle;
-import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.junit.TestRepository.BranchBuilder;
@@ -2148,11 +2146,4 @@ public class DirCacheCheckoutTest extends RepositoryTestCase {
                        assertEquals("WorkDir has not the right size.", i.size(), nrFiles);
                }
        }
-
-       @Test
-       public void shouldReturnAndNotThrowNPEWhenCheckoutEntryIsCalledWithNullEntry() throws Exception{
-               checkoutEntry(new InMemoryRepository(null), null, null, true, new CheckoutMetadata(null, null));
-       }
-
-
 }

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 344626de3786be1a49e1251aea6eb21f20e024fe..671475ed47fd691c01192035cc4bc8ae69734dcc 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
@@ -946,12 +946,14 @@ public class DirCacheCheckout {
                                // called before). Ignore the cached deletion and use what we
                                // find in Merge. Potentially updates the file.
                                if (equalIdAndMode(hId, hMode, mId, mMode)) {
-                                       if (initialCheckout)
+                                       if (initialCheckout || force) {
                                                update(name, mId, mMode);
-                                       else
+                                       } else {
                                                keep(name, dce, f);
-                               } else
+                                       }
+                               } else {
                                        conflict(name, dce, h, m);
+                               }
                        }
                } else {
                        // Something in Index
@@ -1214,10 +1216,13 @@ public class DirCacheCheckout {
 
        private void keep(String path, DirCacheEntry e, WorkingTreeIterator f)
                        throws IOException {
-               if (e != null && !FileMode.TREE.equals(e.getFileMode())) {
+               if (e == null) {
+                       return;
+               }
+               if (!FileMode.TREE.equals(e.getFileMode())) {
                        builder.add(e);
                }
-               if (e != null && force) {
+               if (force) {
                        if (f == null || f.isModified(e, true, walk.getObjectReader())) {
                                kept.add(path);
                                checkoutEntry(repo, e, walk.getObjectReader(), false,
@@ -1448,9 +1453,6 @@ public class DirCacheCheckout {
        public static void checkoutEntry(Repository repo, DirCacheEntry entry,
                        ObjectReader or, boolean deleteRecursive,
                        CheckoutMetadata checkoutMetadata) throws IOException {
-               if (entry == null) {
-                       return;
-               }
                if (checkoutMetadata == null)
                        checkoutMetadata = CheckoutMetadata.EMPTY;
                ObjectLoader ol = or.open(entry.getObjectId());