Browse Source

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 <masayasuzuki@google.com>
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
stable-4.5
Masaya Suzuki 8 years ago committed by Andrey Loskutov
parent
commit
edd8ad4d04
  1. 5
      org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/GcTestCase.java
  2. 26
      org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/ResolveMergerTest.java
  3. 2
      org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/FileTreeIteratorTest.java
  4. 2
      org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java
  5. 18
      org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java
  6. 8
      org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java
  7. 2
      org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java

5
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; return tip;
} }
protected long lastModified(AnyObjectId objectId) { protected long lastModified(AnyObjectId objectId) throws IOException {
return repo.getObjectDatabase().fileFor(objectId).lastModified(); return repo.getFS().lastModified(
repo.getObjectDatabase().fileFor(objectId));
} }
protected static void fsTick() throws InterruptedException, IOException { protected static void fsTick() throws InterruptedException, IOException {

26
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.merge.ResolveMerger.MergeFailureReason;
import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.treewalk.FileTreeIterator; import org.eclipse.jgit.treewalk.FileTreeIterator;
import org.eclipse.jgit.util.FS;
import org.eclipse.jgit.util.FileUtils; import org.eclipse.jgit.util.FileUtils;
import org.junit.Assert; import org.junit.Assert;
import org.junit.experimental.theories.DataPoint; 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. // Create initial content and remember when the last file was written.
f = writeTrashFiles(false, "orig", "orig", "1\n2\n3", "orig", "orig"); 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 // 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 // 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"); checkConsistentLastModified("0", "1", "2", "3", "4");
checkModificationTimeStampOrder("1", "2", "3", "4", "<.git/index"); checkModificationTimeStampOrder("1", "2", "3", "4", "<.git/index");
assertEquals("Commit should not touch working tree file 4", lastTs4, assertEquals("Commit should not touch working tree file 4", lastTs4,
new File(db.getWorkTree(), "4").lastModified()); FS.DETECTED.lastModified(new File(db.getWorkTree(), "4")));
lastTsIndex = indexFile.lastModified(); lastTsIndex = FS.DETECTED.lastModified(indexFile);
// Do modifications on the master branch. Then add and commit. This // Do modifications on the master branch. Then add and commit. This
// should touch only "0", "2 and "3" // should touch only "0", "2 and "3"
@ -722,7 +723,7 @@ public class ResolveMergerTest extends RepositoryTestCase {
checkConsistentLastModified("0", "1", "2", "3", "4"); checkConsistentLastModified("0", "1", "2", "3", "4");
checkModificationTimeStampOrder("1", "4", "*" + lastTs4, "<*" checkModificationTimeStampOrder("1", "4", "*" + lastTs4, "<*"
+ lastTsIndex, "<0", "2", "3", "<.git/index"); + 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" // Checkout a side branch. This should touch only "0", "2 and "3"
fsTick(indexFile); fsTick(indexFile);
@ -731,7 +732,7 @@ public class ResolveMergerTest extends RepositoryTestCase {
checkConsistentLastModified("0", "1", "2", "3", "4"); checkConsistentLastModified("0", "1", "2", "3", "4");
checkModificationTimeStampOrder("1", "4", "*" + lastTs4, "<*" checkModificationTimeStampOrder("1", "4", "*" + lastTs4, "<*"
+ lastTsIndex, "<0", "2", "3", ".git/index"); + 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 // This checkout may have populated worktree and index so fast that we
// may have smudged entries now. Check that we have the right content // may have smudged entries now. Check that we have the right content
@ -744,13 +745,13 @@ public class ResolveMergerTest extends RepositoryTestCase {
indexState(CONTENT)); indexState(CONTENT));
fsTick(indexFile); fsTick(indexFile);
f = writeTrashFiles(false, "orig", "orig", "1\n2\n3", "orig", "orig"); f = writeTrashFiles(false, "orig", "orig", "1\n2\n3", "orig", "orig");
lastTs4 = f.lastModified(); lastTs4 = FS.DETECTED.lastModified(f);
fsTick(f); fsTick(f);
git.add().addFilepattern(".").call(); git.add().addFilepattern(".").call();
checkConsistentLastModified("0", "1", "2", "3", "4"); checkConsistentLastModified("0", "1", "2", "3", "4");
checkModificationTimeStampOrder("*" + lastTsIndex, "<0", "1", "2", "3", checkModificationTimeStampOrder("*" + lastTsIndex, "<0", "1", "2", "3",
"4", "<.git/index"); "4", "<.git/index");
lastTsIndex = indexFile.lastModified(); lastTsIndex = FS.DETECTED.lastModified(indexFile);
// Do modifications on the side branch. Touch only "1", "2 and "3" // Do modifications on the side branch. Touch only "1", "2 and "3"
fsTick(indexFile); fsTick(indexFile);
@ -761,7 +762,7 @@ public class ResolveMergerTest extends RepositoryTestCase {
checkConsistentLastModified("0", "1", "2", "3", "4"); checkConsistentLastModified("0", "1", "2", "3", "4");
checkModificationTimeStampOrder("0", "4", "*" + lastTs4, "<*" checkModificationTimeStampOrder("0", "4", "*" + lastTs4, "<*"
+ lastTsIndex, "<1", "2", "3", "<.git/index"); + 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" // merge master and side. Should only touch "0," "2" and "3"
fsTick(indexFile); fsTick(indexFile);
@ -789,7 +790,7 @@ public class ResolveMergerTest extends RepositoryTestCase {
"IndexEntry with path " "IndexEntry with path "
+ path + path
+ " has lastmodified with is different from the worktree file", + " 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()); .getLastModified());
} }
@ -799,14 +800,15 @@ public class ResolveMergerTest extends RepositoryTestCase {
// then this file must be younger then file i. A path "*<modtime>" // then this file must be younger then file i. A path "*<modtime>"
// represents a file with a modification time of <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 // E.g. ("a", "b", "<c", "f/a.txt") means: a<=b<c<=f/a.txt
private void checkModificationTimeStampOrder(String... pathes) { private void checkModificationTimeStampOrder(String... pathes)
throws IOException {
long lastMod = Long.MIN_VALUE; long lastMod = Long.MIN_VALUE;
for (String p : pathes) { for (String p : pathes) {
boolean strong = p.startsWith("<"); boolean strong = p.startsWith("<");
boolean fixed = p.charAt(strong ? 1 : 0) == '*'; boolean fixed = p.charAt(strong ? 1 : 0) == '*';
p = p.substring((strong ? 1 : 0) + (fixed ? 1 : 0)); p = p.substring((strong ? 1 : 0) + (fixed ? 1 : 0));
long curMod = fixed ? Long.valueOf(p).longValue() : new File( long curMod = fixed ? Long.valueOf(p).longValue()
db.getWorkTree(), p).lastModified(); : FS.DETECTED.lastModified(new File(db.getWorkTree(), p));
if (strong) if (strong)
assertTrue("path " + p + " is not younger than predecesssor", assertTrue("path " + p + " is not younger than predecesssor",
curMod > lastMod); curMod > lastMod);

2
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--) { for (int i = paths.length - 1; i >= 0; i--) {
final String s = paths[i]; final String s = paths[i];
writeTrashFile(s, s); writeTrashFile(s, s);
mtime[i] = new File(trash, s).lastModified(); mtime[i] = FS.DETECTED.lastModified(new File(trash, s));
} }
} }

2
org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCacheCheckout.java vendored

@ -1379,7 +1379,7 @@ public class DirCacheCheckout {
FileUtils.delete(tmpFile); FileUtils.delete(tmpFile);
} }
} }
entry.setLastModified(f.lastModified()); entry.setLastModified(fs.lastModified(f));
} }
@SuppressWarnings("deprecation") @SuppressWarnings("deprecation")

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

@ -44,6 +44,7 @@
package org.eclipse.jgit.internal.storage.file; package org.eclipse.jgit.internal.storage.file;
import java.io.File; import java.io.File;
import java.io.IOException;
import java.text.DateFormat; import java.text.DateFormat;
import java.text.SimpleDateFormat; import java.text.SimpleDateFormat;
import java.util.Date; import java.util.Date;
@ -102,8 +103,13 @@ public class FileSnapshot {
* @return the snapshot. * @return the snapshot.
*/ */
public static FileSnapshot save(File path) { public static FileSnapshot save(File path) {
final long read = System.currentTimeMillis(); long read = System.currentTimeMillis();
final long modified = path.lastModified(); long modified;
try {
modified = FS.DETECTED.lastModified(path);
} catch (IOException e) {
modified = path.lastModified();
}
return new FileSnapshot(read, modified); return new FileSnapshot(read, modified);
} }
@ -153,7 +159,13 @@ public class FileSnapshot {
* @return true if the path needs to be read again. * @return true if the path needs to be read again.
*/ */
public boolean isModified(File path) { 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);
} }
/** /**

8
org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/GC.java

@ -189,9 +189,10 @@ public class GC {
* @param oldPacks * @param oldPacks
* @param newPacks * @param newPacks
* @throws ParseException * @throws ParseException
* @throws IOException
*/ */
private void deleteOldPacks(Collection<PackFile> oldPacks, private void deleteOldPacks(Collection<PackFile> oldPacks,
Collection<PackFile> newPacks) throws ParseException { Collection<PackFile> newPacks) throws ParseException, IOException {
long packExpireDate = getPackExpireDate(); long packExpireDate = getPackExpireDate();
oldPackLoop: for (PackFile oldPack : oldPacks) { oldPackLoop: for (PackFile oldPack : oldPacks) {
String oldName = oldPack.getPackName(); String oldName = oldPack.getPackName();
@ -202,7 +203,8 @@ public class GC {
continue oldPackLoop; continue oldPackLoop;
if (!oldPack.shouldBeKept() if (!oldPack.shouldBeKept()
&& oldPack.getPackFile().lastModified() < packExpireDate) { && repo.getFS().lastModified(
oldPack.getPackFile()) < packExpireDate) {
oldPack.close(); oldPack.close();
prunePack(oldName); prunePack(oldName);
} }
@ -338,7 +340,7 @@ public class GC {
String fName = f.getName(); String fName = f.getName();
if (fName.length() != Constants.OBJECT_ID_STRING_LENGTH - 2) if (fName.length() != Constants.OBJECT_ID_STRING_LENGTH - 2)
continue; continue;
if (f.lastModified() >= expireDate) if (repo.getFS().lastModified(f) >= expireDate)
continue; continue;
try { try {
ObjectId id = ObjectId.fromString(d + fName); ObjectId id = ObjectId.fromString(d + fName);

2
org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java

@ -761,7 +761,7 @@ public class ResolveMerger extends ThreeWayMerger {
: FileMode.fromBits(newMode)); : FileMode.fromBits(newMode));
if (mergedFile != null) { if (mergedFile != null) {
long len = mergedFile.length(); long len = mergedFile.length();
dce.setLastModified(mergedFile.lastModified()); dce.setLastModified(FS.DETECTED.lastModified(mergedFile));
dce.setLength((int) len); dce.setLength((int) len);
InputStream is = new FileInputStream(mergedFile); InputStream is = new FileInputStream(mergedFile);
try { try {

Loading…
Cancel
Save