diff options
author | Matthias Sohn <matthias.sohn@sap.com> | 2021-06-06 10:45:57 +0200 |
---|---|---|
committer | Matthias Sohn <matthias.sohn@sap.com> | 2021-06-13 23:49:25 +0200 |
commit | 7856402c4bee5df4a6211ed8c711eb4d96a4f6c9 (patch) | |
tree | b7a74a84bc5945383a294ba7e8c0ad2e25474398 | |
parent | 37436cc9334ed7bf49bf1d297ae4f90d5f141068 (diff) | |
parent | 5977260af6641b9a82491728d6fe0431b5cd5331 (diff) | |
download | jgit-7856402c4bee5df4a6211ed8c711eb4d96a4f6c9.tar.gz jgit-7856402c4bee5df4a6211ed8c711eb4d96a4f6c9.zip |
Merge branch 'stable-5.5' into stable-5.6
* stable-5.5:
Prepare 5.1.17-SNAPSHOT builds
JGit v5.1.16.202106041830-r
BatchRefUpdate: Skip saving conflicting ref names and prefixes in
memory
BatchRefUpdateTest: Accurately assert RefsChangedEvent(s) fired
Optimize RefDirectory.isNameConflicting()
Update bazlets and bazel version
Change-Id: I504483a4dc979c5e7af18bad45dc18675e32afd2
3 files changed, 317 insertions, 119 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java index cb5b07a4db..32513c9637 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/BatchRefUpdateTest.java @@ -223,15 +223,28 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { if (atomic) { assertResults(cmds, TRANSACTION_ABORTED, REJECTED_NONFASTFORWARD); assertRefs("refs/heads/master", A, "refs/heads/masters", B); - assertEquals(1, refsChangedEvents); } else { assertResults(cmds, OK, REJECTED_NONFASTFORWARD); assertRefs("refs/heads/master", B, "refs/heads/masters", B); - assertEquals(2, refsChangedEvents); } } @Test + public void simpleNoForceRefsChangedEvents() throws IOException { + writeLooseRefs("refs/heads/master", A, "refs/heads/masters", B); + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(A, B, "refs/heads/master", UPDATE), + new ReceiveCommand(B, A, "refs/heads/masters", + UPDATE_NONFASTFORWARD)); + execute(newBatchUpdate(cmds)); + + assertEquals(atomic ? initialRefsChangedEvents + : initialRefsChangedEvents + 1, refsChangedEvents); + } + + @Test public void simpleForce() throws IOException { writeLooseRefs("refs/heads/master", A, "refs/heads/masters", B); @@ -243,7 +256,21 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, OK, OK); assertRefs("refs/heads/master", B, "refs/heads/masters", A); - assertEquals(batchesRefUpdates() ? 2 : 3, refsChangedEvents); + } + + @Test + public void simpleForceRefsChangedEvents() throws IOException { + writeLooseRefs("refs/heads/master", A, "refs/heads/masters", B); + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(A, B, "refs/heads/master", UPDATE), + new ReceiveCommand(B, A, "refs/heads/masters", + UPDATE_NONFASTFORWARD)); + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true)); + + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); } @Test @@ -265,7 +292,27 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, OK); assertRefs("refs/heads/master", A); - assertEquals(2, refsChangedEvents); + } + + @Test + public void nonFastForwardDoesNotDoExpensiveMergeCheckRefsChangedEvents() + throws IOException { + writeLooseRef("refs/heads/master", B); + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays.asList(new ReceiveCommand(B, A, + "refs/heads/master", UPDATE_NONFASTFORWARD)); + try (RevWalk rw = new RevWalk(diskRepo) { + @Override + public boolean isMergedInto(RevCommit base, RevCommit tip) { + throw new AssertionError("isMergedInto() should not be called"); + } + }) { + newBatchUpdate(cmds).setAllowNonFastForwards(true).execute(rw, + new StrictWorkMonitor()); + } + + assertEquals(initialRefsChangedEvents + 1, refsChangedEvents); } @Test @@ -284,17 +331,30 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, LOCK_FAILURE, TRANSACTION_ABORTED, TRANSACTION_ABORTED); assertRefs("refs/heads/master", A, "refs/heads/masters", B); - assertEquals(1, refsChangedEvents); } else { // Non-atomic updates are applied in order: master succeeds, then // master/x fails due to conflict. assertResults(cmds, OK, LOCK_FAILURE, LOCK_FAILURE); assertRefs("refs/heads/master", B, "refs/heads/masters", B); - assertEquals(2, refsChangedEvents); } } @Test + public void fileDirectoryConflictRefsChangedEvents() throws IOException { + writeLooseRefs("refs/heads/master", A, "refs/heads/masters", B); + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(A, B, "refs/heads/master", UPDATE), + new ReceiveCommand(zeroId(), A, "refs/heads/master/x", CREATE), + new ReceiveCommand(zeroId(), A, "refs/heads", CREATE)); + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true), false); + + assertEquals(atomic ? initialRefsChangedEvents + : initialRefsChangedEvents + 1, refsChangedEvents); + } + + @Test public void conflictThanksToDelete() throws IOException { writeLooseRefs("refs/heads/master", A, "refs/heads/masters", B); @@ -306,15 +366,21 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, OK, OK, OK); assertRefs("refs/heads/master", B, "refs/heads/masters/x", A); - if (atomic) { - assertEquals(2, refsChangedEvents); - } else if (!useReftable) { - // The non-atomic case actually produces 5 events, but that's an - // implementation detail. We expect at least 4 events, one for the - // initial read due to writeLooseRef(), and then one for each - // successful ref update. - assertTrue(refsChangedEvents >= 4); - } + } + + @Test + public void conflictThanksToDeleteRefsChangedEvents() throws IOException { + writeLooseRefs("refs/heads/master", A, "refs/heads/masters", B); + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(A, B, "refs/heads/master", UPDATE), + new ReceiveCommand(zeroId(), A, "refs/heads/masters/x", CREATE), + new ReceiveCommand(B, zeroId(), "refs/heads/masters", DELETE)); + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true)); + + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 3, refsChangedEvents); } @Test @@ -331,15 +397,29 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { if (atomic) { assertResults(cmds, REJECTED_MISSING_OBJECT, TRANSACTION_ABORTED); assertRefs("refs/heads/master", A); - assertEquals(1, refsChangedEvents); } else { assertResults(cmds, REJECTED_MISSING_OBJECT, OK); assertRefs("refs/heads/master", A, "refs/heads/foo2", B); - assertEquals(2, refsChangedEvents); } } @Test + public void updateToMissingObjectRefsChangedEvents() throws IOException { + writeLooseRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; + + ObjectId bad = ObjectId + .fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(A, bad, "refs/heads/master", UPDATE), + new ReceiveCommand(zeroId(), B, "refs/heads/foo2", CREATE)); + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true), false); + + assertEquals(atomic ? initialRefsChangedEvents + : initialRefsChangedEvents + 1, refsChangedEvents); + } + + @Test public void addMissingObject() throws IOException { writeLooseRef("refs/heads/master", A); @@ -353,15 +433,29 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { if (atomic) { assertResults(cmds, TRANSACTION_ABORTED, REJECTED_MISSING_OBJECT); assertRefs("refs/heads/master", A); - assertEquals(1, refsChangedEvents); } else { assertResults(cmds, OK, REJECTED_MISSING_OBJECT); assertRefs("refs/heads/master", B); - assertEquals(2, refsChangedEvents); } } @Test + public void addMissingObjectRefsChangedEvents() throws IOException { + writeLooseRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; + + ObjectId bad = ObjectId + .fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(A, B, "refs/heads/master", UPDATE), + new ReceiveCommand(zeroId(), bad, "refs/heads/foo2", CREATE)); + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true), false); + + assertEquals(atomic ? initialRefsChangedEvents + : initialRefsChangedEvents + 1, refsChangedEvents); + } + + @Test public void oneNonExistentRef() throws IOException { List<ReceiveCommand> cmds = Arrays.asList( new ReceiveCommand(A, B, "refs/heads/foo1", UPDATE), @@ -391,15 +485,27 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { if (atomic) { assertResults(cmds, LOCK_FAILURE, TRANSACTION_ABORTED); assertRefs("refs/heads/master", A); - assertEquals(1, refsChangedEvents); } else { assertResults(cmds, LOCK_FAILURE, OK); assertRefs("refs/heads/master", A, "refs/heads/foo2", B); - assertEquals(2, refsChangedEvents); } } @Test + public void oneRefWrongOldValueRefsChangedEvents() throws IOException { + writeLooseRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(B, B, "refs/heads/master", UPDATE), + new ReceiveCommand(zeroId(), B, "refs/heads/foo2", CREATE)); + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true)); + + assertEquals(atomic ? initialRefsChangedEvents + : initialRefsChangedEvents + 1, refsChangedEvents); + } + + @Test public void nonExistentRef() throws IOException { writeLooseRef("refs/heads/master", A); @@ -411,17 +517,31 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { if (atomic) { assertResults(cmds, TRANSACTION_ABORTED, LOCK_FAILURE); assertRefs("refs/heads/master", A); - assertEquals(1, refsChangedEvents); } else { assertResults(cmds, OK, LOCK_FAILURE); assertRefs("refs/heads/master", B); - assertEquals(2, refsChangedEvents); } } @Test + public void nonExistentRefRefsChangedEvents() throws IOException { + writeLooseRef("refs/heads/master", A); + + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(A, B, "refs/heads/master", UPDATE), + new ReceiveCommand(A, zeroId(), "refs/heads/foo2", DELETE)); + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true)); + + assertEquals(atomic ? initialRefsChangedEvents + : initialRefsChangedEvents + 1, refsChangedEvents); + } + + @Test public void noRefLog() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; Map<String, ReflogEntry> oldLogs = getLastReflogs("refs/heads/master", "refs/heads/branch"); @@ -435,7 +555,8 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, OK, OK); assertRefs("refs/heads/master", B, "refs/heads/branch", B); - assertEquals(batchesRefUpdates() ? 2 : 3, refsChangedEvents); + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertReflogUnchanged(oldLogs, "refs/heads/master"); assertReflogUnchanged(oldLogs, "refs/heads/branch"); } @@ -444,6 +565,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { public void reflogDefaultIdent() throws IOException { writeRef("refs/heads/master", A); writeRef("refs/heads/branch2", A); + int initialRefsChangedEvents = refsChangedEvents; Map<String, ReflogEntry> oldLogs = getLastReflogs("refs/heads/master", "refs/heads/branch1", "refs/heads/branch2"); @@ -456,7 +578,8 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, OK, OK); assertRefs("refs/heads/master", B, "refs/heads/branch1", B, "refs/heads/branch2", A); - assertEquals(batchesRefUpdates() ? 3 : 4, refsChangedEvents); + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertReflogEquals(reflog(A, B, new PersonIdent(diskRepo), "a reflog"), getLastReflog("refs/heads/master")); assertReflogEquals( @@ -469,6 +592,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { public void reflogAppendStatusNoMessage() throws IOException { writeRef("refs/heads/master", A); writeRef("refs/heads/branch1", B); + int initialRefsChangedEvents = refsChangedEvents; List<ReceiveCommand> cmds = Arrays.asList( new ReceiveCommand(A, B, "refs/heads/master", UPDATE), @@ -481,7 +605,8 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, OK, OK, OK); assertRefs("refs/heads/master", B, "refs/heads/branch1", A, "refs/heads/branch2", A); - assertEquals(batchesRefUpdates() ? 3 : 5, refsChangedEvents); + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 3, refsChangedEvents); assertReflogEquals( // Always forced; setAllowNonFastForwards(true) bypasses the // check. @@ -498,6 +623,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { @Test public void reflogAppendStatusFastForward() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; List<ReceiveCommand> cmds = Arrays .asList(new ReceiveCommand(A, B, "refs/heads/master", UPDATE)); @@ -505,7 +631,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, OK); assertRefs("refs/heads/master", B); - assertEquals(2, refsChangedEvents); + assertEquals(initialRefsChangedEvents + 1, refsChangedEvents); assertReflogEquals( reflog(A, B, new PersonIdent(diskRepo), "fast-forward"), getLastReflog("refs/heads/master")); @@ -514,6 +640,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { @Test public void reflogAppendStatusWithMessage() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; List<ReceiveCommand> cmds = Arrays.asList( new ReceiveCommand(A, B, "refs/heads/master", UPDATE), @@ -522,7 +649,8 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, OK, OK); assertRefs("refs/heads/master", B, "refs/heads/branch", A); - assertEquals(batchesRefUpdates() ? 2 : 3, refsChangedEvents); + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertReflogEquals( reflog(A, B, new PersonIdent(diskRepo), "a reflog: fast-forward"), @@ -536,6 +664,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { @Test public void reflogCustomIdent() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; List<ReceiveCommand> cmds = Arrays.asList( new ReceiveCommand(A, B, "refs/heads/master", UPDATE), @@ -546,7 +675,8 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { .setRefLogIdent(ident)); assertResults(cmds, OK, OK); - assertEquals(batchesRefUpdates() ? 2 : 3, refsChangedEvents); + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertRefs("refs/heads/master", B, "refs/heads/branch", B); assertReflogEquals(reflog(A, B, ident, "a reflog"), getLastReflog("refs/heads/master"), true); @@ -558,6 +688,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { public void reflogDelete() throws IOException { writeRef("refs/heads/master", A); writeRef("refs/heads/branch", A); + int initialRefsChangedEvents = refsChangedEvents; assertEquals(2, getLastReflogs("refs/heads/master", "refs/heads/branch") .size()); @@ -568,7 +699,8 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, OK, OK); assertRefs("refs/heads/branch", B); - assertEquals(batchesRefUpdates() ? 3 : 4, refsChangedEvents); + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); if (useReftable) { // reftable retains reflog entries for deleted branches. assertReflogEquals( @@ -584,6 +716,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { @Test public void reflogFileDirectoryConflict() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; List<ReceiveCommand> cmds = Arrays.asList( new ReceiveCommand(A, zeroId(), "refs/heads/master", DELETE), @@ -592,7 +725,8 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertResults(cmds, OK, OK); assertRefs("refs/heads/master/x", A); - assertEquals(batchesRefUpdates() ? 2 : 3, refsChangedEvents); + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); if (!useReftable) { // reftable retains reflog entries for deleted branches. assertNull(getLastReflog("refs/heads/master")); @@ -605,6 +739,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { @Test public void reflogOnLockFailure() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; Map<String, ReflogEntry> oldLogs = getLastReflogs("refs/heads/master", "refs/heads/branch"); @@ -616,12 +751,12 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { if (atomic) { assertResults(cmds, TRANSACTION_ABORTED, LOCK_FAILURE); - assertEquals(1, refsChangedEvents); + assertEquals(initialRefsChangedEvents, refsChangedEvents); assertReflogUnchanged(oldLogs, "refs/heads/master"); assertReflogUnchanged(oldLogs, "refs/heads/branch"); } else { assertResults(cmds, OK, LOCK_FAILURE); - assertEquals(2, refsChangedEvents); + assertEquals(initialRefsChangedEvents + 1, refsChangedEvents); assertReflogEquals( reflog(A, B, new PersonIdent(diskRepo), "a reflog"), getLastReflog("refs/heads/master")); @@ -632,6 +767,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { @Test public void overrideRefLogMessage() throws Exception { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; List<ReceiveCommand> cmds = Arrays.asList( new ReceiveCommand(A, B, "refs/heads/master", UPDATE), @@ -642,7 +778,8 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { .setRefLogMessage("a reflog", true)); assertResults(cmds, OK, OK); - assertEquals(batchesRefUpdates() ? 2 : 3, refsChangedEvents); + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertReflogEquals(reflog(A, B, ident, "custom log"), getLastReflog("refs/heads/master"), true); assertReflogEquals(reflog(zeroId(), B, ident, "a reflog: created"), @@ -652,6 +789,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { @Test public void overrideDisableRefLog() throws Exception { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; Map<String, ReflogEntry> oldLogs = getLastReflogs("refs/heads/master", "refs/heads/branch"); @@ -663,7 +801,8 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { execute(newBatchUpdate(cmds).setRefLogMessage("a reflog", true)); assertResults(cmds, OK, OK); - assertEquals(batchesRefUpdates() ? 2 : 3, refsChangedEvents); + assertEquals(batchesRefUpdates() ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertReflogUnchanged(oldLogs, "refs/heads/master"); assertReflogEquals( reflog(zeroId(), B, new PersonIdent(diskRepo), @@ -674,6 +813,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { @Test public void refLogNotWrittenWithoutConfigOption() throws Exception { assumeFalse(useReftable); + setLogAllRefUpdates(false); writeRef("refs/heads/master", A); @@ -694,6 +834,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { @Test public void forceRefLogInUpdate() throws Exception { assumeFalse(useReftable); + setLogAllRefUpdates(false); writeRef("refs/heads/master", A); assertTrue(getLastReflogs("refs/heads/master", "refs/heads/branch") @@ -716,6 +857,7 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { @Test public void forceRefLogInCommand() throws Exception { assumeFalse(useReftable); + setLogAllRefUpdates(false); writeRef("refs/heads/master", A); @@ -756,13 +898,11 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { if (atomic) { assertResults(cmds, LOCK_FAILURE, TRANSACTION_ABORTED); assertRefs("refs/heads/master", A); - assertEquals(1, refsChangedEvents); } else { // Only operates on loose refs, doesn't care that packed-refs is // locked. assertResults(cmds, OK, OK); assertRefs("refs/heads/master", B, "refs/heads/branch", B); - assertEquals(3, refsChangedEvents); } } finally { myLock.unlock(); @@ -770,6 +910,28 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { } @Test + public void packedRefsLockFailureRefsChangedEvents() throws Exception { + assumeFalse(useReftable); + + writeLooseRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(A, B, "refs/heads/master", UPDATE), + new ReceiveCommand(zeroId(), B, "refs/heads/branch", CREATE)); + + LockFile myLock = refdir.lockPackedRefs(); + try { + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true)); + + assertEquals(atomic ? initialRefsChangedEvents + : initialRefsChangedEvents + 2, refsChangedEvents); + } finally { + myLock.unlock(); + } + } + + @Test public void oneRefLockFailure() throws Exception { assumeFalse(useReftable); @@ -790,11 +952,9 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { if (atomic) { assertResults(cmds, TRANSACTION_ABORTED, LOCK_FAILURE); assertRefs("refs/heads/master", A); - assertEquals(1, refsChangedEvents); } else { assertResults(cmds, OK, LOCK_FAILURE); assertRefs("refs/heads/branch", B, "refs/heads/master", A); - assertEquals(2, refsChangedEvents); } } finally { myLock.unlock(); @@ -802,8 +962,32 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { } @Test + public void oneRefLockFailureRefsChangedEvents() throws Exception { + assumeFalse(useReftable); + + writeLooseRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(zeroId(), B, "refs/heads/branch", CREATE), + new ReceiveCommand(A, B, "refs/heads/master", UPDATE)); + + LockFile myLock = new LockFile(refdir.fileFor("refs/heads/master")); + assertTrue(myLock.lock()); + try { + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true)); + + assertEquals(atomic ? initialRefsChangedEvents + : initialRefsChangedEvents + 1, refsChangedEvents); + } finally { + myLock.unlock(); + } + } + + @Test public void singleRefUpdateDoesNotRequirePackedRefsLock() throws Exception { assumeFalse(useReftable); + writeLooseRef("refs/heads/master", A); List<ReceiveCommand> cmds = Arrays @@ -815,7 +999,6 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { assertFalse(getLockFile("refs/heads/master").exists()); assertResults(cmds, OK); - assertEquals(2, refsChangedEvents); assertRefs("refs/heads/master", B); } finally { myLock.unlock(); @@ -823,6 +1006,27 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { } @Test + public void singleRefUpdateDoesNotRequirePackedRefsLockRefsChangedEvents() + throws Exception { + assumeFalse(useReftable); + + writeLooseRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays + .asList(new ReceiveCommand(A, B, "refs/heads/master", UPDATE)); + + LockFile myLock = refdir.lockPackedRefs(); + try { + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true)); + + assertEquals(initialRefsChangedEvents + 1, refsChangedEvents); + } finally { + myLock.unlock(); + } + } + + @Test public void atomicUpdateRespectsInProcessLock() throws Exception { assumeTrue(atomic); assumeFalse(useReftable); @@ -871,10 +1075,56 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { } assertResults(cmds, OK, OK); - assertEquals(2, refsChangedEvents); assertRefs("refs/heads/master", B, "refs/heads/branch", B); } + @Test + public void atomicUpdateRespectsInProcessLockRefsChangedEvents() + throws Exception { + assumeTrue(atomic); + assumeFalse(useReftable); + + writeLooseRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; + + List<ReceiveCommand> cmds = Arrays.asList( + new ReceiveCommand(A, B, "refs/heads/master", UPDATE), + new ReceiveCommand(zeroId(), B, "refs/heads/branch", CREATE)); + + Thread t = new Thread(() -> { + try { + execute(newBatchUpdate(cmds).setAllowNonFastForwards(true)); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + + ReentrantLock l = refdir.inProcessPackedRefsLock; + l.lock(); + try { + t.start(); + long timeoutSecs = 10; + + // Hold onto the lock until we observe the worker thread has + // attempted to + // acquire it. + while (l.getQueueLength() == 0) { + Thread.sleep(3); + } + + // Once we unlock, the worker thread should finish the update + // promptly. + l.unlock(); + t.join(SECONDS.toMillis(timeoutSecs)); + } finally { + if (l.isHeldByCurrentThread()) { + l.unlock(); + } + } + + assertEquals(initialRefsChangedEvents + 1, refsChangedEvents); + } + private void setLogAllRefUpdates(boolean enable) throws Exception { StoredConfig cfg = diskRepo.getConfig(); cfg.load(); @@ -888,6 +1138,11 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { writeRef(name, id); } else { write(new File(diskRepo.getDirectory(), name), id.name() + "\n"); + // force the refs-changed event to be fired for the loose ref that + // was created. We do this to get the events fired during the test + // 'setup' out of the way and this allows us to now accurately + // assert only for the new events fired during the BatchRefUpdate. + refdir.exactRef(name); } } @@ -990,11 +1245,11 @@ public class BatchRefUpdateTest extends LocalDiskRepositoryTestCase { } enum Result { - OK(ReceiveCommand.Result.OK), LOCK_FAILURE( - ReceiveCommand.Result.LOCK_FAILURE), REJECTED_NONFASTFORWARD( - ReceiveCommand.Result.REJECTED_NONFASTFORWARD), REJECTED_MISSING_OBJECT( - ReceiveCommand.Result.REJECTED_MISSING_OBJECT), TRANSACTION_ABORTED( - ReceiveCommand::isTransactionAborted); + OK(ReceiveCommand.Result.OK), + LOCK_FAILURE(ReceiveCommand.Result.LOCK_FAILURE), + REJECTED_NONFASTFORWARD(ReceiveCommand.Result.REJECTED_NONFASTFORWARD), + REJECTED_MISSING_OBJECT(ReceiveCommand.Result.REJECTED_MISSING_OBJECT), + TRANSACTION_ABORTED(ReceiveCommand::isTransactionAborted); @SuppressWarnings("ImmutableEnumChecker") final Predicate<? super ReceiveCommand> p; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java index 0cec5a4832..9476d35475 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java @@ -277,47 +277,18 @@ public class RefDirectory extends RefDatabase { /** {@inheritDoc} */ @Override public boolean isNameConflicting(String name) throws IOException { - RefList<Ref> packed = getPackedRefs(); - RefList<LooseRef> loose = getLooseRefs(); - // Cannot be nested within an existing reference. int lastSlash = name.lastIndexOf('/'); while (0 < lastSlash) { String needle = name.substring(0, lastSlash); - if (loose.contains(needle) || packed.contains(needle)) + if (exactRef(needle) != null) { return true; + } lastSlash = name.lastIndexOf('/', lastSlash - 1); } // Cannot be the container of an existing reference. - String prefix = name + '/'; - int idx; - - idx = -(packed.find(prefix) + 1); - if (idx < packed.size() && packed.get(idx).getName().startsWith(prefix)) - return true; - - idx = -(loose.find(prefix) + 1); - if (idx < loose.size() && loose.get(idx).getName().startsWith(prefix)) - return true; - - return false; - } - - private RefList<LooseRef> getLooseRefs() { - final RefList<LooseRef> oldLoose = looseRefs.get(); - - LooseScanner scan = new LooseScanner(oldLoose); - scan.scan(ALL); - - RefList<LooseRef> loose; - if (scan.newLoose != null) { - loose = scan.newLoose.toRefList(); - if (looseRefs.compareAndSet(oldLoose, loose)) - modCnt.incrementAndGet(); - } else - loose = oldLoose; - return loose; + return !getRefsByPrefix(name + '/').isEmpty(); } @Nullable diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java index 925b6beadb..46d2ab3397 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BatchRefUpdate.java @@ -46,7 +46,6 @@ package org.eclipse.jgit.lib; import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED; import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_REASON; -import static java.util.stream.Collectors.toCollection; import java.io.IOException; import java.text.MessageFormat; @@ -62,7 +61,6 @@ import java.util.concurrent.TimeoutException; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.internal.JGitText; -import org.eclipse.jgit.lib.RefUpdate.Result; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.PushCertificate; import org.eclipse.jgit.transport.ReceiveCommand; @@ -528,42 +526,24 @@ public class BatchRefUpdate { } } if (!commands2.isEmpty()) { - // What part of the name space is already taken - Collection<String> takenNames = refdb.getRefs().stream() - .map(Ref::getName) - .collect(toCollection(HashSet::new)); - Collection<String> takenPrefixes = getTakenPrefixes(takenNames); - - // Now to the update that may require more room in the name space + // Perform updates that may require more room in the name space for (ReceiveCommand cmd : commands2) { try { if (cmd.getResult() == NOT_ATTEMPTED) { cmd.updateType(walk); RefUpdate ru = newUpdate(cmd); - SWITCH: switch (cmd.getType()) { - case DELETE: - // Performed in the first phase - break; - case UPDATE: - case UPDATE_NONFASTFORWARD: - RefUpdate ruu = newUpdate(cmd); - cmd.setResult(ruu.update(walk)); - break; - case CREATE: - for (String prefix : getPrefixes(cmd.getRefName())) { - if (takenNames.contains(prefix)) { - cmd.setResult(Result.LOCK_FAILURE); - break SWITCH; - } - } - if (takenPrefixes.contains(cmd.getRefName())) { - cmd.setResult(Result.LOCK_FAILURE); - break SWITCH; - } - ru.setCheckConflicting(false); - takenPrefixes.addAll(getPrefixes(cmd.getRefName())); - takenNames.add(cmd.getRefName()); - cmd.setResult(ru.update(walk)); + switch (cmd.getType()) { + case DELETE: + // Performed in the first phase + break; + case UPDATE: + case UPDATE_NONFASTFORWARD: + RefUpdate ruu = newUpdate(cmd); + cmd.setResult(ruu.update(walk)); + break; + case CREATE: + cmd.setResult(ru.update(walk)); + break; } } } catch (IOException err) { @@ -635,14 +615,6 @@ public class BatchRefUpdate { execute(walk, monitor, null); } - private static Collection<String> getTakenPrefixes(Collection<String> names) { - Collection<String> ref = new HashSet<>(); - for (String name : names) { - addPrefixesTo(name, ref); - } - return ref; - } - /** * Get all path prefixes of a ref name. * |