Fix status in index entries after checkout of paths

The checkout command was producing an inconsistent state of the index
which even confuses native git. The content sha1 of the touched index
entries was updated, but the length and the filemode was not updated.
Later in coding the index entries got automatically corrected (through
Dircache.checkoutEntry()) but the correction was after persisting the
index to disk. So, the correction was lost and we ended up with an index
where length and sha1 don't fit together.
A similar problem is fixed with "lastModified" of DircacheEntry. When
checking out a path without specifying an explicit commit (you want to
checkout what's in the index) the index was not updated regarding
lastModified. Readers of the index will think the checked-out
file is dirty because the file has a younger lastmodified then what's
in the index.

Change-Id: Ifc6d806fbf96f53c94d9ded0befcc932d943aa04
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Jens Baumgart <jens.baumgart@sap.com>
Bug: 355205

org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PathCheckoutCommandTest.java

@@ -45,7 +45,12 @@ package org.eclipse.jgit.api;
 import static org.junit.Assert.assertEquals;
 
 import java.io.File;
+import java.io.IOException;
 
+import org.eclipse.jgit.dircache.DirCache;
+import org.eclipse.jgit.dircache.DirCacheEntry;
+import org.eclipse.jgit.errors.NoWorkTreeException;
+import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.RepositoryTestCase;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Before;
@@ -60,6 +65,8 @@ public class PathCheckoutCommandTest extends RepositoryTestCase {
 
 	private static final String FILE2 = "Test2.txt";
 
+	private static final String FILE3 = "Test3.txt";
+
 	Git git;
 
 	RevCommit initialCommit;
@@ -148,4 +155,49 @@ public class PathCheckoutCommandTest extends RepositoryTestCase {
 		assertEquals("c", read(new File(db.getWorkTree(), FILE2)));
 	}
 
+	@Test
+	public void testUpdateWorkingDirectoryFromIndex2() throws Exception {
+		CheckoutCommand co = git.checkout();
+		fsTick(git.getRepository().getIndexFile());
+
+		File written1 = writeTrashFile(FILE1, "3(modified)");
+		File written2 = writeTrashFile(FILE2, "a(modified)");
+		fsTick(written2);
+
+		// make sure that we get unsmudged entries for FILE1 and FILE2
+		writeTrashFile(FILE3, "foo");
+		git.add().addFilepattern(FILE3).call();
+		fsTick(git.getRepository().getIndexFile());
+
+		git.add().addFilepattern(FILE1).addFilepattern(FILE2).call();
+		fsTick(git.getRepository().getIndexFile());
+
+		writeTrashFile(FILE1, "3(modified again)");
+		writeTrashFile(FILE2, "a(modified again)");
+		fsTick(written2);
+
+		co.addPath(FILE1).setStartPoint(secondCommit).call();
+
+		assertEquals("2", read(written1));
+		assertEquals("a(modified again)", read(written2));
+
+		validateIndex(git);
+	}
+
+	public static void validateIndex(Git git) throws NoWorkTreeException,
+			IOException {
+		DirCache dc = git.getRepository().lockDirCache();
+		ObjectReader r = git.getRepository().getObjectDatabase().newReader();
+		try {
+			for (int i = 0; i < dc.getEntryCount(); ++i) {
+				DirCacheEntry entry = dc.getEntry(i);
+				if (entry.getLength() > 0)
+					assertEquals(entry.getLength(), r.getObjectSize(
+							entry.getObjectId(), ObjectReader.OBJ_ANY));
+			}
+		} finally {
+			dc.unlock();
+			r.release();
+		}
+	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/api/CheckoutCommand.java

@@ -65,6 +65,7 @@ import org.eclipse.jgit.errors.AmbiguousObjectException;
 import org.eclipse.jgit.errors.CheckoutConflictException;
 import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.Ref;
@@ -245,40 +246,42 @@ public class CheckoutCommand extends GitCommand<Ref> {
 		RevWalk revWalk = new RevWalk(repo);
 		DirCache dc = repo.lockDirCache();
 		try {
-			TreeWalk treeWalk = new TreeWalk(revWalk.getObjectReader());
-			treeWalk.setRecursive(true);
-			treeWalk.addTree(new DirCacheIterator(dc));
-			treeWalk.setFilter(PathFilterGroup.createFromStrings(paths));
-			List<String> files = new LinkedList<String>();
-			while (treeWalk.next())
-				files.add(treeWalk.getPathString());
-
-			if (startCommit != null || startPoint != null) {
 			DirCacheEditor editor = dc.editor();
 			TreeWalk startWalk = new TreeWalk(revWalk.getObjectReader());
 			startWalk.setRecursive(true);
-				startWalk.setFilter(treeWalk.getFilter());
+			startWalk.setFilter(PathFilterGroup.createFromStrings(paths));
+			boolean checkoutIndex = startCommit == null && startPoint == null;
+			if (!checkoutIndex)
 				startWalk.addTree(revWalk.parseCommit(getStartPoint())
 						.getTree());
+			else
+				startWalk.addTree(new DirCacheIterator(dc));
+
+			final File workTree = repo.getWorkTree();
+			final ObjectReader r = repo.getObjectDatabase().newReader();
+			try {
 				while (startWalk.next()) {
 					final ObjectId blobId = startWalk.getObjectId(0);
+					final FileMode mode = startWalk.getFileMode(0);
 					editor.add(new PathEdit(startWalk.getPathString()) {
-
 						public void apply(DirCacheEntry ent) {
 							ent.setObjectId(blobId);
+							ent.setFileMode(mode);
+							try {
+								DirCacheCheckout.checkoutEntry(repo, new File(
+										workTree, ent.getPathString()), ent, r);
+							} catch (IOException e) {
+								throw new JGitInternalException(
+										MessageFormat.format(
+												JGitText.get().checkoutConflictWithFile,
+												ent.getPathString()), e);
+							}
 						}
 					});
 				}
 				editor.commit();
-			}
-
-			File workTree = repo.getWorkTree();
-			ObjectReader r = repo.getObjectDatabase().newReader();
-			try {
-				for (String file : files)
-					DirCacheCheckout.checkoutEntry(repo, new File(workTree,
-							file), dc.getEntry(file), r);
 			} finally {
+				startWalk.release();
 				r.release();
 			}
 		} finally {