DirCacheCheckout: Fix handling of files not in index

When a file is not in the index and neither contents nor mode differ
between "head" and "merge", the index state should be kept. If they
differ, a checkout conflict should occur. This is described in Git's
git-read-tree.txt.

JGit used to replace the index state with "merge" in both of the above
cases.

A confusing effect of this was that when one removed a file and then did
a rebase, the file silently reappeared again.

The changes to dir/file conflict handling are a consequence of this
change, as the index handling change made tests in DirCacheCheckoutTest
break. I compared these cases to C Git and the new behavior there also
matches what C Git does.

Bug: 387390
Change-Id: I5beb781f12172a68f98c67d4c8029eb51ceae62d
Signed-off-by: Robin Stocker <robin@nibor.org>

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 dbad322be20c448919f396cef1e34edfd367f537..81730b93aa384013d5653074a37a50d8673ac2bf 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
@@ -1368,6 +1368,38 @@ public class RebaseCommandTest extends RepositoryTestCase {
                assertEquals(RepositoryState.SAFE, db.getRepositoryState());
        }
 
+       @Test
+       public void testRebaseWithUncommittedDelete() throws Exception {
+               // create file0 + file1, add and commit
+               File file0 = writeTrashFile("file0", "file0");
+               writeTrashFile(FILE1, "file1");
+               git.add().addFilepattern("file0").addFilepattern(FILE1).call();
+               RevCommit commit = git.commit().setMessage("commit1").call();
+
+               // create topic branch
+               createBranch(commit, "refs/heads/topic");
+
+               // still on master / modify file1, add and commit
+               writeTrashFile(FILE1, "modified file1");
+               git.add().addFilepattern(FILE1).call();
+               git.commit().setMessage("commit2").call();
+
+               // checkout topic branch / delete file0 and add to index
+               checkoutBranch("refs/heads/topic");
+               git.rm().addFilepattern("file0").call();
+               // do not commit
+
+               // rebase
+               RebaseResult result = git.rebase().setUpstream("refs/heads/master")
+                               .call();
+               assertEquals(Status.FAST_FORWARD, result.getStatus());
+               assertFalse("File should still be deleted", file0.exists());
+               // index should only have updated file1
+               assertEquals("[file1, mode:100644, content:modified file1]",
+                               indexState(CONTENT));
+               assertEquals(RepositoryState.SAFE, db.getRepositoryState());
+       }
+
        private int countPicks() throws IOException {
                int count = 0;
                File todoFile = getTodoFile();

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 6c6c911f238aa546516c2be81f90dd5fba5b997f..23647f06d745968c374303fd24b4f0ff360d4599 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
@@ -261,11 +261,10 @@ public class DirCacheCheckoutTest extends RepositoryTestCase {
 
                merge = buildTree(mkmap("foo", "a"));
                tw = TreeWalk.forPath(db, "foo", merge);
-               ObjectId anotherId = tw.getObjectId(0);
 
                prescanTwoTrees(head, merge);
 
-               assertEquals(anotherId, getUpdated().get("foo"));
+               assertConflict("foo");
        }
 
        void setupCase(HashMap<String, String> headEntries, HashMap<String, String> mergeEntries, HashMap<String, String> indexEntries) throws IOException {
@@ -464,8 +463,8 @@ public class DirCacheCheckoutTest extends RepositoryTestCase {
         *     ------------------------------------------------------------------
         *1    D        D       F       Y         N       Y       N           Update
         *2    D        D       F       N         N       Y       N           Conflict
-        *3    D        F       D                 Y       N       N           Update
-        *4    D        F       D                 N       N       N           Update
+        *3    D        F       D                 Y       N       N           Keep
+        *4    D        F       D                 N       N       N           Conflict
         *5    D        F       F       Y         N       N       Y           Keep
         *6    D        F       F       N         N       N       Y           Keep
         *7    F        D       F       Y         Y       N       N           Update
@@ -522,18 +521,16 @@ public class DirCacheCheckoutTest extends RepositoryTestCase {
 
        @Test
        public void testDirectoryFileConflicts_3() throws Exception {
-               // 3 - the first to break!
+               // 3
                doit(mk("DF/DF"), mk("DF/DF"), mk("DF"));
-               assertUpdated("DF/DF");
-               assertRemoved("DF");
+               assertNoConflicts();
        }
 
        @Test
        public void testDirectoryFileConflicts_4() throws Exception {
                // 4 (basically same as 3, just with H and M different)
                doit(mk("DF/DF"), mkmap("DF/DF", "foo"), mk("DF"));
-               assertUpdated("DF/DF");
-               assertRemoved("DF");
+               assertConflict("DF/DF");
 
        }
 

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 d43fef84772a2a4ee8bfd41c20f06e44ad8dbb10..9780993659714026afadf9606879bd7d353ef78b 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
@@ -552,8 +552,8 @@ public class DirCacheCheckout {
                 *      ------------------------------------------------------------------
                 * 1    D        D       F       Y         N       Y       N           Update
                 * 2    D        D       F       N         N       Y       N           Conflict
-                * 3    D        F       D                 Y       N       N           Update
-                * 4    D        F       D                 N       N       N           Update
+                * 3    D        F       D                 Y       N       N           Keep
+                * 4    D        F       D                 N       N       N           Conflict
                 * 5    D        F       F       Y         N       N       Y           Keep
                 * 6    D        F       F       N         N       N       Y           Keep
                 * 7    F        D       F       Y         Y       N       N           Update
@@ -615,10 +615,7 @@ public class DirCacheCheckout {
 
                                break;
                        case 0xDFD: // 3 4
-                               // CAUTION: I put it into removed instead of updated, because
-                               // that's what our tests expect
-                               // updated.put(name, mId);
-                               remove(name);
+                               keep(dce);
                                break;
                        case 0xF0D: // 18
                                remove(name);
@@ -703,12 +700,13 @@ public class DirCacheCheckout {
 
                        /**
                         * <pre>
-                        *                  I (index)                H        M        Result
-                        *              -------------------------------------------------------
-                        *              0 nothing             nothing  nothing  (does not happen)
-                        *              1 nothing             nothing  exists   use M
-                        *              2 nothing             exists   nothing  remove path from index
-                        *              3 nothing             exists   exists   use M
+                        *                I (index)     H        M     H==M  Result
+                        *              -------------------------------------------
+                        *              0 nothing    nothing  nothing        (does not happen)
+                        *              1 nothing    nothing  exists         use M
+                        *              2 nothing    exists   nothing        remove path from index
+                        *              3 nothing    exists   exists   yes   keep index
+                        *                nothing    exists   exists   no    fail
                         * </pre>
                         */
 
@@ -716,8 +714,12 @@ public class DirCacheCheckout {
                                update(name, mId, mMode); // 1
                        else if (m == null)
                                remove(name); // 2
-                       else
-                               update(name, mId, mMode); // 3
+                       else { // 3
+                               if (equalIdAndMode(hId, hMode, mId, mMode))
+                                       keep(dce);
+                               else
+                                       conflict(name, dce, h, m);
+                       }
                } else {
                        if (h == null) {
                                /**