diff --git a/.bazelversion b/.bazelversion index 227cea215..7ec1d6db4 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -2.0.0 +2.1.0 diff --git a/Documentation/technical/reftable.md b/Documentation/technical/reftable.md index 1236a7909..ebef68f29 100644 --- a/Documentation/technical/reftable.md +++ b/Documentation/technical/reftable.md @@ -773,9 +773,6 @@ A repository must set its `$GIT_DIR/config` to configure reftable: ### Layout -The `$GIT_DIR/refs` path is a file when reftable is configured, not a -directory. This prevents loose references from being stored. - A collection of reftable files are stored in the `$GIT_DIR/reftable/` directory: @@ -789,28 +786,38 @@ the function `${min_update_index}-${max_update_index}.ref`. Log-only files use the `.log` extension, while ref-only and mixed ref and log files use `.ref`. extension. -The stack ordering file is `$GIT_DIR/refs` and lists the current -files, one per line, in order, from oldest (base) to newest (most -recent): - $ cat .git/refs +The stack ordering file is `$GIT_DIR/reftable/tables.list` and lists the current +files, one per line, in order, from oldest (base) to newest (most recent): + + $ cat .git/reftable/tables.list 00000001-00000001.log 00000002-00000002.ref 00000003-00000003.ref -Readers must read `$GIT_DIR/refs` to determine which files are -relevant right now, and search through the stack in reverse order -(last reftable is examined first). +Readers must read `$GIT_DIR/reftable/tables.list` to determine which files are +relevant right now, and search through the stack in reverse order (last reftable +is examined first). -Reftable files not listed in `refs` may be new (and about to be added +Reftable files not listed in `tables.list` may be new (and about to be added to the stack by the active writer), or ancient and ready to be pruned. +### Backward compatibility + +Older clients should continue to recognize the directory as a git repository so +they don't look for an enclosing repository in parent directories. To this end, +a reftable-enabled repository must contain the following dummy files + +* `.git/HEAD`, a regular file containing `ref: refs/heads/.invalid`. +* `.git/refs/`, a directory +* `.git/refs/heads`, a regular file + ### Readers Readers can obtain a consistent snapshot of the reference space by following: -1. Open and read the `refs` file. +1. Open and read the `tables.list` file. 2. Open each of the reftable files that it mentions. 3. If any of the files is missing, goto 1. 4. Read from the now-open files as long as necessary. @@ -820,13 +827,13 @@ following: Although reftables are immutable, mutations are supported by writing a new reftable and atomically appending it to the stack: -1. Acquire `refs.lock`. -2. Read `refs` to determine current reftables. +1. Acquire `tables.list.lock`. +2. Read `tables.list` to determine current reftables. 3. Select `update_index` to be most recent file's `max_update_index + 1`. 4. Prepare temp reftable `tmp_XXXXXX`, including log entries. 5. Rename `tmp_XXXXXX` to `${update_index}-${update_index}.ref`. -6. Copy `refs` to `refs.lock`, appending file from (5). -7. Rename `refs.lock` to `refs`. +6. Copy `tables.list` to `tables.list.lock`, appending file from (5). +7. Rename `tables.list.lock` to `tables.list`. During step 4 the new file's `min_update_index` and `max_update_index` are both set to the `update_index` selected by step 3. All log @@ -834,9 +841,9 @@ records for the transaction use the same `update_index` in their keys. This enables later correlation of which references were updated by the same transaction. -Because a single `refs.lock` file is used to manage locking, the +Because a single `tables.list.lock` file is used to manage locking, the repository is single-threaded for writers. Writers may have to -busy-spin (with backoff) around creating `refs.lock`, for up to an +busy-spin (with backoff) around creating `tables.list.lock`, for up to an acceptable wait period, aborting if the repository is too busy to mutate. Application servers wrapped around repositories (e.g. Gerrit Code Review) can layer their own lock/wait queue to improve fairness @@ -864,21 +871,21 @@ For sake of illustration, assume the stack currently consists of reftable files (from oldest to newest): A, B, C, and D. The compactor is going to compact B and C, leaving A and D alone. -1. Obtain lock `refs.lock` and read the `refs` file. +1. Obtain lock `tables.list.lock` and read the `tables.list` file. 2. Obtain locks `B.lock` and `C.lock`. Ownership of these locks prevents other processes from trying to compact these files. -3. Release `refs.lock`. +3. Release `tables.list.lock`. 4. Compact `B` and `C` into a temp file `${min_update_index}-${max_update_index}_XXXXXX`. -5. Reacquire lock `refs.lock`. +5. Reacquire lock `tables.list.lock`. 6. Verify that `B` and `C` are still in the stack, in that order. This should always be the case, assuming that other processes are adhering to the locking protocol. 7. Rename `${min_update_index}-${max_update_index}_XXXXXX` to `${min_update_index}-${max_update_index}.ref`. -8. Write the new stack to `refs.lock`, replacing `B` and `C` with the +8. Write the new stack to `tables.list.lock`, replacing `B` and `C` with the file from (4). -9. Rename `refs.lock` to `refs`. +9. Rename `tables.list.lock` to `tables.list`. 10. Delete `B` and `C`, perhaps after a short sleep to avoid forcing readers to backtrack. diff --git a/WORKSPACE b/WORKSPACE index f456e4c31..5fa28f9e2 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -20,7 +20,7 @@ check_bazel_version() load("//tools:bazlets.bzl", "load_bazlets") -load_bazlets(commit = "f53f51fb660552d0581aa0ba52c3836ed63d56a3") +load_bazlets(commit = "f30a992da9fc855dce819875afb59f9dd6f860cd") load( "@com_googlesource_gerrit_bazlets//tools:maven_jar.bzl", diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java index 86239023d..d1522e98e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java @@ -563,10 +563,10 @@ public class RebaseCommandTest extends RepositoryTestCase { RevCommit newD = rw.next(); assertDerivedFrom(newD, d); assertEquals(2, newD.getParentCount()); - RevCommit newC = rw.next(); - assertDerivedFrom(newC, c); RevCommit newE = rw.next(); assertEquals(e, newE); + RevCommit newC = rw.next(); + assertDerivedFrom(newC, c); assertEquals(newC, newD.getParent(0)); assertEquals(e, newD.getParent(1)); assertEquals(g, rw.next()); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java index 6e0762fe3..f235b2eaa 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/dfs/DfsGarbageCollectorTest.java @@ -19,7 +19,6 @@ import java.util.Collections; import java.util.concurrent.TimeUnit; import org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase.PackSource; -import org.eclipse.jgit.internal.storage.dfs.DfsRefDatabase; import org.eclipse.jgit.internal.storage.reftable.RefCursor; import org.eclipse.jgit.internal.storage.reftable.ReftableConfig; import org.eclipse.jgit.internal.storage.reftable.ReftableReader; diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableCompactorTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableCompactorTest.java index 874bab75a..6fc7f2547 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableCompactorTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftable/ReftableCompactorTest.java @@ -19,7 +19,9 @@ import static org.junit.Assert.assertTrue; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import org.eclipse.jgit.internal.storage.io.BlockSource; import org.eclipse.jgit.internal.storage.reftable.ReftableWriter.Stats; @@ -63,7 +65,9 @@ public class ReftableCompactorTest { ReftableCompactor compactor; try (ByteArrayOutputStream outBuf = new ByteArrayOutputStream()) { compactor = new ReftableCompactor(outBuf); - compactor.tryAddFirst(read(inTab)); + List readers = new ArrayList<>(); + readers.add(read(inTab)); + compactor.addAll(readers); compactor.compact(); outTab = outBuf.toByteArray(); } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revplot/PlotCommitListTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revplot/PlotCommitListTest.java index 4e0bba2f2..45225a2bd 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revplot/PlotCommitListTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revplot/PlotCommitListTest.java @@ -254,12 +254,12 @@ public class PlotCommitListTest extends RevWalkTestCase { int posI = test.commit(i).lanePos(childPositions).parents(h) .getLanePos(); test.commit(h).lanePos(posI).parents(f); - test.commit(g).lanePos(childPositions).parents(a); test.commit(f).lanePos(posI).parents(e, d); + test.commit(d).lanePos(1).parents(b); test.commit(e).lanePos(posI).parents(c); - test.commit(d).lanePos(2).parents(b); test.commit(c).lanePos(posI).parents(b); test.commit(b).lanePos(posI).parents(a); + test.commit(g).lanePos(childPositions).parents(a); test.commit(a).lanePos(0).parents(); } } @@ -325,42 +325,42 @@ public class PlotCommitListTest extends RevWalkTestCase { .lanePos(mainPos); test.commit(merge_update_eclipse) .parents(add_a_clear, update_eclipse).lanePos(mainPos); + test.commit(update_eclipse).parents(add_Maven).lanePos(2); test.commit(add_a_clear).parents(fix_broken).lanePos(mainPos); test.commit(fix_broken).parents(merge_disable_comment) .lanePos(mainPos); test.commit(merge_disable_comment) .parents(merge_resolve_handler, disable_comment) .lanePos(mainPos); - test.commit(disable_comment).parents(clone_operation).lanePos(2); + test.commit(disable_comment).parents(clone_operation).lanePos(3); test.commit(merge_resolve_handler) .parents(clone_operation, resolve_handler).lanePos(mainPos); - test.commit(update_eclipse).parents(add_Maven).lanePos(3); + test.commit(resolve_handler).parents(merge_fix).lanePos(4); test.commit(clone_operation).parents(merge_changeset_implementation) .lanePos(mainPos); test.commit(merge_changeset_implementation) .parents(merge_disable_source, changeset_implementation) .lanePos(mainPos); + test.commit(changeset_implementation).parents(clear_repositorycache) + .lanePos(1); test.commit(merge_disable_source) .parents(update_eclipse_iplog2, disable_source) .lanePos(mainPos); + test.commit(disable_source).parents(merge_use_remote).lanePos(3); test.commit(update_eclipse_iplog2).parents(merge_use_remote) .lanePos(mainPos); - test.commit(disable_source).parents(merge_use_remote).lanePos(1); test.commit(merge_use_remote) .parents(update_eclipse_iplog, use_remote).lanePos(mainPos); - test.commit(changeset_implementation).parents(clear_repositorycache) - .lanePos(2); + test.commit(use_remote).parents(clear_repositorycache).lanePos(3); test.commit(update_eclipse_iplog).parents(merge_add_Maven) .lanePos(mainPos); test.commit(merge_add_Maven).parents(findToolBar_layout, add_Maven) .lanePos(mainPos); + test.commit(add_Maven).parents(clear_repositorycache).lanePos(2); test.commit(findToolBar_layout).parents(clear_repositorycache) .lanePos(mainPos); - test.commit(use_remote).parents(clear_repositorycache).lanePos(1); - test.commit(add_Maven).parents(clear_repositorycache).lanePos(3); test.commit(clear_repositorycache).parents(merge_remove) .lanePos(mainPos); - test.commit(resolve_handler).parents(merge_fix).lanePos(4); test.commit(merge_remove).parents(add_simple, remove_unused) .lanePos(mainPos); test.commit(remove_unused).parents(merge_fix).lanePos(1); @@ -453,33 +453,36 @@ public class PlotCommitListTest extends RevWalkTestCase { pcl.source(pw); pcl.fillTo(Integer.MAX_VALUE); - // test that the commits b1, b2 and b3 are on the same position - int bPos = pcl.get(9).lane.position; // b1 - assertEquals("b2 is an a different position", bPos, - pcl.get(7).lane.position); - assertEquals("b3 is on a different position", bPos, - pcl.get(4).lane.position); - - // test that nothing blocks the connections between b1, b2 and b3 - assertNotEquals("b lane is blocked by c", bPos, - pcl.get(8).lane.position); - assertNotEquals("b lane is blocked by a2", bPos, - pcl.get(6).lane.position); - assertNotEquals("b lane is blocked by d", bPos, - pcl.get(5).lane.position); + Set positions = asSet(0, 1); + CommitListAssert test = new CommitListAssert(pcl); + int posA = test.commit(a5).lanePos(positions).getLanePos(); + test.commit(a4); + test.commit(a3).lanePos(posA); + test.commit(e); + test.commit(d); + test.commit(a2).lanePos(posA); + int posB = test.commit(b3).lanePos(positions).getLanePos(); + test.commit(b2).lanePos(posB); + test.commit(b1).lanePos(posB); + test.commit(c); + test.commit(a1).lanePos(posA); + test.noMoreCommits(); + assertNotEquals("a lane is the same as b lane", posA, posB); } } /** *
 	 *    b3
+	 * a5 |
+	 * |  |
 	 * a4 |
 	 * | \|
 	 * |  b2
 	 * a3 |
 	 * | \|
-	 * a2 |
 	 * |  b1
+	 * a2 |
 	 * | /
 	 * a1
 	 * 
@@ -494,10 +497,11 @@ public class PlotCommitListTest extends RevWalkTestCase { final RevCommit a3 = commit(a2, b1); final RevCommit b2 = commit(b1); final RevCommit a4 = commit(a3, b2); + final RevCommit a5 = commit(a4); final RevCommit b3 = commit(b2); try (PlotWalk pw = new PlotWalk(db)) { - pw.markStart(pw.lookupCommit(a4)); + pw.markStart(pw.lookupCommit(a5)); pw.markStart(pw.lookupCommit(b3)); PlotCommitList pcl = new PlotCommitList<>(); pcl.source(pw); @@ -506,11 +510,12 @@ public class PlotCommitListTest extends RevWalkTestCase { Set positions = asSet(0, 1); CommitListAssert test = new CommitListAssert(pcl); int posB = test.commit(b3).lanePos(positions).getLanePos(); - int posA = test.commit(a4).lanePos(positions).getLanePos(); + int posA = test.commit(a5).lanePos(positions).getLanePos(); + test.commit(a4).lanePos(posA); test.commit(b2).lanePos(posB); test.commit(a3).lanePos(posA); - test.commit(a2).lanePos(posA); test.commit(b1).lanePos(posB); + test.commit(a2).lanePos(posA); test.commit(a1).lanePos(posA); test.noMoreCommits(); } @@ -519,13 +524,17 @@ public class PlotCommitListTest extends RevWalkTestCase { /** *
 	 * a4
-	 * |   b3
-	 * a3  |
-	 * | \\|
-	 * |   |\\
-	 * |   b2||
-	 * a2  | //
-	 * |   b1
+	 * |
+	 * a3
+	 * | \\
+	 * a2  \\
+	 * |    \\
+	 * |  b3 ||
+	 * |  |  ||
+	 * |  b2 ||
+	 * |  | //
+	 * |  b1
+	 * |  |
 	 * | /
 	 * a1
 	 * 
@@ -552,10 +561,10 @@ public class PlotCommitListTest extends RevWalkTestCase { Set positions = asSet(0, 1); CommitListAssert test = new CommitListAssert(pcl); int posA = test.commit(a4).lanePos(positions).getLanePos(); - int posB = test.commit(b3).lanePos(positions).getLanePos(); test.commit(a3).lanePos(posA); - test.commit(b2).lanePos(posB); test.commit(a2).lanePos(posA); + int posB = test.commit(b3).lanePos(positions).getLanePos(); + test.commit(b2).lanePos(posB); // b1 is not repositioned, uses "detour lane" // (drawn as a double arc in the ascii graph above) test.commit(b1).lanePos(posB); @@ -569,13 +578,14 @@ public class PlotCommitListTest extends RevWalkTestCase { * b2 * a4 | * | \ | - * a3 \| + * | b1 + * a3 | * | \ | * | c | * | / | * a2 | - * | b1 - * / + * | | + * | / * | / * a1 * @@ -604,10 +614,10 @@ public class PlotCommitListTest extends RevWalkTestCase { CommitListAssert test = new CommitListAssert(pcl); int posB = test.commit(b2).lanePos(positions).getLanePos(); int posA = test.commit(a4).lanePos(positions).getLanePos(); + test.commit(b1).lanePos(posB); // repositioned to go around c test.commit(a3).lanePos(posA); test.commit(c).lanePos(positions); test.commit(a2).lanePos(posA); - test.commit(b1).lanePos(posB); // repositioned to go around c test.commit(a1).lanePos(posA); test.noMoreCommits(); } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkSortTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkSortTest.java index 6f110fa31..3f29e09e3 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkSortTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevWalkSortTest.java @@ -144,4 +144,110 @@ public class RevWalkSortTest extends RevWalkTestCase { assertCommit(d, rw.next()); assertNull(rw.next()); } + + @Test + public void testSort_TOPO_OutOfOrderCommitTimes() throws Exception { + // b is committed before c2 in a different line of history. + // + final RevCommit a = commit(); + final RevCommit c1 = commit(a); + final RevCommit b = commit(a); + final RevCommit c2 = commit(c1); + final RevCommit d = commit(b, c2); + + rw.sort(RevSort.TOPO); + markStart(d); + assertCommit(d, rw.next()); + assertCommit(c2, rw.next()); + assertCommit(c1, rw.next()); + assertCommit(b, rw.next()); + assertCommit(a, rw.next()); + assertNull(rw.next()); + } + + @Test + public void testSort_TOPO_MultipleLinesOfHistory() throws Exception { + final RevCommit a1 = commit(); + final RevCommit b1 = commit(a1); + final RevCommit a2 = commit(a1, b1); + final RevCommit b2 = commit(b1); + final RevCommit b3 = commit(b1); + final RevCommit a3 = commit(a2, b2); + final RevCommit a4 = commit(a3, b3); + + rw.sort(RevSort.TOPO); + markStart(a4); + assertCommit(a4, rw.next()); + assertCommit(b3, rw.next()); + assertCommit(a3, rw.next()); + assertCommit(b2, rw.next()); + assertCommit(a2, rw.next()); + assertCommit(b1, rw.next()); + assertCommit(a1, rw.next()); + assertNull(rw.next()); + } + + @Test + public void testSort_TOPO_REVERSE_MultipleLinesOfHistory() + throws Exception { + final RevCommit a1 = commit(); + final RevCommit b1 = commit(a1); + final RevCommit a2 = commit(a1, b1); + final RevCommit b2 = commit(b1); + final RevCommit b3 = commit(b1); + final RevCommit a3 = commit(a2, b2); + final RevCommit a4 = commit(a3, b3); + + rw.sort(RevSort.TOPO); + rw.sort(RevSort.REVERSE, true); + markStart(a4); + assertCommit(a1, rw.next()); + assertCommit(b1, rw.next()); + assertCommit(a2, rw.next()); + assertCommit(b2, rw.next()); + assertCommit(a3, rw.next()); + assertCommit(b3, rw.next()); + assertCommit(a4, rw.next()); + assertNull(rw.next()); + } + + @Test + public void testSort_TOPO_ParentOfMultipleStartChildren() throws Exception { + final RevCommit a = commit(); + final RevCommit b = commit(a); + final RevCommit c = commit(a); + final RevCommit d1 = commit(a); + final RevCommit d2 = commit(d1); + final RevCommit e = commit(a); + + rw.sort(RevSort.TOPO); + markStart(b); + markStart(c); + markStart(d2); + markStart(e); + assertCommit(e, rw.next()); + assertCommit(d2, rw.next()); + assertCommit(d1, rw.next()); + assertCommit(c, rw.next()); + assertCommit(b, rw.next()); + assertCommit(a, rw.next()); + assertNull(rw.next()); + } + + @Test + public void testSort_TOPO_Uninteresting() throws Exception { + final RevCommit a1 = commit(); + final RevCommit a2 = commit(a1); + final RevCommit a3 = commit(a2); + final RevCommit b = commit(a1); + final RevCommit a4 = commit(a3, b); + + rw.sort(RevSort.TOPO); + markStart(a4); + markUninteresting(a2); + assertCommit(a4, rw.next()); + assertCommit(b, rw.next()); + assertCommit(a3, rw.next()); + assertNull(rw.next()); + } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/StatsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/StatsTest.java index e4927376d..6c4b4a4bb 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/StatsTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/StatsTest.java @@ -12,7 +12,6 @@ package org.eclipse.jgit.util; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import org.eclipse.jgit.util.Stats; import org.junit.Test; public class StatsTest { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java index 4cb0bd3e8..cded6700a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java @@ -452,10 +452,6 @@ public class FileReftableStack implements AutoCloseable { try (FileOutputStream fos = new FileOutputStream(tmpTable)) { ReftableCompactor c = new ReftableCompactor(fos) .setConfig(reftableConfig()) - .setMinUpdateIndex( - stack.get(first).reftableReader.minUpdateIndex()) - .setMaxUpdateIndex( - stack.get(last).reftableReader.maxUpdateIndex()) .setIncludeDeletes(first > 0); List compactMe = new ArrayList<>(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java index 63f0635f7..18c013fd8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/MergedReftable.java @@ -65,6 +65,15 @@ public class MergedReftable extends Reftable { : 0; } + /** + * {@inheritDoc} + */ + @Override + public long minUpdateIndex() throws IOException { + return tables.length > 0 ? tables[0].minUpdateIndex() + : 0; + } + /** {@inheritDoc} */ @Override public boolean hasObjectMap() throws IOException { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/Reftable.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/Reftable.java index dd185f9f7..63786dc17 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/Reftable.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/Reftable.java @@ -65,19 +65,28 @@ public abstract class Reftable { includeDeletes = deletes; } - /** - * Get the maximum update index for log entries that appear in this + * Get the maximum update index for ref entries that appear in this * reftable. * - * @return the maximum update index for log entries that appear in this - * reftable. This should be 1 higher than the prior reftable's - * {@code maxUpdateIndex} if this table is used in a stack. + * @return the maximum update index for ref entries that appear in this + * reftable. * @throws java.io.IOException * file cannot be read. */ public abstract long maxUpdateIndex() throws IOException; + /** + * Get the minimum update index for ref entries that appear in this + * reftable. + * + * @return the minimum update index for ref entries that appear in this + * reftable. + * @throws java.io.IOException + * file cannot be read. + */ + public abstract long minUpdateIndex() throws IOException; + /** * Seek to the first reference, to iterate in order. * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java index 6bc602178..3c4bc7579 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableCompactor.java @@ -28,22 +28,20 @@ import org.eclipse.jgit.lib.ReflogEntry; * to shadow any lower reftable that may have the reference present. *

* By default all log entries within the range defined by - * {@link #setMinUpdateIndex(long)} and {@link #setMaxUpdateIndex(long)} are + * {@link #setReflogExpireMinUpdateIndex(long)} and {@link #setReflogExpireMaxUpdateIndex(long)} are * copied, even if no references in the output file match the log records. * Callers may truncate the log to a more recent time horizon with - * {@link #setOldestReflogTimeMillis(long)}, or disable the log altogether with + * {@link #setReflogExpireOldestReflogTimeMillis(long)}, or disable the log altogether with * {@code setOldestReflogTimeMillis(Long.MAX_VALUE)}. */ public class ReftableCompactor { private final ReftableWriter writer; private final ArrayDeque tables = new ArrayDeque<>(); - private long compactBytesLimit; - private long bytesToCompact; private boolean includeDeletes; - private long minUpdateIndex = -1; - private long maxUpdateIndex; - private long oldestReflogTimeMillis; + private long reflogExpireMinUpdateIndex = 0; + private long reflogExpireMaxUpdateIndex = Long.MAX_VALUE; + private long reflogExpireOldestReflogTimeMillis; private Stats stats; /** @@ -69,18 +67,6 @@ public class ReftableCompactor { return this; } - /** - * Set limit on number of bytes from source tables to compact. - * - * @param bytes - * limit on number of bytes from source tables to compact. - * @return {@code this} - */ - public ReftableCompactor setCompactBytesLimit(long bytes) { - compactBytesLimit = bytes; - return this; - } - /** * Whether to include deletions in the output, which may be necessary for * partial compaction. @@ -106,8 +92,8 @@ public class ReftableCompactor { * in a stack. * @return {@code this} */ - public ReftableCompactor setMinUpdateIndex(long min) { - minUpdateIndex = min; + public ReftableCompactor setReflogExpireMinUpdateIndex(long min) { + reflogExpireMinUpdateIndex = min; return this; } @@ -122,8 +108,8 @@ public class ReftableCompactor { * used in a stack. * @return {@code this} */ - public ReftableCompactor setMaxUpdateIndex(long max) { - maxUpdateIndex = max; + public ReftableCompactor setReflogExpireMaxUpdateIndex(long max) { + reflogExpireMaxUpdateIndex = max; return this; } @@ -137,16 +123,13 @@ public class ReftableCompactor { * Specified in Java standard milliseconds since the epoch. * @return {@code this} */ - public ReftableCompactor setOldestReflogTimeMillis(long timeMillis) { - oldestReflogTimeMillis = timeMillis; + public ReftableCompactor setReflogExpireOldestReflogTimeMillis(long timeMillis) { + reflogExpireOldestReflogTimeMillis = timeMillis; return this; } /** * Add all of the tables, in the specified order. - *

- * Unconditionally adds all tables, ignoring the - * {@link #setCompactBytesLimit(long)}. * * @param readers * tables to compact. Tables should be ordered oldest first/most @@ -158,44 +141,7 @@ public class ReftableCompactor { public void addAll(List readers) throws IOException { for (ReftableReader r : readers) { tables.add(r); - adjustUpdateIndexes(r); - } - } - - /** - * Try to add this reader at the bottom of the stack. - *

- * A reader may be rejected by returning {@code false} if the compactor is - * already rewriting its {@link #setCompactBytesLimit(long)}. When this - * happens the caller should stop trying to add tables, and execute the - * compaction. - * - * @param reader - * the reader to insert at the bottom of the stack. Caller is - * responsible for closing the reader. - * @return {@code true} if the compactor accepted this table; {@code false} - * if the compactor has reached its limit. - * @throws java.io.IOException - * if size of {@code reader}, or its update indexes cannot be read. - */ - public boolean tryAddFirst(ReftableReader reader) throws IOException { - long sz = reader.size(); - if (compactBytesLimit > 0 && bytesToCompact + sz > compactBytesLimit) { - return false; } - bytesToCompact += sz; - adjustUpdateIndexes(reader); - tables.addFirst(reader); - return true; - } - - private void adjustUpdateIndexes(ReftableReader reader) throws IOException { - if (minUpdateIndex == -1) { - minUpdateIndex = reader.minUpdateIndex(); - } else { - minUpdateIndex = Math.min(minUpdateIndex, reader.minUpdateIndex()); - } - maxUpdateIndex = Math.max(maxUpdateIndex, reader.maxUpdateIndex()); } /** @@ -208,8 +154,9 @@ public class ReftableCompactor { MergedReftable mr = new MergedReftable(new ArrayList<>(tables)); mr.setIncludeDeletes(includeDeletes); - writer.setMinUpdateIndex(Math.max(minUpdateIndex, 0)); - writer.setMaxUpdateIndex(maxUpdateIndex); + writer.setMaxUpdateIndex(mr.maxUpdateIndex()); + writer.setMinUpdateIndex(mr.minUpdateIndex()); + writer.begin(); mergeRefs(mr); mergeLogs(mr); @@ -235,16 +182,14 @@ public class ReftableCompactor { } private void mergeLogs(MergedReftable mr) throws IOException { - if (oldestReflogTimeMillis == Long.MAX_VALUE) { + if (reflogExpireOldestReflogTimeMillis == Long.MAX_VALUE) { return; } try (LogCursor lc = mr.allLogs()) { while (lc.next()) { long updateIndex = lc.getUpdateIndex(); - if (updateIndex < minUpdateIndex - || updateIndex > maxUpdateIndex) { - // Cannot merge log records outside the header's range. + if (updateIndex > reflogExpireMaxUpdateIndex || updateIndex < reflogExpireMinUpdateIndex) { continue; } @@ -258,7 +203,7 @@ public class ReftableCompactor { } PersonIdent who = log.getWho(); - if (who.getWhen().getTime() >= oldestReflogTimeMillis) { + if (who.getWhen().getTime() >= reflogExpireOldestReflogTimeMillis) { writer.writeLog( refName, updateIndex, diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java index c19a6d38f..095276f57 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftable/ReftableReader.java @@ -106,15 +106,9 @@ public class ReftableReader extends Reftable implements AutoCloseable { } /** - * Get the minimum update index for log entries that appear in this - * reftable. - * - * @return the minimum update index for log entries that appear in this - * reftable. This should be 1 higher than the prior reftable's - * {@code maxUpdateIndex} if this table is used in a stack. - * @throws java.io.IOException - * file cannot be read. + * {@inheritDoc} */ + @Override public long minUpdateIndex() throws IOException { if (blockSize == -1) { readFileHeader(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevObject.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevObject.java index 5ce4bc33b..1abcf6962 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevObject.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevObject.java @@ -151,7 +151,7 @@ public abstract class RevObject extends ObjectIdOwnerMap.Entry { * buffer to append a debug description of core RevFlags onto. */ protected void appendCoreFlags(StringBuilder s) { - s.append((flags & RevWalk.TOPO_DELAY) != 0 ? 'o' : '-'); + s.append((flags & RevWalk.TOPO_QUEUED) != 0 ? 'o' : '-'); s.append((flags & RevWalk.TEMP_MARK) != 0 ? 't' : '-'); s.append((flags & RevWalk.REWRITE) != 0 ? 'r' : '-'); s.append((flags & RevWalk.UNINTERESTING) != 0 ? 'u' : '-'); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java index f425e8761..383428c85 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java @@ -125,11 +125,11 @@ public class RevWalk implements Iterable, AutoCloseable { /** * Temporary mark for use within {@link TopoSortGenerator}. *

- * This mark indicates the commit could not produce when it wanted to, as at - * least one child was behind it. Commits with this flag are delayed until - * all children have been output first. + * This mark indicates the commit has been queued for emission in + * {@link TopoSortGenerator} and can be produced. This mark is removed when + * the commit has been produced. */ - static final int TOPO_DELAY = 1 << 5; + static final int TOPO_QUEUED = 1 << 5; /** Number of flag bits we keep internal for our own use. See above flags. */ static final int RESERVED_FLAGS = 6; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TopoSortGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TopoSortGenerator.java index 7a5db43a7..3c553b06c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TopoSortGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TopoSortGenerator.java @@ -17,7 +17,7 @@ import org.eclipse.jgit.errors.MissingObjectException; /** Sorts commits in topological order. */ class TopoSortGenerator extends Generator { - private static final int TOPO_DELAY = RevWalk.TOPO_DELAY; + private static final int TOPO_QUEUED = RevWalk.TOPO_QUEUED; private final FIFORevQueue pending; @@ -47,12 +47,16 @@ class TopoSortGenerator extends Generator { if (c == null) { break; } - for (RevCommit p : c.parents) { - p.inDegree++; - if (firstParent) { - break; + if ((c.flags & TOPO_QUEUED) == 0) { + for (RevCommit p : c.parents) { + p.inDegree++; + + if (firstParent) { + break; + } } } + c.flags |= TOPO_QUEUED; pending.add(c); } } @@ -71,34 +75,42 @@ class TopoSortGenerator extends Generator { RevCommit next() throws MissingObjectException, IncorrectObjectTypeException, IOException { for (;;) { - final RevCommit c = pending.next(); - if (c == null) + RevCommit c = pending.next(); + if (c == null) { return null; + } if (c.inDegree > 0) { // At least one of our children is missing. We delay // production until all of our children are output. // - c.flags |= TOPO_DELAY; continue; } - // All of our children have already produced, - // so it is OK for us to produce now as well. - // + if ((c.flags & TOPO_QUEUED) == 0) { + // c is a parent that already produced or a parent that + // was never in the priority queue and should never produce. + // + continue; + } + for (RevCommit p : c.parents) { - if (--p.inDegree == 0 && (p.flags & TOPO_DELAY) != 0) { - // This parent tried to come before us, but we are - // his last child. unpop the parent so it goes right - // behind this child. + if (--p.inDegree == 0 && (p.flags & TOPO_QUEUED) != 0) { + // The parent has no unproduced interesting children. unpop + // the parent so it goes right behind this child. This means + // that this parent commit may appear in "pending" more than + // once, but this is safe since upon the second and + // subsequent iterations with this commit, it will no longer + // have TOPO_QUEUED set, and thus will be skipped. // - p.flags &= ~TOPO_DELAY; pending.unpop(p); } if (firstParent) { break; } } + + c.flags &= ~TOPO_QUEUED; return c; } } diff --git a/pom.xml b/pom.xml index d0a5c83a1..3d9acd43f 100644 --- a/pom.xml +++ b/pom.xml @@ -234,7 +234,7 @@ org.apache.maven.plugins maven-shade-plugin - 3.2.1 + 3.2.2 @@ -846,12 +846,12 @@ org.codehaus.plexus plexus-compiler-javac - 2.8.5 + 2.8.6 org.codehaus.plexus plexus-compiler-javac-errorprone - 2.8.5 + 2.8.6 @@ -890,7 +890,7 @@ org.codehaus.plexus plexus-compiler-eclipse - 2.8.5 + 2.8.6 org.eclipse.jdt