From edd8ad4d04897830f08b6793a9493c70c0e9b87a Mon Sep 17 00:00:00 2001 From: Masaya Suzuki Date: Sun, 21 Aug 2016 15:27:30 -0700 Subject: [PATCH] Use FS#lastModified instead of File#lastModified This fixes the tests failed in JDK8. FS uses java.nio API to get file attributes. The timestamps obtained from that API are more precise than the ones from java.io.File#lastModified() since Java8. This difference accidentally makes JGit detect newly added files as smudged. Use the precised timestamp to avoid this false positive. Bug: 500058 Change-Id: I9e587583c85cb6efa7562ad6c5f26577869a2e7c Signed-off-by: Masaya Suzuki Signed-off-by: Andrey Loskutov --- .../internal/storage/file/GcTestCase.java | 5 ++-- .../eclipse/jgit/merge/ResolveMergerTest.java | 26 ++++++++++--------- .../jgit/treewalk/FileTreeIteratorTest.java | 2 +- .../jgit/dircache/DirCacheCheckout.java | 2 +- .../internal/storage/file/FileSnapshot.java | 18 ++++++++++--- .../jgit/internal/storage/file/GC.java | 8 +++--- .../org/eclipse/jgit/merge/ResolveMerger.java | 2 +- 7 files changed, 40 insertions(+), 23 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcTestCase.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcTestCase.java index 5abf62548..e46328591 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcTestCase.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcTestCase.java @@ -105,8 +105,9 @@ public abstract class GcTestCase extends LocalDiskRepositoryTestCase { return tip; } - protected long lastModified(AnyObjectId objectId) { - return repo.getObjectDatabase().fileFor(objectId).lastModified(); + protected long lastModified(AnyObjectId objectId) throws IOException { + return repo.getFS().lastModified( + repo.getObjectDatabase().fileFor(objectId)); } protected static void fsTick() throws InterruptedException, IOException { 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 55bb93acf..a08dbbcc8 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 @@ -62,6 +62,7 @@ import org.eclipse.jgit.junit.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.FS; import org.eclipse.jgit.util.FileUtils; import org.junit.Assert; import org.junit.experimental.theories.DataPoint; @@ -694,7 +695,7 @@ public class ResolveMergerTest extends RepositoryTestCase { // Create initial content and remember when the last file was written. f = writeTrashFiles(false, "orig", "orig", "1\n2\n3", "orig", "orig"); - lastTs4 = f.lastModified(); + lastTs4 = FS.DETECTED.lastModified(f); // 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 @@ -707,8 +708,8 @@ public class ResolveMergerTest extends RepositoryTestCase { 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(); + FS.DETECTED.lastModified(new File(db.getWorkTree(), "4"))); + lastTsIndex = FS.DETECTED.lastModified(indexFile); // Do modifications on the master branch. Then add and commit. This // should touch only "0", "2 and "3" @@ -722,7 +723,7 @@ public class ResolveMergerTest extends RepositoryTestCase { checkConsistentLastModified("0", "1", "2", "3", "4"); checkModificationTimeStampOrder("1", "4", "*" + lastTs4, "<*" + lastTsIndex, "<0", "2", "3", "<.git/index"); - lastTsIndex = indexFile.lastModified(); + lastTsIndex = FS.DETECTED.lastModified(indexFile); // Checkout a side branch. This should touch only "0", "2 and "3" fsTick(indexFile); @@ -731,7 +732,7 @@ public class ResolveMergerTest extends RepositoryTestCase { checkConsistentLastModified("0", "1", "2", "3", "4"); checkModificationTimeStampOrder("1", "4", "*" + lastTs4, "<*" + lastTsIndex, "<0", "2", "3", ".git/index"); - lastTsIndex = indexFile.lastModified(); + lastTsIndex = FS.DETECTED.lastModified(indexFile); // This checkout may have populated worktree and index so fast that we // may have smudged entries now. Check that we have the right content @@ -744,13 +745,13 @@ public class ResolveMergerTest extends RepositoryTestCase { indexState(CONTENT)); fsTick(indexFile); f = writeTrashFiles(false, "orig", "orig", "1\n2\n3", "orig", "orig"); - lastTs4 = f.lastModified(); + lastTs4 = FS.DETECTED.lastModified(f); fsTick(f); git.add().addFilepattern(".").call(); checkConsistentLastModified("0", "1", "2", "3", "4"); checkModificationTimeStampOrder("*" + lastTsIndex, "<0", "1", "2", "3", "4", "<.git/index"); - lastTsIndex = indexFile.lastModified(); + lastTsIndex = FS.DETECTED.lastModified(indexFile); // Do modifications on the side branch. Touch only "1", "2 and "3" fsTick(indexFile); @@ -761,7 +762,7 @@ public class ResolveMergerTest extends RepositoryTestCase { checkConsistentLastModified("0", "1", "2", "3", "4"); checkModificationTimeStampOrder("0", "4", "*" + lastTs4, "<*" + lastTsIndex, "<1", "2", "3", "<.git/index"); - lastTsIndex = indexFile.lastModified(); + lastTsIndex = FS.DETECTED.lastModified(indexFile); // merge master and side. Should only touch "0," "2" and "3" fsTick(indexFile); @@ -789,7 +790,7 @@ public class ResolveMergerTest extends RepositoryTestCase { "IndexEntry with path " + path + " has lastmodified with is different from the worktree file", - new File(workTree, path).lastModified(), dc.getEntry(path) + FS.DETECTED.lastModified(new File(workTree, path)), dc.getEntry(path) .getLastModified()); } @@ -799,14 +800,15 @@ public class ResolveMergerTest extends RepositoryTestCase { // then this file must be younger then file i. A path "*" // represents a file with a modification time of // E.g. ("a", "b", " lastMod); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java index efb5ddb4e..ef0f2d91f 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java @@ -99,7 +99,7 @@ public class FileTreeIteratorTest extends RepositoryTestCase { for (int i = paths.length - 1; i >= 0; i--) { final String s = paths[i]; writeTrashFile(s, s); - mtime[i] = new File(trash, s).lastModified(); + mtime[i] = FS.DETECTED.lastModified(new File(trash, s)); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java index ab61c5937..8af7e46a0 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java @@ -1379,7 +1379,7 @@ public class DirCacheCheckout { FileUtils.delete(tmpFile); } } - entry.setLastModified(f.lastModified()); + entry.setLastModified(fs.lastModified(f)); } @SuppressWarnings("deprecation") diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java index e3f1e53dc..8926d7930 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java @@ -44,6 +44,7 @@ package org.eclipse.jgit.internal.storage.file; import java.io.File; +import java.io.IOException; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Date; @@ -102,8 +103,13 @@ public class FileSnapshot { * @return the snapshot. */ public static FileSnapshot save(File path) { - final long read = System.currentTimeMillis(); - final long modified = path.lastModified(); + long read = System.currentTimeMillis(); + long modified; + try { + modified = FS.DETECTED.lastModified(path); + } catch (IOException e) { + modified = path.lastModified(); + } return new FileSnapshot(read, modified); } @@ -153,7 +159,13 @@ public class FileSnapshot { * @return true if the path needs to be read again. */ public boolean isModified(File path) { - return isModified(path.lastModified()); + long currLastModified; + try { + currLastModified = FS.DETECTED.lastModified(path); + } catch (IOException e) { + currLastModified = path.lastModified(); + } + return isModified(currLastModified); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java index c998ebe96..791e04bae 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java @@ -189,9 +189,10 @@ public class GC { * @param oldPacks * @param newPacks * @throws ParseException + * @throws IOException */ private void deleteOldPacks(Collection oldPacks, - Collection newPacks) throws ParseException { + Collection newPacks) throws ParseException, IOException { long packExpireDate = getPackExpireDate(); oldPackLoop: for (PackFile oldPack : oldPacks) { String oldName = oldPack.getPackName(); @@ -202,7 +203,8 @@ public class GC { continue oldPackLoop; if (!oldPack.shouldBeKept() - && oldPack.getPackFile().lastModified() < packExpireDate) { + && repo.getFS().lastModified( + oldPack.getPackFile()) < packExpireDate) { oldPack.close(); prunePack(oldName); } @@ -338,7 +340,7 @@ public class GC { String fName = f.getName(); if (fName.length() != Constants.OBJECT_ID_STRING_LENGTH - 2) continue; - if (f.lastModified() >= expireDate) + if (repo.getFS().lastModified(f) >= expireDate) continue; try { ObjectId id = ObjectId.fromString(d + fName); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java index e224d71d4..fd5e7ef7b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java @@ -761,7 +761,7 @@ public class ResolveMerger extends ThreeWayMerger { : FileMode.fromBits(newMode)); if (mergedFile != null) { long len = mergedFile.length(); - dce.setLastModified(mergedFile.lastModified()); + dce.setLastModified(FS.DETECTED.lastModified(mergedFile)); dce.setLength((int) len); InputStream is = new FileInputStream(mergedFile); try {