Again teach ResolveMerger to create more correct DirCacheEntry's

Currently, after a merge/cherry-pick/rebase, all index entries are
smudged as the ResolveMerger never sets entry lengths and/or
modification times. This change teaches it to re-set them at least for
things it did not touch. The other entries are then repaired when the
index is persisted, or entries are checked out.

The first attempt to get this in was commit
3ea694c2523d909190b5350e13254a62e94ec5d5 which has been reverted.
Since then some fixes to ResolveMerger and a few more tests have
been added which check situations where the index is not matching
HEAD before we merge.

Change-Id: I648fda30846615b3bf688c34274c6cf4bc857832
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Also-by: Markus Duft <markus.duft@salomon.at>

2 files changed, 468 insertions, 0 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java
index 0c573ebe71..c06322e8e4 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryTestCase.java
@@ -397,4 +397,33 @@ public abstract class RepositoryTestCase extends LocalDiskRepositoryTestCase {
 		RefUpdate refUpdate = db.updateRef(Constants.HEAD);
 		refUpdate.link(branchName);
 	}
+
+	/**
+	 * Writes a number of files in the working tree. The first content specified
+	 * will be written into a file named '0', the second into a file named "1"
+	 * and so on. If <code>null</code> is specified as content then this file is
+	 * skipped.
+	 *
+	 * @param ensureDistinctTimestamps
+	 *            if set to <code>true</code> then between two write operations
+	 *            this method will wait to ensure that the second file will get
+	 *            a different lastmodification timestamp than the first file.
+	 * @param contents
+	 *            the contents which should be written into the files
+	 * @return the File object associated to the last written file.
+	 * @throws IOException
+	 * @throws InterruptedException
+	 */
+	protected File writeTrashFiles(boolean ensureDistinctTimestamps,
+			String... contents)
+			throws IOException, InterruptedException {
+				File f = null;
+				for (int i = 0; i < contents.length; i++)
+					if (contents[i] != null) {
+						if (ensureDistinctTimestamps && (f != null))
+							fsTick(f);
+						f = writeTrashFile(Integer.toString(i), contents[i]);
+					}
+				return f;
+			}
 }
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/ResolveMergerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/ResolveMergerTest.java
index 4cb0896023..9876100ec0 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/ResolveMergerTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/ResolveMergerTest.java
@@ -42,15 +42,25 @@
  */
 package org.eclipse.jgit.merge;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
 
 import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.MergeResult;
+import org.eclipse.jgit.api.MergeResult.MergeStatus;
+import org.eclipse.jgit.api.errors.CheckoutConflictException;
+import org.eclipse.jgit.dircache.DirCache;
 import org.eclipse.jgit.lib.RepositoryTestCase;
+import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.treewalk.FileTreeIterator;
 import org.eclipse.jgit.util.FileUtils;
+import org.junit.Assert;
 import org.junit.Test;
 
 public class ResolveMergerTest extends RepositoryTestCase {
@@ -95,4 +105,433 @@ public class ResolveMergerTest extends RepositoryTestCase {
 		assertFalse(ok);
 	}
 
+	/**
+	 * Merging two conflicting subtrees when the index does not contain any file
+	 * in that subtree should lead to a conflicting state.
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void checkMergeConflictingTreesWithoutIndex() throws Exception {
+		Git git = Git.wrap(db);
+
+		writeTrashFile("d/1", "orig");
+		git.add().addFilepattern("d/1").call();
+		RevCommit first = git.commit().setMessage("added d/1").call();
+
+		writeTrashFile("d/1", "master");
+		RevCommit masterCommit = git.commit().setAll(true)
+				.setMessage("modified d/1 on master").call();
+
+		git.checkout().setCreateBranch(true).setStartPoint(first)
+				.setName("side").call();
+		writeTrashFile("d/1", "side");
+		git.commit().setAll(true).setMessage("modified d/1 on side").call();
+
+		git.rm().addFilepattern("d/1").call();
+		git.rm().addFilepattern("d").call();
+		MergeResult mergeRes = git.merge().include(masterCommit).call();
+		assertTrue(MergeStatus.CONFLICTING.equals(mergeRes.getMergeStatus()));
+		assertEquals(
+				"[d/1, mode:100644, stage:1, content:orig][d/1, mode:100644, stage:2, content:side][d/1, mode:100644, stage:3, content:master]",
+				indexState(CONTENT));
+	}
+
+	/**
+	 * Merging two different but mergeable subtrees when the index does not
+	 * contain any file in that subtree should lead to a merged state.
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void checkMergeMergeableTreesWithoutIndex() throws Exception {
+		Git git = Git.wrap(db);
+
+		writeTrashFile("d/1", "1\n2\n3");
+		git.add().addFilepattern("d/1").call();
+		RevCommit first = git.commit().setMessage("added d/1").call();
+
+		writeTrashFile("d/1", "1master\n2\n3");
+		RevCommit masterCommit = git.commit().setAll(true)
+				.setMessage("modified d/1 on master").call();
+
+		git.checkout().setCreateBranch(true).setStartPoint(first)
+				.setName("side").call();
+		writeTrashFile("d/1", "1\n2\n3side");
+		git.commit().setAll(true).setMessage("modified d/1 on side").call();
+
+		git.rm().addFilepattern("d/1").call();
+		git.rm().addFilepattern("d").call();
+		MergeResult mergeRes = git.merge().include(masterCommit).call();
+		assertTrue(MergeStatus.MERGED.equals(mergeRes.getMergeStatus()));
+		assertEquals("[d/1, mode:100644, content:1master\n2\n3side\n]",
+				indexState(CONTENT));
+	}
+
+	/**
+	 * Merging two equal subtrees when the index does not contain any file in
+	 * that subtree should lead to a merged state.
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void checkMergeEqualTreesWithoutIndex() throws Exception {
+		Git git = Git.wrap(db);
+
+		writeTrashFile("d/1", "orig");
+		git.add().addFilepattern("d/1").call();
+		RevCommit first = git.commit().setMessage("added d/1").call();
+
+		writeTrashFile("d/1", "modified");
+		RevCommit masterCommit = git.commit().setAll(true)
+				.setMessage("modified d/1 on master").call();
+
+		git.checkout().setCreateBranch(true).setStartPoint(first)
+				.setName("side").call();
+		writeTrashFile("d/1", "modified");
+		git.commit().setAll(true).setMessage("modified d/1 on side").call();
+
+		git.rm().addFilepattern("d/1").call();
+		git.rm().addFilepattern("d").call();
+		MergeResult mergeRes = git.merge().include(masterCommit).call();
+		assertTrue(MergeStatus.MERGED.equals(mergeRes.getMergeStatus()));
+		assertEquals("[d/1, mode:100644, content:modified]",
+				indexState(CONTENT));
+	}
+
+	/**
+	 * Merging two equal subtrees with an incore merger should lead to a merged
+	 * state (The 'Gerrit' use case).
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void checkMergeEqualTreesInCore() throws Exception {
+		Git git = Git.wrap(db);
+
+		writeTrashFile("d/1", "orig");
+		git.add().addFilepattern("d/1").call();
+		RevCommit first = git.commit().setMessage("added d/1").call();
+
+		writeTrashFile("d/1", "modified");
+		RevCommit masterCommit = git.commit().setAll(true)
+				.setMessage("modified d/1 on master").call();
+
+		git.checkout().setCreateBranch(true).setStartPoint(first)
+				.setName("side").call();
+		writeTrashFile("d/1", "modified");
+		RevCommit sideCommit = git.commit().setAll(true)
+				.setMessage("modified d/1 on side").call();
+
+		git.rm().addFilepattern("d/1").call();
+		git.rm().addFilepattern("d").call();
+
+		ThreeWayMerger resolveMerger = MergeStrategy.RESOLVE
+				.newMerger(db, true);
+		boolean noProblems = resolveMerger.merge(masterCommit, sideCommit);
+		assertTrue(noProblems);
+	}
+
+	/**
+	 * Merging two equal subtrees when the index and HEAD does not contain any
+	 * file in that subtree should lead to a merged state.
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void checkMergeEqualNewTrees() throws Exception {
+		Git git = Git.wrap(db);
+
+		writeTrashFile("2", "orig");
+		git.add().addFilepattern("2").call();
+		RevCommit first = git.commit().setMessage("added 2").call();
+
+		writeTrashFile("d/1", "orig");
+		git.add().addFilepattern("d/1").call();
+		RevCommit masterCommit = git.commit().setAll(true)
+				.setMessage("added d/1 on master").call();
+
+		git.checkout().setCreateBranch(true).setStartPoint(first)
+				.setName("side").call();
+		writeTrashFile("d/1", "orig");
+		git.add().addFilepattern("d/1").call();
+		git.commit().setAll(true).setMessage("added d/1 on side").call();
+
+		git.rm().addFilepattern("d/1").call();
+		git.rm().addFilepattern("d").call();
+		MergeResult mergeRes = git.merge().include(masterCommit).call();
+		assertTrue(MergeStatus.MERGED.equals(mergeRes.getMergeStatus()));
+		assertEquals(
+				"[2, mode:100644, content:orig][d/1, mode:100644, content:orig]",
+				indexState(CONTENT));
+	}
+
+	/**
+	 * Merging two conflicting subtrees when the index and HEAD does not contain
+	 * any file in that subtree should lead to a conflicting state.
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void checkMergeConflictingNewTrees() throws Exception {
+		Git git = Git.wrap(db);
+
+		writeTrashFile("2", "orig");
+		git.add().addFilepattern("2").call();
+		RevCommit first = git.commit().setMessage("added 2").call();
+
+		writeTrashFile("d/1", "master");
+		git.add().addFilepattern("d/1").call();
+		RevCommit masterCommit = git.commit().setAll(true)
+				.setMessage("added d/1 on master").call();
+
+		git.checkout().setCreateBranch(true).setStartPoint(first)
+				.setName("side").call();
+		writeTrashFile("d/1", "side");
+		git.add().addFilepattern("d/1").call();
+		git.commit().setAll(true).setMessage("added d/1 on side").call();
+
+		git.rm().addFilepattern("d/1").call();
+		git.rm().addFilepattern("d").call();
+		MergeResult mergeRes = git.merge().include(masterCommit).call();
+		assertTrue(MergeStatus.CONFLICTING.equals(mergeRes.getMergeStatus()));
+		assertEquals(
+				"[2, mode:100644, content:orig][d/1, mode:100644, stage:2, content:side][d/1, mode:100644, stage:3, content:master]",
+				indexState(CONTENT));
+	}
+
+	/**
+	 * Merging two conflicting files when the index contains a tree for that
+	 * path should lead to a failed state.
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void checkMergeConflictingFilesWithTreeInIndex() throws Exception {
+		Git git = Git.wrap(db);
+
+		writeTrashFile("0", "orig");
+		git.add().addFilepattern("0").call();
+		RevCommit first = git.commit().setMessage("added 0").call();
+
+		writeTrashFile("0", "master");
+		RevCommit masterCommit = git.commit().setAll(true)
+				.setMessage("modified 0 on master").call();
+
+		git.checkout().setCreateBranch(true).setStartPoint(first)
+				.setName("side").call();
+		writeTrashFile("0", "side");
+		git.commit().setAll(true).setMessage("modified 0 on side").call();
+
+		git.rm().addFilepattern("0").call();
+		writeTrashFile("0/0", "side");
+		git.add().addFilepattern("0/0").call();
+		MergeResult mergeRes = git.merge().include(masterCommit).call();
+		assertEquals(MergeStatus.FAILED, mergeRes.getMergeStatus());
+	}
+
+	/**
+	 * Merging two equal files when the index contains a tree for that path
+	 * should lead to a failed state.
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void checkMergeMergeableFilesWithTreeInIndex() throws Exception {
+		Git git = Git.wrap(db);
+
+		writeTrashFile("0", "orig");
+		writeTrashFile("1", "1\n2\n3");
+		git.add().addFilepattern("0").addFilepattern("1").call();
+		RevCommit first = git.commit().setMessage("added 0, 1").call();
+
+		writeTrashFile("1", "1master\n2\n3");
+		RevCommit masterCommit = git.commit().setAll(true)
+				.setMessage("modified 1 on master").call();
+
+		git.checkout().setCreateBranch(true).setStartPoint(first)
+				.setName("side").call();
+		writeTrashFile("1", "1\n2\n3side");
+		git.commit().setAll(true).setMessage("modified 1 on side").call();
+
+		git.rm().addFilepattern("0").call();
+		writeTrashFile("0/0", "modified");
+		git.add().addFilepattern("0/0").call();
+		try {
+			git.merge().include(masterCommit).call();
+			Assert.fail("Didn't get the expected exception");
+		} catch (CheckoutConflictException e) {
+			assertEquals(1, e.getConflictingPaths().size());
+			assertEquals("0/0", e.getConflictingPaths().get(0));
+		}
+	}
+
+	@Test
+	public void checkLockedFilesToBeDeleted() throws Exception {
+		Git git = Git.wrap(db);
+
+		writeTrashFile("a.txt", "orig");
+		writeTrashFile("b.txt", "orig");
+		git.add().addFilepattern("a.txt").addFilepattern("b.txt").call();
+		RevCommit first = git.commit().setMessage("added a.txt, b.txt").call();
+
+		// modify and delete files on the master branch
+		writeTrashFile("a.txt", "master");
+		git.rm().addFilepattern("b.txt").call();
+		RevCommit masterCommit = git.commit()
+				.setMessage("modified a.txt, deleted b.txt").setAll(true)
+				.call();
+
+		// switch back to a side branch
+		git.checkout().setCreateBranch(true).setStartPoint(first)
+				.setName("side").call();
+		writeTrashFile("c.txt", "side");
+		git.add().addFilepattern("c.txt").call();
+		git.commit().setMessage("added c.txt").call();
+
+		// Get a handle to the the file so on windows it can't be deleted.
+		FileInputStream fis = new FileInputStream(new File(db.getWorkTree(),
+				"b.txt"));
+		MergeResult mergeRes = git.merge().include(masterCommit).call();
+		if (mergeRes.getMergeStatus().equals(MergeStatus.FAILED)) {
+			// probably windows
+			assertEquals(1, mergeRes.getFailingPaths().size());
+			assertEquals(MergeFailureReason.COULD_NOT_DELETE, mergeRes
+					.getFailingPaths().get("b.txt"));
+		}
+		assertEquals("[a.txt, mode:100644, content:master]"
+				+ "[c.txt, mode:100644, content:side]", indexState(CONTENT));
+		fis.close();
+	}
+
+	@Test
+	public void checkForCorrectIndex() throws Exception {
+		File f;
+		long lastTs4, lastTsIndex;
+		Git git = Git.wrap(db);
+		File indexFile = db.getIndexFile();
+
+		// Create initial content and remember when the last file was written.
+		f = writeTrashFiles(false, "orig", "orig", "1\n2\n3", "orig", "orig");
+		lastTs4 = f.lastModified();
+
+		// add all files, commit and check this doesn't update any working tree
+		// files and that the index is in a new file system timer tick. Make
+		// sure to wait long enough before adding so the index doesn't contain
+		// racily clean entries
+		fsTick(f);
+		git.add().addFilepattern(".").call();
+		RevCommit firstCommit = git.commit().setMessage("initial commit")
+				.call();
+		checkConsistentLastModified("0", "1", "2", "3", "4");
+		checkModificationTimeStampOrder("1", "2", "3", "4", "<.git/index");
+		assertEquals("Commit should not touch working tree file 4", lastTs4,
+				new File(db.getWorkTree(), "4").lastModified());
+		lastTsIndex = indexFile.lastModified();
+
+		// Do modifications on the master branch. Then add and commit. This
+		// should touch only "0", "2 and "3"
+		fsTick(indexFile);
+		f = writeTrashFiles(false, "master", null, "1master\n2\n3", "master",
+				null);
+		fsTick(f);
+		git.add().addFilepattern(".").call();
+		RevCommit masterCommit = git.commit().setMessage("master commit")
+				.call();
+		checkConsistentLastModified("0", "1", "2", "3", "4");
+		checkModificationTimeStampOrder("1", "4", "*" + lastTs4, "<*"
+				+ lastTsIndex, "<0", "2", "3", "<.git/index");
+		lastTsIndex = indexFile.lastModified();
+
+		// Checkout a side branch. This should touch only "0", "2 and "3"
+		fsTick(indexFile);
+		git.checkout().setCreateBranch(true).setStartPoint(firstCommit)
+				.setName("side").call();
+		checkConsistentLastModified("0", "1", "2", "3", "4");
+		checkModificationTimeStampOrder("1", "4", "*" + lastTs4, "<*"
+				+ lastTsIndex, "<0", "2", "3", ".git/index");
+		lastTsIndex = indexFile.lastModified();
+
+		// This checkout may have populated worktree and index so fast that we
+		// may have smudged entries now. Check that we have the right content
+		// and then rewrite the index to get rid of smudged state
+		assertEquals("[0, mode:100644, content:orig]" //
+				+ "[1, mode:100644, content:orig]" //
+				+ "[2, mode:100644, content:1\n2\n3]" //
+				+ "[3, mode:100644, content:orig]" //
+				+ "[4, mode:100644, content:orig]", //
+				indexState(CONTENT));
+		fsTick(indexFile);
+		f = writeTrashFiles(false, "orig", "orig", "1\n2\n3", "orig", "orig");
+		lastTs4 = f.lastModified();
+		fsTick(f);
+		git.add().addFilepattern(".").call();
+		checkConsistentLastModified("0", "1", "2", "3", "4");
+		checkModificationTimeStampOrder("*" + lastTsIndex, "<0", "1", "2", "3",
+				"4", "<.git/index");
+		lastTsIndex = indexFile.lastModified();
+
+		// Do modifications on the side branch. Touch only "1", "2 and "3"
+		fsTick(indexFile);
+		f = writeTrashFiles(false, null, "side", "1\n2\n3side", "side", null);
+		fsTick(f);
+		git.add().addFilepattern(".").call();
+		git.commit().setMessage("side commit").call();
+		checkConsistentLastModified("0", "1", "2", "3", "4");
+		checkModificationTimeStampOrder("0", "4", "*" + lastTs4, "<*"
+				+ lastTsIndex, "<1", "2", "3", "<.git/index");
+		lastTsIndex = indexFile.lastModified();
+
+		// merge master and side. Should only touch "0," "2" and "3"
+		fsTick(indexFile);
+		git.merge().include(masterCommit).call();
+		checkConsistentLastModified("0", "1", "2", "4");
+		checkModificationTimeStampOrder("4", "*" + lastTs4, "<1", "<*"
+				+ lastTsIndex, "<0", "2", "3", ".git/index");
+		assertEquals(
+				"[0, mode:100644, content:master]" //
+						+ "[1, mode:100644, content:side]" //
+						+ "[2, mode:100644, content:1master\n2\n3side\n]" //
+						+ "[3, mode:100644, stage:1, content:orig][3, mode:100644, stage:2, content:side][3, mode:100644, stage:3, content:master]" //
+						+ "[4, mode:100644, content:orig]", //
+				indexState(CONTENT));
+	}
+
+	// Assert that every specified index entry has the same last modification
+	// timestamp as the associated file
+	private void checkConsistentLastModified(String... pathes)
+			throws IOException {
+		DirCache dc = db.readDirCache();
+		File workTree = db.getWorkTree();
+		for (String path : pathes)
+			assertEquals(
+					"IndexEntry with path "
+							+ path
+							+ " has lastmodified with is different from the worktree file",
+					new File(workTree, path).lastModified(), dc.getEntry(path)
+							.getLastModified());
+	}
+
+	// Assert that modification timestamps of working tree files are as
+	// expected. You may specify n files. It is asserted that every file
+	// i+1 is not older than file i. If a path of file i+1 is prefixed with "<"
+	// then this file must be younger then file i. A path "*<modtime>"
+	// represents a file with a modification time of <modtime>
+	// E.g. ("a", "b", "<c", "f/a.txt") means: a<=b<c<=f/a.txt
+	private void checkModificationTimeStampOrder(String... pathes) {
+		long lastMod = Long.MIN_VALUE;
+		for (String p : pathes) {
+			boolean strong = p.startsWith("<");
+			boolean fixed = p.charAt(strong ? 1 : 0) == '*';
+			p = p.substring((strong ? 1 : 0) + (fixed ? 1 : 0));
+			long curMod = fixed ? Long.valueOf(p).longValue() : new File(
+					db.getWorkTree(), p).lastModified();
+			if (strong)
+				assertTrue("path " + p + " is not younger than predecesssor",
+						curMod > lastMod);
+			else
+				assertTrue("path " + p + " is older than predecesssor",
+						curMod >= lastMod);
+		}
+	}
 }