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>
This commit is contained in:
Thomas Wolf 2020-12-30 10:12:34 +01:00
parent 086f474054
commit 5b1a6e0e38
3 changed files with 59 additions and 17 deletions

View File

@ -146,6 +146,55 @@ public void testCheckoutForced_deleteFileAndRestore() throws Exception {
assertTrue(testFile.exists()); 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 @Test
public void testCreateBranchOnCheckout() throws Exception { public void testCreateBranchOnCheckout() throws Exception {
git.checkout().setCreateBranch(true).setName("test2").call(); git.checkout().setCreateBranch(true).setName("test2").call();

View File

@ -13,7 +13,6 @@
package org.eclipse.jgit.lib; package org.eclipse.jgit.lib;
import static java.nio.charset.StandardCharsets.UTF_8; 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.assertArrayEquals;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
@ -48,7 +47,6 @@
import org.eclipse.jgit.errors.NoWorkTreeException; import org.eclipse.jgit.errors.NoWorkTreeException;
import org.eclipse.jgit.events.ChangeRecorder; import org.eclipse.jgit.events.ChangeRecorder;
import org.eclipse.jgit.events.ListenerHandle; 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.RepositoryTestCase;
import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.junit.TestRepository.BranchBuilder; import org.eclipse.jgit.junit.TestRepository.BranchBuilder;
@ -2148,11 +2146,4 @@ public void assertWorkDir(Map<String, String> i)
assertEquals("WorkDir has not the right size.", i.size(), nrFiles); 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));
}
} }

View File

@ -946,13 +946,15 @@ else if (m == null)
// called before). Ignore the cached deletion and use what we // called before). Ignore the cached deletion and use what we
// find in Merge. Potentially updates the file. // find in Merge. Potentially updates the file.
if (equalIdAndMode(hId, hMode, mId, mMode)) { if (equalIdAndMode(hId, hMode, mId, mMode)) {
if (initialCheckout) if (initialCheckout || force) {
update(name, mId, mMode); update(name, mId, mMode);
else } else {
keep(name, dce, f); keep(name, dce, f);
} else }
} else {
conflict(name, dce, h, m); conflict(name, dce, h, m);
} }
}
} else { } else {
// Something in Index // Something in Index
if (h == null) { if (h == null) {
@ -1214,10 +1216,13 @@ private void conflict(String path, DirCacheEntry e, AbstractTreeIterator h, Abst
private void keep(String path, DirCacheEntry e, WorkingTreeIterator f) private void keep(String path, DirCacheEntry e, WorkingTreeIterator f)
throws IOException { throws IOException {
if (e != null && !FileMode.TREE.equals(e.getFileMode())) { if (e == null) {
return;
}
if (!FileMode.TREE.equals(e.getFileMode())) {
builder.add(e); builder.add(e);
} }
if (e != null && force) { if (force) {
if (f == null || f.isModified(e, true, walk.getObjectReader())) { if (f == null || f.isModified(e, true, walk.getObjectReader())) {
kept.add(path); kept.add(path);
checkoutEntry(repo, e, walk.getObjectReader(), false, checkoutEntry(repo, e, walk.getObjectReader(), false,
@ -1448,9 +1453,6 @@ public static void checkoutEntry(Repository repo, DirCacheEntry entry,
public static void checkoutEntry(Repository repo, DirCacheEntry entry, public static void checkoutEntry(Repository repo, DirCacheEntry entry,
ObjectReader or, boolean deleteRecursive, ObjectReader or, boolean deleteRecursive,
CheckoutMetadata checkoutMetadata) throws IOException { CheckoutMetadata checkoutMetadata) throws IOException {
if (entry == null) {
return;
}
if (checkoutMetadata == null) if (checkoutMetadata == null)
checkoutMetadata = CheckoutMetadata.EMPTY; checkoutMetadata = CheckoutMetadata.EMPTY;
ObjectLoader ol = or.open(entry.getObjectId()); ObjectLoader ol = or.open(entry.getObjectId());