DfsObjDatabase: read reftables from midx covered packs
The multipack index is build over existing packs and some can have
also reftables. getPacks() is only looking for reftables in top level
packs, missing those hidden inside the midx.
Process first all packs (which goes recursively inside the midxs)
recording which ones have reftables. Then process the reftables.
Change-Id: Ib779f619a522b7f1073d96197d9801af3dae67ca
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabaseTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabaseTest.java
index e515696..c30753e 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabaseTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabaseTest.java
@@ -14,6 +14,7 @@
import static org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource.INSERT;
import static org.eclipse.jgit.internal.storage.pack.PackExt.MULTI_PACK_INDEX;
import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK;
+import static org.eclipse.jgit.internal.storage.pack.PackExt.REFTABLE;
import static org.junit.Assert.assertEquals;
import java.io.IOException;
@@ -177,12 +178,86 @@ public void getPacks_midxChain_midxDisabled_packsNoMidx()
gcPack);
}
+ @Test
+ public void getReftables_multipleInsideMidx() throws IOException {
+ db.getObjectDatabase().setUseMultipackIndex(true);
+
+ DfsPackDescription gcPack = pack("aaaa", GC, 100, PACK, REFTABLE);
+ DfsPackDescription compactPack = pack("cccc", COMPACT, 101, PACK,
+ REFTABLE);
+ DfsPackDescription compactTwoPack = pack("bbbb", COMPACT, 102, PACK);
+
+ DfsPackDescription multiPackIndex = pack("xxxx", GC, 104,
+ MULTI_PACK_INDEX);
+ multiPackIndex
+ .setCoveredPacks(List.of(gcPack, compactPack, compactTwoPack));
+
+ db.getObjectDatabase().commitPack(
+ List.of(gcPack, compactPack, compactTwoPack, multiPackIndex),
+ Collections.emptyList());
+
+ DfsReftable[] reftables = db.getObjectDatabase().getReftables();
+ assertReftableList(reftables, gcPack, compactPack);
+ }
+
+ @Test
+ public void getReftables_midxChain() throws IOException {
+ db.getObjectDatabase().setUseMultipackIndex(true);
+
+ DfsPackDescription gcPack = pack("aaaa", GC, 100, PACK, REFTABLE);
+ DfsPackDescription compactPack = pack("cccc", COMPACT, 101, PACK,
+ REFTABLE);
+ DfsPackDescription insertPack = pack("bbbb", COMPACT, 102, PACK);
+
+ DfsPackDescription midxBase = pack("xxxx", GC, 104, MULTI_PACK_INDEX);
+ midxBase.setCoveredPacks(List.of(gcPack, compactPack, insertPack));
+ db.getObjectDatabase().commitPack(
+ List.of(gcPack, compactPack, insertPack, midxBase), null);
+
+ DfsPackDescription insert1 = pack("insert1", INSERT, 105, PACK,
+ REFTABLE);
+ DfsPackDescription insert2 = pack("insert2", INSERT, 106, PACK);
+ DfsPackDescription midxTip = pack("xxx2", GC, 107, MULTI_PACK_INDEX);
+ midxTip.setCoveredPacks(List.of(insert1, insert2));
+ midxTip.setMultiPackIndexBase(midxBase);
+ db.getObjectDatabase().commitPack(List.of(insert1, insert2, midxTip),
+ null);
+
+ DfsReftable[] reftables = db.getObjectDatabase().getReftables();
+ assertReftableList(reftables, gcPack, compactPack, insert1);
+ }
+
+ @Test
+ public void getReftables_inAndOutOfMidx() throws IOException {
+ db.getObjectDatabase().setUseMultipackIndex(true);
+
+ DfsPackDescription gcPack = pack("aaaa", GC, 100, PACK, REFTABLE);
+ DfsPackDescription compactPack = pack("cccc", COMPACT, 101, PACK);
+ DfsPackDescription insertPack = pack("bbbb", COMPACT, 102, PACK);
+
+ DfsPackDescription multiPackIndex = pack("xxxx", GC, 104,
+ MULTI_PACK_INDEX);
+ multiPackIndex
+ .setCoveredPacks(List.of(gcPack, compactPack, insertPack));
+ db.getObjectDatabase().commitPack(
+ List.of(gcPack, compactPack, insertPack, multiPackIndex), null);
+
+ DfsPackDescription uncoveredPack = pack("dddd", COMPACT, 103, PACK,
+ REFTABLE);
+ db.getObjectDatabase().commitPack(List.of(uncoveredPack), null);
+
+ DfsReftable[] reftables = db.getObjectDatabase().getReftables();
+ assertReftableList(reftables, gcPack, uncoveredPack);
+ }
+
private static DfsPackDescription pack(String name,
- DfsObjDatabase.PackSource source, long timeMs, PackExt ext) {
+ DfsObjDatabase.PackSource source, long timeMs, PackExt... ext) {
DfsPackDescription desc = new DfsPackDescription(repoDesc, name,
source);
desc.setLastModified(timeMs);
- desc.addFileExt(ext);
+ for (PackExt packExt : ext) {
+ desc.addFileExt(packExt);
+ }
return desc;
}
@@ -193,4 +268,12 @@ private static void assertPackList(DfsPackFile[] actual,
assertEquals(expected[i], actual[i].getPackDescription());
}
}
+
+ private static void assertReftableList(DfsReftable[] actual,
+ DfsPackDescription... expected) {
+ assertEquals(expected.length, actual.length);
+ for (int i = 0; i < expected.length; i++) {
+ assertEquals(expected[i], actual[i].getPackDescription());
+ }
+ }
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java
index 97fdd34..7b7a378 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsObjDatabase.java
@@ -14,13 +14,13 @@
import static org.eclipse.jgit.internal.storage.pack.PackExt.BITMAP_INDEX;
import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX;
import static org.eclipse.jgit.internal.storage.pack.PackExt.MULTI_PACK_INDEX;
+import static org.eclipse.jgit.internal.storage.pack.PackExt.REFTABLE;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
-import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
@@ -614,10 +614,10 @@ private PackList scanPacksImpl(PackList old) throws IOException {
Map<DfsPackDescription, DfsReftable> reftables = reftableMap(old);
List<DfsPackDescription> scanned = listPacks();
- Collections.sort(scanned, packComparator);
+ scanned.sort(packComparator);
List<DfsPackFile> newPacks = new ArrayList<>(scanned.size());
- List<DfsReftable> newReftables = new ArrayList<>(scanned.size());
+ List<DfsPackDescription> packsWithReftables = new ArrayList<>();
boolean foundNew = false;
for (DfsPackDescription dsc : scanned) {
DfsPackFile oldPack = packs.remove(dsc);
@@ -627,10 +627,19 @@ private PackList scanPacksImpl(PackList old) throws IOException {
newPacks.add(createDfsPackFile(cache, dsc));
foundNew = true;
} else if (dsc.hasFileExt(MULTI_PACK_INDEX)) {
- newPacks.add(createDfsPackFileMidx(cache, dsc));
+ newPacks.add(
+ createDfsPackFileMidx(cache, dsc, packsWithReftables));
foundNew = true;
}
+ if (dsc.hasFileExt(REFTABLE)) {
+ packsWithReftables.add(dsc);
+ }
+ }
+
+ List<DfsReftable> newReftables = new ArrayList<>(
+ packsWithReftables.size());
+ for (DfsPackDescription dsc : packsWithReftables) {
DfsReftable oldReftable = reftables.remove(dsc);
if (oldReftable != null) {
newReftables.add(oldReftable);
@@ -645,7 +654,7 @@ private PackList scanPacksImpl(PackList old) throws IOException {
if (!foundNew) {
return old;
}
- Collections.sort(newReftables, reftableComparator());
+ newReftables.sort(reftableComparator());
return new PackList(
newPacks.toArray(new DfsPackFile[0]),
newReftables.toArray(new DfsReftable[0]));
@@ -653,7 +662,7 @@ private PackList scanPacksImpl(PackList old) throws IOException {
/**
* Create instances of DfsPackFile
- *
+ * <p>
* Implementors can decide to construct or wrap DfsPackFile in different
* ways.
*
@@ -675,14 +684,19 @@ protected DfsPackFile createDfsPackFile(DfsBlockCache cache,
* block cache
* @param dsc
* pack description
+ * @param containsReftables
+ * used to return packs inside this midx that also have reftables
+ *
* @return the dfs packfile
*/
protected DfsPackFileMidx createDfsPackFileMidx(DfsBlockCache cache,
- DfsPackDescription dsc) {
+ DfsPackDescription dsc,
+ List<DfsPackDescription> containsReftables) {
DfsPackFileMidx base = null;
if (dsc.getMultiPackIndexBase() != null) {
// The base is always a multipack index
- base = createDfsPackFileMidx(cache, dsc.getMultiPackIndexBase());
+ base = createDfsPackFileMidx(cache, dsc.getMultiPackIndexBase(),
+ containsReftables);
}
// A pack shouldn't be in the pack list and inside a multipack index
// at the same time. In that case, we will have it under two
@@ -690,6 +704,8 @@ protected DfsPackFileMidx createDfsPackFileMidx(DfsBlockCache cache,
List<DfsPackFile> coveredPacks = dsc.getCoveredPacks().stream()
.map(desc -> createDfsPackFile(cache, desc))
.collect(Collectors.toUnmodifiableList());
+ dsc.getCoveredPacks().stream().filter(d -> d.hasFileExt(REFTABLE))
+ .forEach(containsReftables::add);
return DfsPackFileMidx.create(cache, dsc, coveredPacks, base);
}
