From 82b1af31e295273eeedd82fc5db3266c909c9e80 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Mon, 8 Apr 2019 13:26:12 +0100 Subject: [PATCH] Fix pack files scan when filesnapshot isn't modified Do not reload packfiles when their associated filesnapshot is not modified on disk compared to the one currently stored in memory. Fix the regression introduced by fef78212 which, in conjunction with core.trustfolderstats = false, caused any lookup of objects inside the packlist to loop forever when the object was not found in the pack list. Bug: 546190 Change-Id: I38d752ebe47cefc3299740aeba319a2641f19391 Signed-off-by: Luca Milanesio Signed-off-by: Matthias Sohn --- .../storage/file/ObjectDirectoryTest.java | 49 ++++++++++++++++++- .../storage/file/ObjectDirectory.java | 11 +++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java index 923f28389..e9b3d2bc7 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ObjectDirectoryTest.java @@ -42,6 +42,10 @@ package org.eclipse.jgit.internal.storage.file; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.concurrent.Callable; @@ -49,14 +53,29 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; -import org.eclipse.jgit.internal.storage.file.ObjectDirectory; import org.eclipse.jgit.junit.RepositoryTestCase; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.storage.file.FileBasedConfig; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +@RunWith(Parameterized.class) public class ObjectDirectoryTest extends RepositoryTestCase { + @Parameter + public Boolean trustFolderStats; + + @Parameters(name= "core.trustfolderstat={0}") + public static Iterable data() { + return Arrays.asList(Boolean.TRUE, Boolean.FALSE); + } + @Test public void testConcurrentInsertionOfBlobsToTheSameNewFanOutDirectory() throws Exception { @@ -69,6 +88,34 @@ public void testConcurrentInsertionOfBlobsToTheSameNewFanOutDirectory() } } + @Test + public void testShouldNotSearchPacksAgainTheSecondTime() throws Exception { + FileRepository bareRepository = newTestRepositoryWithOnePackfile(); + ObjectDirectory dir = bareRepository.getObjectDatabase(); + + // Make sure that timestamps are modified and read so that a full + // file snapshot check is performed + Thread.sleep(3000L); + + assertTrue(dir.searchPacksAgain(dir.packList.get())); + assertFalse(dir.searchPacksAgain(dir.packList.get())); + } + + private FileRepository newTestRepositoryWithOnePackfile() throws Exception { + FileRepository repository = createBareRepository(); + TestRepository testRepository = new TestRepository(repository); + testRepository.commit(); + testRepository.packAndPrune(); + + FileBasedConfig repoConfig = repository.getConfig(); + repoConfig.setBoolean(ConfigConstants.CONFIG_CORE_SECTION,null, + ConfigConstants.CONFIG_KEY_TRUSTFOLDERSTAT, + trustFolderStats.booleanValue()); + repoConfig.save(); + + return repository; + } + private Collection> blobInsertersForTheSameFanOutDir( final ObjectDirectory dir) { Callable callable = new Callable() { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java index 44ad99bb9..7c7a39ecc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java @@ -127,8 +127,6 @@ public class ObjectDirectory extends FileObjectDatabase { private final File alternatesFile; - private final AtomicReference packList; - private final FS fs; private final AtomicReference alternates; @@ -141,6 +139,8 @@ public class ObjectDirectory extends FileObjectDatabase { private Set shallowCommitsIds; + final AtomicReference packList; + /** * Initialize a reference to an on-disk object directory. * @@ -674,7 +674,7 @@ InsertLooseObjectResult insertUnpackedObject(File tmp, ObjectId id, return InsertLooseObjectResult.FAILURE; } - private boolean searchPacksAgain(PackList old) { + boolean searchPacksAgain(PackList old) { // Whether to trust the pack folder's modification time. If set // to false we will always scan the .git/objects/pack folder to // check for new pack files. If set to true (default) we use the @@ -822,7 +822,8 @@ private PackList scanPacksImpl(final PackList old) { final String packName = base + PACK.getExtension(); final File packFile = new File(packDirectory, packName); final PackFile oldPack = forReuse.remove(packName); - if (oldPack != null && oldPack.getFileSnapshot().isModified(packFile)) { + if (oldPack != null + && !oldPack.getFileSnapshot().isModified(packFile)) { list.add(oldPack); continue; } @@ -960,7 +961,7 @@ public File fileFor(AnyObjectId objectId) { return new File(new File(getDirectory(), d), f); } - private static final class PackList { + static final class PackList { /** State just before reading the pack directory. */ final FileSnapshot snapshot;