Include filekey file attribute when comparing FileSnapshots

Due to finite filesystem timestamp resolution the last modified
timestamp of files cannot detect file changes which happened in the
immediate past (less than one filesystem timer tick ago).

Some filesystems expose unique file identifiers, e.g. inodes in Posix
filesystems which are named filekeys in Java's BasicFileAttributes. Use
them as another means to detect file modifications based on stat
information.

Running git gc on a repository yields a new packfile with the same id as
a packfile which existed before the gc if these packfiles contain the
same set of objects. The content of the old and the new packfile might
differ if a different PackConfig was used when writing the packfile.
Considering filekeys in FileSnapshot may help to detect such packfile
modifications.

Bug: 546891
Change-Id: I711a80328c55e1a31171d540880b8e80ec1fe095
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileSnapshotTest.java

@@ -47,6 +47,9 @@ import static org.junit.Assert.assertTrue;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.StandardCopyOption;
+import java.nio.file.attribute.FileTime;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -127,6 +130,31 @@ public class FileSnapshotTest {
 		assertTrue(save.isModified(f1));
 	}
 
+	/**
+	 * Simulate packfile replacement in same file which may occur if set of
+	 * objects in the pack is the same but pack config was different. On Posix
+	 * filesystems this should change the inode (filekey in java.nio
+	 * terminology).
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void testSimulatePackfileReplacement() throws Exception {
+		File f1 = createFile("file"); // inode y
+		File f2 = createFile("fool"); // Guarantees new inode x
+		// wait on f2 since this method resets lastModified of the file
+		// and leaves lastModified of f1 untouched
+		waitNextSec(f2);
+		waitNextSec(f2);
+		FileTime timestamp = Files.getLastModifiedTime(f1.toPath());
+		FileSnapshot save = FileSnapshot.save(f1);
+		Files.move(f2.toPath(), f1.toPath(), // Now "file" is inode x
+				StandardCopyOption.REPLACE_EXISTING,
+				StandardCopyOption.ATOMIC_MOVE);
+		Files.setLastModifiedTime(f1.toPath(), timestamp);
+		assertTrue(save.isModified(f1));
+	}
+
 	private File createFile(String string) throws IOException {
 		trash.mkdirs();
 		File f = File.createTempFile(string, "tdat", trash);

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java

@@ -80,6 +80,8 @@ public class FileSnapshot {
 	 */
 	public static final long UNKNOWN_SIZE = -1;
 
+	private static final Object MISSING_FILEKEY = new Object();
+
 	/**
 	 * A FileSnapshot that is considered to always be modified.
 	 * <p>
@@ -88,7 +90,7 @@ public class FileSnapshot {
 	 * snapshot contains only invalid status information.
 	 */
 	public static final FileSnapshot DIRTY = new FileSnapshot(-1, -1,
-			UNKNOWN_SIZE, Duration.ZERO);
+			UNKNOWN_SIZE, Duration.ZERO, MISSING_FILEKEY);
 
 	/**
 	 * A FileSnapshot that is clean if the file does not exist.
@@ -98,7 +100,7 @@ public class FileSnapshot {
 	 * path does not exist.
 	 */
 	public static final FileSnapshot MISSING_FILE = new FileSnapshot(0, 0, 0,
-			Duration.ZERO) {
+			Duration.ZERO, MISSING_FILEKEY) {
 		@Override
 		public boolean isModified(File path) {
 			return FS.DETECTED.exists(path);
@@ -119,17 +121,26 @@ public class FileSnapshot {
 		long read = System.currentTimeMillis();
 		long modified;
 		long size;
+		Object fileKey = null;
 		Duration fsTimerResolution = FS
 				.getFsTimerResolution(path.toPath().getParent());
 		try {
 			BasicFileAttributes fileAttributes = FS.DETECTED.fileAttributes(path);
 			modified = fileAttributes.lastModifiedTime().toMillis();
 			size = fileAttributes.size();
+			fileKey = getFileKey(fileAttributes);
 		} catch (IOException e) {
 			modified = path.lastModified();
 			size = path.length();
+			fileKey = MISSING_FILEKEY;
 		}
-		return new FileSnapshot(read, modified, size, fsTimerResolution);
+		return new FileSnapshot(read, modified, size, fsTimerResolution,
+				fileKey);
+	}
+
+	private static Object getFileKey(BasicFileAttributes fileAttributes) {
+		Object fileKey = fileAttributes.fileKey();
+		return fileKey == null ? MISSING_FILEKEY : fileKey;
 	}
 
 	/**
@@ -149,7 +160,8 @@ public class FileSnapshot {
 	 */
 	public static FileSnapshot save(long modified) {
 		final long read = System.currentTimeMillis();
-		return new FileSnapshot(read, modified, -1, Duration.ZERO);
+		return new FileSnapshot(read, modified, -1, Duration.ZERO,
+				MISSING_FILEKEY);
 	}
 
 	/** Last observed modification time of the path. */
@@ -169,13 +181,20 @@ public class FileSnapshot {
 	/** measured filesystem timestamp resolution */
 	private Duration fsTimestampResolution;
 
+	/**
+	 * Object that uniquely identifies the given file, or {@code
+	 * null} if a file key is not available
+	 */
+	private Object fileKey;
+
 	private FileSnapshot(long read, long modified, long size,
-			@NonNull Duration fsTimestampResolution) {
+			@NonNull Duration fsTimestampResolution, @NonNull Object fileKey) {
 		this.lastRead = read;
 		this.lastModified = modified;
 		this.fsTimestampResolution = fsTimestampResolution;
 		this.size = size;
 		this.cannotBeRacilyClean = notRacyClean(read);
+		this.fileKey = fileKey;
 	}
 
 	/**
@@ -204,15 +223,20 @@ public class FileSnapshot {
 	public boolean isModified(File path) {
 		long currLastModified;
 		long currSize;
+		Object currFileKey;
 		try {
 			BasicFileAttributes fileAttributes = FS.DETECTED.fileAttributes(path);
 			currLastModified = fileAttributes.lastModifiedTime().toMillis();
 			currSize = fileAttributes.size();
+			currFileKey = getFileKey(fileAttributes);
 		} catch (IOException e) {
 			currLastModified = path.lastModified();
 			currSize = path.length();
+			currFileKey = MISSING_FILEKEY;
 		}
-		return (currSize != UNKNOWN_SIZE && currSize != size) || isModified(currLastModified);
+		return isSizeChanged(currSize)
+				|| isFileKeyChanged(currFileKey)
+				|| isModified(currLastModified);
 	}
 
 	/**
@@ -252,7 +276,8 @@ public class FileSnapshot {
 	 * @return true if the two snapshots share the same information.
 	 */
 	public boolean equals(FileSnapshot other) {
-		return lastModified == other.lastModified && size == other.size;
+		return lastModified == other.lastModified && size == other.size
+				&& Objects.equals(fileKey, other.fileKey);
 	}
 
 	/** {@inheritDoc} */
@@ -274,7 +299,8 @@ public class FileSnapshot {
 	/** {@inheritDoc} */
 	@Override
 	public int hashCode() {
-		return Objects.hash(Long.valueOf(lastModified), Long.valueOf(size));
+		return Objects.hash(Long.valueOf(lastModified), Long.valueOf(size),
+				fileKey);
 	}
 
 	/** {@inheritDoc} */
@@ -291,7 +317,7 @@ public class FileSnapshot {
 				Locale.US);
 		return "FileSnapshot[modified: " + f.format(new Date(lastModified))
 				+ ", read: " + f.format(new Date(lastRead)) + ", size:" + size
-				+ "]";
+				+ ", fileKey: " + fileKey + "]";
 	}
 
 	private boolean notRacyClean(long read) {
@@ -327,4 +353,12 @@ public class FileSnapshot {
 		//
 		return true;
 	}
+
+	private boolean isFileKeyChanged(Object currFileKey) {
+		return currFileKey != MISSING_FILEKEY && !currFileKey.equals(fileKey);
+	}
+
+	private boolean isSizeChanged(long currSize) {
+		return currSize != UNKNOWN_SIZE && currSize != size;
+	}
 }