Browse Source

Fix DirCacheCheckout to return CheckoutConflictException

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 <ruediger.herrmann@gmx.de>
Also-by: Rüdiger Herrmann <ruediger.herrmann@gmx.de>
stable-4.5
Christian Halstrick 9 years ago
parent
commit
5fe44ed3ee
  1. 19
      org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java
  2. 58
      org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java
  3. 32
      org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java

19
org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CheckoutTest.java

@ -47,6 +47,7 @@ import static org.junit.Assert.assertEquals;
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 class CheckoutTest extends CLIRepositoryTestCase {
* <li>Checkout branch '1'
* </ol>
* <p>
* 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 class CheckoutTest extends CLIRepositoryTestCase {
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());
}
}
}

58
org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java

@ -1614,6 +1614,64 @@ public class DirCacheCheckoutTest extends RepositoryTestCase {
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<String, String> i)
throws CorruptObjectException,
IOException {

32
org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java vendored

@ -718,10 +718,23 @@ public class DirCacheCheckout {
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 @@ public class DirCacheCheckout {
}
}
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

Loading…
Cancel
Save