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>
This commit is contained in:
parent
0a9e010e14
commit
51c20b27ac
|
@ -1368,6 +1368,38 @@ public void testRebaseWithUncommittedMasterChangeOtherCommit()
|
||||||
assertEquals(RepositoryState.SAFE, db.getRepositoryState());
|
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 {
|
private int countPicks() throws IOException {
|
||||||
int count = 0;
|
int count = 0;
|
||||||
File todoFile = getTodoFile();
|
File todoFile = getTodoFile();
|
||||||
|
|
|
@ -261,11 +261,10 @@ public void testRules1thru3_NoIndexEntry() throws IOException {
|
||||||
|
|
||||||
merge = buildTree(mkmap("foo", "a"));
|
merge = buildTree(mkmap("foo", "a"));
|
||||||
tw = TreeWalk.forPath(db, "foo", merge);
|
tw = TreeWalk.forPath(db, "foo", merge);
|
||||||
ObjectId anotherId = tw.getObjectId(0);
|
|
||||||
|
|
||||||
prescanTwoTrees(head, merge);
|
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 {
|
void setupCase(HashMap<String, String> headEntries, HashMap<String, String> mergeEntries, HashMap<String, String> indexEntries) throws IOException {
|
||||||
|
@ -464,8 +463,8 @@ private void assertAllEmpty() {
|
||||||
* ------------------------------------------------------------------
|
* ------------------------------------------------------------------
|
||||||
*1 D D F Y N Y N Update
|
*1 D D F Y N Y N Update
|
||||||
*2 D D F N N Y N Conflict
|
*2 D D F N N Y N Conflict
|
||||||
*3 D F D Y N N Update
|
*3 D F D Y N N Keep
|
||||||
*4 D F D N N N Update
|
*4 D F D N N N Conflict
|
||||||
*5 D F F Y N N Y Keep
|
*5 D F F Y N N Y Keep
|
||||||
*6 D F F N N N Y Keep
|
*6 D F F N N N Y Keep
|
||||||
*7 F D F Y Y N N Update
|
*7 F D F Y Y N N Update
|
||||||
|
@ -522,18 +521,16 @@ public void testDirectoryFileConflicts_2() throws Exception {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testDirectoryFileConflicts_3() throws Exception {
|
public void testDirectoryFileConflicts_3() throws Exception {
|
||||||
// 3 - the first to break!
|
// 3
|
||||||
doit(mk("DF/DF"), mk("DF/DF"), mk("DF"));
|
doit(mk("DF/DF"), mk("DF/DF"), mk("DF"));
|
||||||
assertUpdated("DF/DF");
|
assertNoConflicts();
|
||||||
assertRemoved("DF");
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testDirectoryFileConflicts_4() throws Exception {
|
public void testDirectoryFileConflicts_4() throws Exception {
|
||||||
// 4 (basically same as 3, just with H and M different)
|
// 4 (basically same as 3, just with H and M different)
|
||||||
doit(mk("DF/DF"), mkmap("DF/DF", "foo"), mk("DF"));
|
doit(mk("DF/DF"), mkmap("DF/DF", "foo"), mk("DF"));
|
||||||
assertUpdated("DF/DF");
|
assertConflict("DF/DF");
|
||||||
assertRemoved("DF");
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -552,8 +552,8 @@ void processEntry(CanonicalTreeParser h, CanonicalTreeParser m,
|
||||||
* ------------------------------------------------------------------
|
* ------------------------------------------------------------------
|
||||||
* 1 D D F Y N Y N Update
|
* 1 D D F Y N Y N Update
|
||||||
* 2 D D F N N Y N Conflict
|
* 2 D D F N N Y N Conflict
|
||||||
* 3 D F D Y N N Update
|
* 3 D F D Y N N Keep
|
||||||
* 4 D F D N N N Update
|
* 4 D F D N N N Conflict
|
||||||
* 5 D F F Y N N Y Keep
|
* 5 D F F Y N N Y Keep
|
||||||
* 6 D F F N N N Y Keep
|
* 6 D F F N N N Y Keep
|
||||||
* 7 F D F Y Y N N Update
|
* 7 F D F Y Y N N Update
|
||||||
|
@ -615,10 +615,7 @@ void processEntry(CanonicalTreeParser h, CanonicalTreeParser m,
|
||||||
|
|
||||||
break;
|
break;
|
||||||
case 0xDFD: // 3 4
|
case 0xDFD: // 3 4
|
||||||
// CAUTION: I put it into removed instead of updated, because
|
keep(dce);
|
||||||
// that's what our tests expect
|
|
||||||
// updated.put(name, mId);
|
|
||||||
remove(name);
|
|
||||||
break;
|
break;
|
||||||
case 0xF0D: // 18
|
case 0xF0D: // 18
|
||||||
remove(name);
|
remove(name);
|
||||||
|
@ -703,12 +700,13 @@ void processEntry(CanonicalTreeParser h, CanonicalTreeParser m,
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* <pre>
|
* <pre>
|
||||||
* I (index) H M Result
|
* I (index) H M H==M Result
|
||||||
* -------------------------------------------------------
|
* -------------------------------------------
|
||||||
* 0 nothing nothing nothing (does not happen)
|
* 0 nothing nothing nothing (does not happen)
|
||||||
* 1 nothing nothing exists use M
|
* 1 nothing nothing exists use M
|
||||||
* 2 nothing exists nothing remove path from index
|
* 2 nothing exists nothing remove path from index
|
||||||
* 3 nothing exists exists use M
|
* 3 nothing exists exists yes keep index
|
||||||
|
* nothing exists exists no fail
|
||||||
* </pre>
|
* </pre>
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
@ -716,8 +714,12 @@ void processEntry(CanonicalTreeParser h, CanonicalTreeParser m,
|
||||||
update(name, mId, mMode); // 1
|
update(name, mId, mMode); // 1
|
||||||
else if (m == null)
|
else if (m == null)
|
||||||
remove(name); // 2
|
remove(name); // 2
|
||||||
else
|
else { // 3
|
||||||
update(name, mId, mMode); // 3
|
if (equalIdAndMode(hId, hMode, mId, mMode))
|
||||||
|
keep(dce);
|
||||||
|
else
|
||||||
|
conflict(name, dce, h, m);
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
if (h == null) {
|
if (h == null) {
|
||||||
/**
|
/**
|
||||||
|
|
Loading…
Reference in New Issue