diff --git a/.bazelversion b/.bazelversion index 3eefcb9dd..227cea215 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -1.0.0 +2.0.0 diff --git a/WORKSPACE b/WORKSPACE index dc89a6c54..8e6b8642b 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -15,7 +15,7 @@ versions.check(minimum_bazel_version = "0.29.0") load("//tools:bazlets.bzl", "load_bazlets") -load_bazlets(commit = "09a035e98077dce549d5f6a7472d06c4b8f792d2") +load_bazlets(commit = "f53f51fb660552d0581aa0ba52c3836ed63d56a3") load( "@com_googlesource_gerrit_bazlets//tools:maven_jar.bzl", 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 2ac4a846e..d0e3943d1 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 @@ -205,16 +205,32 @@ public void simpleNoForce() throws IOException { 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 { + writeLooseRef("refs/heads/master", A); + writeLooseRef("refs/heads/masters", B); + refdir.exactRef("refs/heads/master"); + refdir.exactRef("refs/heads/masters"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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 { writeLooseRef("refs/heads/master", A); @@ -229,7 +245,24 @@ public void simpleForce() throws IOException { assertRefs( "refs/heads/master", B, "refs/heads/masters", A); - assertEquals(atomic ? 2 : 3, refsChangedEvents); + } + + @Test + public void simpleForceRefsChangedEvents() throws IOException { + writeLooseRef("refs/heads/master", A); + writeLooseRef("refs/heads/masters", B); + refdir.exactRef("refs/heads/master"); + refdir.exactRef("refs/heads/masters"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); } @Test @@ -251,7 +284,28 @@ public boolean isMergedInto(RevCommit base, RevCommit tip) { assertResults(cmds, OK); assertRefs("refs/heads/master", A); - assertEquals(2, refsChangedEvents); + } + + @Test + public void nonFastForwardDoesNotDoExpensiveMergeCheckRefsChangedEvents() + throws IOException { + writeLooseRef("refs/heads/master", B); + refdir.exactRef("refs/heads/master"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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 @@ -273,7 +327,6 @@ public void fileDirectoryConflict() throws IOException { 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. @@ -281,10 +334,27 @@ public void fileDirectoryConflict() throws IOException { assertRefs( "refs/heads/master", B, "refs/heads/masters", B); - assertEquals(2, refsChangedEvents); } } + @Test + public void fileDirectoryConflictRefsChangedEvents() throws IOException { + writeLooseRef("refs/heads/master", A); + writeLooseRef("refs/heads/masters", B); + refdir.exactRef("refs/heads/master"); + refdir.exactRef("refs/heads/masters"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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 { writeLooseRef("refs/heads/master", A); @@ -300,15 +370,24 @@ public void conflictThanksToDelete() throws IOException { assertRefs( "refs/heads/master", B, "refs/heads/masters/x", A); - if (atomic) { - assertEquals(2, refsChangedEvents); - } else { - // 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 { + writeLooseRef("refs/heads/master", A); + writeLooseRef("refs/heads/masters", B); + refdir.exactRef("refs/heads/master"); + refdir.exactRef("refs/heads/masters"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 3, refsChangedEvents); } @Test @@ -325,16 +404,31 @@ public void updateToMissingObject() throws IOException { 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); + refdir.exactRef("refs/heads/master"); + int initialRefsChangedEvents = refsChangedEvents; + + ObjectId bad = ObjectId + .fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); + List 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); @@ -349,14 +443,29 @@ public void addMissingObject() throws IOException { 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); + refdir.exactRef("refs/heads/master"); + int initialRefsChangedEvents = refsChangedEvents; + + ObjectId bad = ObjectId + .fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); + List 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 cmds = Arrays.asList( @@ -387,16 +496,29 @@ public void oneRefWrongOldValue() throws IOException { 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); + refdir.exactRef("refs/heads/master"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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); @@ -409,17 +531,31 @@ public void nonExistentRef() throws IOException { 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); + refdir.exactRef("refs/heads/master"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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 oldLogs = getLastReflogs("refs/heads/master", "refs/heads/branch"); @@ -434,7 +570,8 @@ public void noRefLog() throws IOException { assertRefs( "refs/heads/master", B, "refs/heads/branch", B); - assertEquals(atomic ? 2 : 3, refsChangedEvents); + assertEquals(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertReflogUnchanged(oldLogs, "refs/heads/master"); assertReflogUnchanged(oldLogs, "refs/heads/branch"); } @@ -443,6 +580,7 @@ public void noRefLog() throws IOException { public void reflogDefaultIdent() throws IOException { writeRef("refs/heads/master", A); writeRef("refs/heads/branch2", A); + int initialRefsChangedEvents = refsChangedEvents; Map oldLogs = getLastReflogs( "refs/heads/master", "refs/heads/branch1", "refs/heads/branch2"); @@ -459,7 +597,8 @@ public void reflogDefaultIdent() throws IOException { "refs/heads/master", B, "refs/heads/branch1", B, "refs/heads/branch2", A); - assertEquals(atomic ? 3 : 4, refsChangedEvents); + assertEquals(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertReflogEquals( reflog(A, B, new PersonIdent(diskRepo), "a reflog"), getLastReflog("refs/heads/master")); @@ -473,6 +612,7 @@ public void reflogDefaultIdent() throws IOException { public void reflogAppendStatusNoMessage() throws IOException { writeRef("refs/heads/master", A); writeRef("refs/heads/branch1", B); + int initialRefsChangedEvents = refsChangedEvents; List cmds = Arrays.asList( new ReceiveCommand(A, B, "refs/heads/master", UPDATE), @@ -488,7 +628,9 @@ public void reflogAppendStatusNoMessage() throws IOException { "refs/heads/master", B, "refs/heads/branch1", A, "refs/heads/branch2", A); - assertEquals(atomic ? 3 : 5, refsChangedEvents); + assertEquals(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 3, + refsChangedEvents); assertReflogEquals( // Always forced; setAllowNonFastForwards(true) bypasses the check. reflog(A, B, new PersonIdent(diskRepo), "forced-update"), @@ -504,6 +646,7 @@ public void reflogAppendStatusNoMessage() throws IOException { @Test public void reflogAppendStatusFastForward() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; List cmds = Arrays.asList( new ReceiveCommand(A, B, "refs/heads/master", UPDATE)); @@ -511,7 +654,7 @@ public void reflogAppendStatusFastForward() throws IOException { 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")); @@ -520,6 +663,7 @@ public void reflogAppendStatusFastForward() throws IOException { @Test public void reflogAppendStatusWithMessage() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; List cmds = Arrays.asList( new ReceiveCommand(A, B, "refs/heads/master", UPDATE), @@ -530,7 +674,8 @@ public void reflogAppendStatusWithMessage() throws IOException { assertRefs( "refs/heads/master", B, "refs/heads/branch", A); - assertEquals(atomic ? 2 : 3, refsChangedEvents); + assertEquals(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertReflogEquals( reflog(A, B, new PersonIdent(diskRepo), "a reflog: fast-forward"), getLastReflog("refs/heads/master")); @@ -542,6 +687,7 @@ public void reflogAppendStatusWithMessage() throws IOException { @Test public void reflogCustomIdent() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; List cmds = Arrays.asList( new ReceiveCommand(A, B, "refs/heads/master", UPDATE), @@ -553,7 +699,8 @@ public void reflogCustomIdent() throws IOException { .setRefLogIdent(ident)); assertResults(cmds, OK, OK); - assertEquals(atomic ? 2 : 3, refsChangedEvents); + assertEquals(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertRefs( "refs/heads/master", B, "refs/heads/branch", B); @@ -571,6 +718,8 @@ public void reflogCustomIdent() throws IOException { 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()); @@ -581,7 +730,8 @@ public void reflogDelete() throws IOException { assertResults(cmds, OK, OK); assertRefs("refs/heads/branch", B); - assertEquals(atomic ? 3 : 4, refsChangedEvents); + assertEquals(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertNull(getLastReflog("refs/heads/master")); assertReflogEquals( reflog(A, B, new PersonIdent(diskRepo), "a reflog"), @@ -591,6 +741,7 @@ public void reflogDelete() throws IOException { @Test public void reflogFileDirectoryConflict() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; List cmds = Arrays.asList( new ReceiveCommand(A, zeroId(), "refs/heads/master", DELETE), @@ -599,7 +750,8 @@ public void reflogFileDirectoryConflict() throws IOException { assertResults(cmds, OK, OK); assertRefs("refs/heads/master/x", A); - assertEquals(atomic ? 2 : 3, refsChangedEvents); + assertEquals(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertNull(getLastReflog("refs/heads/master")); assertReflogEquals( reflog(zeroId(), A, new PersonIdent(diskRepo), "a reflog"), @@ -609,6 +761,7 @@ public void reflogFileDirectoryConflict() throws IOException { @Test public void reflogOnLockFailure() throws IOException { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; Map oldLogs = getLastReflogs("refs/heads/master", "refs/heads/branch"); @@ -620,12 +773,12 @@ public void reflogOnLockFailure() throws IOException { 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")); @@ -636,6 +789,7 @@ public void reflogOnLockFailure() throws IOException { @Test public void overrideRefLogMessage() throws Exception { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; List cmds = Arrays.asList( new ReceiveCommand(A, B, "refs/heads/master", UPDATE), @@ -648,7 +802,8 @@ public void overrideRefLogMessage() throws Exception { .setRefLogMessage("a reflog", true)); assertResults(cmds, OK, OK); - assertEquals(atomic ? 2 : 3, refsChangedEvents); + assertEquals(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertReflogEquals( reflog(A, B, ident, "custom log"), getLastReflog("refs/heads/master"), @@ -662,6 +817,7 @@ public void overrideRefLogMessage() throws Exception { @Test public void overrideDisableRefLog() throws Exception { writeRef("refs/heads/master", A); + int initialRefsChangedEvents = refsChangedEvents; Map oldLogs = getLastReflogs("refs/heads/master", "refs/heads/branch"); @@ -673,7 +829,8 @@ public void overrideDisableRefLog() throws Exception { execute(newBatchUpdate(cmds).setRefLogMessage("a reflog", true)); assertResults(cmds, OK, OK); - assertEquals(atomic ? 2 : 3, refsChangedEvents); + assertEquals(atomic ? initialRefsChangedEvents + 1 + : initialRefsChangedEvents + 2, refsChangedEvents); assertReflogUnchanged(oldLogs, "refs/heads/master"); assertReflogEquals( reflog(zeroId(), B, new PersonIdent(diskRepo), "a reflog: created"), @@ -763,20 +920,41 @@ public void packedRefsLockFailure() throws Exception { 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(); } } + @Test + public void packedRefsLockFailureRefsChangedEvents() throws Exception { + writeLooseRef("refs/heads/master", A); + refdir.exactRef("refs/heads/master"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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 { writeLooseRef("refs/heads/master", A); @@ -796,19 +974,41 @@ public void oneRefLockFailure() throws Exception { 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(); } } + @Test + public void oneRefLockFailureRefsChangedEvents() throws Exception { + writeLooseRef("refs/heads/master", A); + refdir.exactRef("refs/heads/master"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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 { writeLooseRef("refs/heads/master", A); @@ -822,13 +1022,32 @@ public void singleRefUpdateDoesNotRequirePackedRefsLock() throws Exception { assertFalse(getLockFile("refs/heads/master").exists()); assertResults(cmds, OK); - assertEquals(2, refsChangedEvents); assertRefs("refs/heads/master", B); } finally { myLock.unlock(); } } + @Test + public void singleRefUpdateDoesNotRequirePackedRefsLockRefsChangedEvents() + throws Exception { + writeLooseRef("refs/heads/master", A); + refdir.exactRef("refs/heads/master"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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); @@ -881,6 +1100,53 @@ public void atomicUpdateRespectsInProcessLock() throws Exception { "refs/heads/branch", B); } + @Test + public void atomicUpdateRespectsInProcessLockRefsChangedEvents() + throws Exception { + assumeTrue(atomic); + + writeLooseRef("refs/heads/master", A); + refdir.exactRef("refs/heads/master"); + int initialRefsChangedEvents = refsChangedEvents; + + List 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(); 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 a4729bba4..6153ac5e1 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 void refresh() { /** {@inheritDoc} */ @Override public boolean isNameConflicting(String name) throws IOException { - RefList packed = getPackedRefs(); - RefList 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 getLooseRefs() { - final RefList oldLoose = looseRefs.get(); - - LooseScanner scan = new LooseScanner(oldLoose); - scan.scan(ALL); - - RefList 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 925b6bead..46d2ab339 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 @@ 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 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 void execute(RevWalk walk, ProgressMonitor monitor, } } if (!commands2.isEmpty()) { - // What part of the name space is already taken - Collection takenNames = refdb.getRefs().stream() - .map(Ref::getName) - .collect(toCollection(HashSet::new)); - Collection 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 void execute(RevWalk walk, ProgressMonitor monitor) execute(walk, monitor, null); } - private static Collection getTakenPrefixes(Collection names) { - Collection ref = new HashSet<>(); - for (String name : names) { - addPrefixesTo(name, ref); - } - return ref; - } - /** * Get all path prefixes of a ref name. *