diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/junit/TestRepositoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/junit/TestRepositoryTest.java index 8b54dabce..b7027f327 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/junit/TestRepositoryTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/junit/TestRepositoryTest.java @@ -62,6 +62,7 @@ import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevObject; @@ -374,6 +375,44 @@ public void cherryPickWithIdenticalContents() throws Exception { assertEquals(head, repo.exactRef("HEAD").getObjectId()); } + @Test + public void reattachToMaster_Race() throws Exception { + RevCommit commit = tr.branch("master").commit().create(); + tr.branch("master").update(commit); + tr.branch("other").update(commit); + repo.updateRef("HEAD").link("refs/heads/master"); + + // Create a detached HEAD that is not an . + tr.reset(commit); + Ref head = repo.exactRef("HEAD"); + assertEquals(commit, head.getObjectId()); + assertFalse(head.isSymbolic()); + + // Try to reattach to master. + RefUpdate refUpdate = repo.updateRef("HEAD"); + + // Make a change during reattachment. + repo.updateRef("HEAD").link("refs/heads/other"); + + assertEquals( + RefUpdate.Result.LOCK_FAILURE, refUpdate.link("refs/heads/master")); + } + + @Test + public void nonRacingChange() throws Exception { + tr.branch("master").update(tr.branch("master").commit().create()); + tr.branch("other").update(tr.branch("other").commit().create()); + repo.updateRef("HEAD").link("refs/heads/master"); + + // Try to update HEAD. + RefUpdate refUpdate = repo.updateRef("HEAD"); + + // Proceed a master. This should not affect changing HEAD. + tr.branch("master").update(tr.branch("master").commit().create()); + + assertEquals(RefUpdate.Result.FORCED, refUpdate.link("refs/heads/other")); + } + private String blobAsString(AnyObjectId treeish, String path) throws Exception { RevObject obj = tr.get(rw.parseTree(treeish), path); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsRefDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsRefDatabase.java index 5611d23be..4ddcec167 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsRefDatabase.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsRefDatabase.java @@ -311,6 +311,15 @@ private RefCache read() throws IOException { /** * Compare a reference, and put if it matches. + *

+ * Two reference match if and only if they satisfy the following: + * + *

* * @param oldRef * old value to compare to. If the reference is expected to not diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java index 2e1c90d3c..6f390a4b3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java @@ -24,7 +24,6 @@ import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Ref.Storage; import org.eclipse.jgit.lib.RefDatabase; -import org.eclipse.jgit.lib.SymbolicRef; import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevTag; import org.eclipse.jgit.revwalk.RevWalk; @@ -401,27 +400,8 @@ protected boolean compareAndPut(Ref oldRef, Ref newRef) return refs.putIfAbsent(name, newRef) == null; Ref cur = refs.get(name); - Ref toCompare = cur; - if (toCompare != null) { - if (toCompare.isSymbolic()) { - // Arm's-length dereference symrefs before the compare, since - // DfsRefUpdate#doLink(String) stores them undereferenced. - Ref leaf = toCompare.getLeaf(); - if (leaf.getObjectId() == null) { - leaf = refs.get(leaf.getName()); - if (leaf.isSymbolic()) - // Not supported at the moment. - throw new IllegalArgumentException(); - toCompare = new SymbolicRef( - name, - new ObjectIdRef.Unpeeled( - Storage.NEW, - leaf.getName(), - leaf.getObjectId())); - } else - toCompare = toCompare.getLeaf(); - } - if (eq(toCompare, oldRef)) + if (cur != null) { + if (eq(cur, oldRef)) return refs.replace(name, cur, newRef); } @@ -452,10 +432,12 @@ protected boolean compareAndRemove(Ref oldRef) throws IOException { private boolean eq(Ref a, Ref b) { if (!Objects.equals(a.getName(), b.getName())) return false; - // Compare leaf object IDs, since the oldRef passed into compareAndPut - // when detaching a symref is an ObjectIdRef. - return Objects.equals(a.getLeaf().getObjectId(), - b.getLeaf().getObjectId()); + if (a.isSymbolic() != b.isSymbolic()) + return false; + if (a.isSymbolic()) + return Objects.equals(a.getTarget().getName(), b.getTarget().getName()); + else + return Objects.equals(a.getObjectId(), b.getObjectId()); } } }