From: Christian Halstrick <christian.halstrick@sap.com>
Date: Fri, 16 Jul 2010 07:59:13 +0000 (+0200)
Subject: Check for racy git in WorkingTreeIterator
X-Git-Tag: v0.9.1~163
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5c94321b47bf98645a10b685f9af31274292aecd;p=jgit.git

Check for racy git in WorkingTreeIterator

The WorkingTreeIterator has a method to check whether
the current file differs from the corresponding index
entry. This commit improves this check to also handle
racy git situations.

See http://git.kernel.org/?p=git/git.git;a=blob;f=Documentation/technical/racy-git.txt;hb=HEAD

Change-Id: I3ad0897211dcbb2eac9eebcb19d095a5052fb06b
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
---

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
index cd06c2498f..65e6607505 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
@@ -398,20 +398,20 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
 		if (entry.isUpdateNeeded())
 			return true;
 
-		if (getEntryLength() != entry.getLength())
+		if (!entry.isSmudged() && (getEntryLength() != entry.getLength()))
 			return true;
 
-		// determine difference in mode-bits of file and index-entry. In the
+		// Determine difference in mode-bits of file and index-entry. In the
 		// bitwise presentation of modeDiff we'll have a '1' when the two modes
 		// differ at this position.
 		int modeDiff = getEntryRawMode() ^ entry.getRawMode();
-		// ignore the executable file bits if checkFilemode tells me to do so.
+		// Ignore the executable file bits if checkFilemode tells me to do so.
 		// Ignoring is done by setting the bits representing a EXECUTABLE_FILE
 		// to '0' in modeDiff
 		if (!checkFilemode)
 			modeDiff &= ~FileMode.EXECUTABLE_FILE.getBits();
 		if (modeDiff != 0)
-			// report a modification if the modes still (after potentially
+			// Report a modification if the modes still (after potentially
 			// ignoring EXECUTABLE_FILE bits) differ
 			return true;
 
@@ -422,14 +422,58 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
 		long fileLastModified = getEntryLastModified();
 		if (cacheLastModified % 1000 == 0)
 			fileLastModified = fileLastModified - fileLastModified % 1000;
-		if (forceContentCheck) {
-			if (fileLastModified == cacheLastModified)
-				return false; // Same time, don't check content.
-			else
-				return !getEntryObjectId().equals(entry.getObjectId());
+
+		if (fileLastModified != cacheLastModified) {
+			// The file is dirty by timestamps
+			if (forceContentCheck) {
+				// But we are told to look at content even though timestamps
+				// tell us about modification
+				return contentCheck(entry);
+			} else {
+				// We are told to assume a modification if timestamps differs
+				return true;
+			}
 		} else {
-			// No content check forced, assume dirty if stat differs.
-			return fileLastModified != cacheLastModified;
+			// The file is clean when you look at timestamps.
+			if (entry.isSmudged()) {
+				// The file is clean by timestamps but the entry was smudged.
+				// Lets do a content check
+				return contentCheck(entry);
+			} else {
+				// The file is clean by timestamps and the entry is not
+				// smudged: Can't get any cleaner!
+				return false;
+			}
+		}
+	}
+
+	/**
+	 * Compares the entries content with the content in the filesystem.
+	 * Unsmudges the entry when it is detected that it is clean.
+	 *
+	 * @param entry
+	 *            the entry to be checked
+	 * @return <code>true</code> if the content matches, <code>false</code>
+	 *         otherwise
+	 */
+	private boolean contentCheck(DirCacheEntry entry) {
+		if (getEntryObjectId().equals(entry.getObjectId())) {
+			// Content has not changed
+
+			// We know the entry can't be racily clean because it's still clean.
+			// Therefore we unsmudge the entry!
+			// If by any chance we now unsmudge although we are still in the
+			// same time-slot as the last modification to the index file the
+			// next index write operation will smudge again.
+			// Caution: we are unsmudging just by setting the length of the
+			// in-memory entry object. It's the callers task to detect that we
+			// have modified the entry and to persist the modified index.
+			entry.setLength((int) getEntryLength());
+
+			return false;
+		} else {
+			// Content differs: that's a real change!
+			return true;
 		}
 	}