Skip to content

Commit 0b6d037

Browse files
committed
MidxPackList: getAllPlainPacks returns object-lookup order
For object lookup we want the most recent packs first, because we usually talk more about recent object. In midx, we want oldest packs first to choose the oldest copy of an object for duplicates. MidxPackList replaces a midx in the pack list with its covered packs, but it keep them in midx order, producing a list that is in no useful order. e.g. [INSERT, midx(GC, COMPACT)] returns [INSERT, GC, COMPACT]. Introduced the covered packs in reverse order, so the list matches the object lookup order. Then [INSERT, midx(GC, COMPACT)] becomes [INSERT, COMPACT, GC]. Link: https://review.gerrithub.io/id/I96458e5585b00d31ae6f1085412a2f5b6a6a6964
1 parent a16b144 commit 0b6d037

File tree

2 files changed

+15
-6
lines changed

2 files changed

+15
-6
lines changed

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/MidxPackListTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public void getAllPlainPacks_onlyMidx() {
4444
DfsPackFileMidx midx = packPool.midx("midx", null, "a", "b", "c");
4545

4646
MidxPackList packList = MidxPackList.create(midx);
47-
assertEquals(List.of(a, b, c), packList.getAllPlainPacks());
47+
assertEquals(List.of(c, b, a), packList.getAllPlainPacks());
4848
}
4949

5050
@Test
@@ -56,7 +56,7 @@ public void getAllPlainPacks_midxPlusOne() {
5656
DfsPackFile d = packPool.pack("d");
5757

5858
MidxPackList packList = MidxPackList.create(d, midx);
59-
assertEquals(List.of(d, a, b, c), packList.getAllPlainPacks());
59+
assertEquals(List.of(d, c, b, a), packList.getAllPlainPacks());
6060
}
6161

6262
@Test
@@ -74,7 +74,7 @@ public void getAllPlainPacks_nestedMidx() {
7474
"f");
7575

7676
MidxPackList packList = MidxPackList.create(midxTip);
77-
assertEquals(List.of(e, f, c, d, a, b), packList.getAllPlainPacks());
77+
assertEquals(List.of(f, e, d, c, b, a), packList.getAllPlainPacks());
7878
}
7979

8080
@Test
@@ -304,9 +304,9 @@ public void getPlainPacksNotCoveredBy_midxChain() {
304304
"f");
305305

306306
MidxPackList packList = MidxPackList.create(midxTip);
307-
assertEquals(List.of(e, f, c, d),
307+
assertEquals(List.of(f, e, d, c),
308308
packList.getPlainPacksNotCoveredBy(midxBase));
309-
assertEquals(List.of(e, f),
309+
assertEquals(List.of(f, e),
310310
packList.getPlainPacksNotCoveredBy(midxMiddle));
311311
assertEquals(List.of(), packList.getPlainPacksNotCoveredBy(midxTip));
312312
}

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/MidxPackList.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ private MidxPackList(List<DfsPackFile> packs) {
5353

5454
/**
5555
* Get all plain packs in the list, either top-level or inside midxs
56+
* <p>
57+
* Inside midx, the packs are in reverse lookup order. This code restores
58+
* their original order. i.e. the list with packs [INSERT, midx(COMPACT-2,
59+
* COMPACT-3), midx(GC, COMPACT-1)] becomes [INSERT, COMPACT-3, COMPACT-2,
60+
* COMPACT-1, GC].
5661
*
5762
* @return a list of all "real" packs in this pack list, either top level or
5863
* inside midxs.
@@ -63,7 +68,11 @@ public List<DfsPackFile> getAllPlainPacks() {
6368
while (!pending.isEmpty()) {
6469
DfsPackFile pack = pending.poll();
6570
if (pack instanceof DfsPackFileMidx midxPack) {
66-
plainPacks.addAll(midxPack.getCoveredPacks());
71+
// Midx order is the reverse of object lookup order
72+
ArrayList<DfsPackFile> coveredPacks = new ArrayList<>(
73+
midxPack.getCoveredPacks());
74+
Collections.reverse(coveredPacks);
75+
plainPacks.addAll(coveredPacks);
6776
if (midxPack.getMultipackIndexBase() != null) {
6877
pending.add(midxPack.getMultipackIndexBase());
6978
}

0 commit comments

Comments
 (0)