From d79cadb3cf4bb26c01bc5d4795b05858bb25ef42 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 11 Mar 2015 15:21:48 -0700 Subject: [PATCH] TestRepository: Add a reset method to move HEAD around This flushed out a number of bugs in the way DfsRefUpdate, or at least the InMemoryRepository implementation, processes symrefs. These have been fixed, to an extent, in InMemoryRepository, but other implementations may still suffer from these bugs. Change-Id: Ifd12115a0060b9ff45a88d305b72f91ca0472f9a --- .../eclipse/jgit/junit/TestRepository.java | 55 ++++++++++++ .../jgit/junit/TestRepositoryTest.java | 83 +++++++++++++++++++ .../storage/dfs/InMemoryRepository.java | 57 +++++++++---- 3 files changed, 180 insertions(+), 15 deletions(-) diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java index 8549118ab..479ac7e5c 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java @@ -478,6 +478,61 @@ public class TestRepository { } } + /** + * Soft-reset HEAD to a detached state. + *

+ * @param id + * ID of detached head. + * @throws Exception + * @see #reset(String) + */ + public void reset(AnyObjectId id) throws Exception { + RefUpdate ru = db.updateRef(Constants.HEAD, true); + ru.setNewObjectId(id); + RefUpdate.Result result = ru.forceUpdate(); + switch (result) { + case FAST_FORWARD: + case FORCED: + case NEW: + case NO_CHANGE: + break; + default: + throw new IOException(String.format( + "Checkout \"%s\" failed: %s", id.name(), result)); + } + } + + /** + * Soft-reset HEAD to a different commit. + *

+ * This is equivalent to {@code git reset --soft} in that it modifies HEAD but + * not the index or the working tree of a non-bare repository. + * + * @param name + * revision string; either an existing ref name, or something that + * can be parsed to an object ID. + * @throws Exception + */ + public void reset(String name) throws Exception { + RefUpdate.Result result; + ObjectId id = db.resolve(name); + if (id == null) + throw new IOException("Not a revision: " + name); + RefUpdate ru = db.updateRef(Constants.HEAD, false); + ru.setNewObjectId(id); + result = ru.forceUpdate(); + switch (result) { + case FAST_FORWARD: + case FORCED: + case NEW: + case NO_CHANGE: + break; + default: + throw new IOException(String.format( + "Checkout \"%s\" failed: %s", name, result)); + } + } + /** * Update the dumb client server info files. * 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 d2ad0d9a6..c14b32e6c 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 @@ -44,12 +44,16 @@ package org.eclipse.jgit.junit; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.regex.Pattern; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; import org.junit.After; @@ -58,12 +62,14 @@ import org.junit.Test; public class TestRepositoryTest { private TestRepository tr; + private InMemoryRepository repo; private RevWalk rw; @Before public void setUp() throws Exception { tr = new TestRepository<>(new InMemoryRepository( new DfsRepositoryDescription("test"))); + repo = tr.getRepository(); rw = tr.getRevWalk(); } @@ -85,4 +91,81 @@ public class TestRepositoryTest { assertEquals("\n\nChange-Id: I0000000000000000000000000000000000000000\n", c2.getFullMessage()); } + + @Test + public void resetFromSymref() throws Exception { + repo.updateRef("HEAD").link("refs/heads/master"); + Ref head = repo.getRef("HEAD"); + RevCommit master = tr.branch("master").commit().create(); + RevCommit branch = tr.branch("branch").commit().create(); + RevCommit detached = tr.commit().create(); + + assertTrue(head.isSymbolic()); + assertEquals("refs/heads/master", head.getTarget().getName()); + assertEquals(master, repo.getRef("refs/heads/master").getObjectId()); + assertEquals(branch, repo.getRef("refs/heads/branch").getObjectId()); + + // Reset to branches preserves symref. + tr.reset("master"); + head = repo.getRef("HEAD"); + assertEquals(master, head.getObjectId()); + assertTrue(head.isSymbolic()); + assertEquals("refs/heads/master", head.getTarget().getName()); + + tr.reset("branch"); + head = repo.getRef("HEAD"); + assertEquals(branch, head.getObjectId()); + assertTrue(head.isSymbolic()); + assertEquals("refs/heads/master", head.getTarget().getName()); + ObjectId lastHeadBeforeDetach = head.getObjectId().copy(); + + // Reset to a SHA-1 detaches. + tr.reset(detached); + head = repo.getRef("HEAD"); + assertEquals(detached, head.getObjectId()); + assertFalse(head.isSymbolic()); + + tr.reset(detached.name()); + head = repo.getRef("HEAD"); + assertEquals(detached, head.getObjectId()); + assertFalse(head.isSymbolic()); + + // Reset back to a branch remains detached. + tr.reset("master"); + head = repo.getRef("HEAD"); + assertEquals(lastHeadBeforeDetach, head.getObjectId()); + assertFalse(head.isSymbolic()); + } + + @Test + public void resetFromDetachedHead() throws Exception { + Ref head = repo.getRef("HEAD"); + RevCommit master = tr.branch("master").commit().create(); + RevCommit branch = tr.branch("branch").commit().create(); + RevCommit detached = tr.commit().create(); + + assertNull(head); + assertEquals(master, repo.getRef("refs/heads/master").getObjectId()); + assertEquals(branch, repo.getRef("refs/heads/branch").getObjectId()); + + tr.reset("master"); + head = repo.getRef("HEAD"); + assertEquals(master, head.getObjectId()); + assertFalse(head.isSymbolic()); + + tr.reset("branch"); + head = repo.getRef("HEAD"); + assertEquals(branch, head.getObjectId()); + assertFalse(head.isSymbolic()); + + tr.reset(detached); + head = repo.getRef("HEAD"); + assertEquals(detached, head.getObjectId()); + assertFalse(head.isSymbolic()); + + tr.reset(detached.name()); + head = repo.getRef("HEAD"); + assertEquals(detached, head.getObjectId()); + assertFalse(head.isSymbolic()); + } } 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 18fedf8b9..965aa8d8b 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 @@ -9,14 +9,17 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectIdRef; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Ref.Storage; +import org.eclipse.jgit.lib.SymbolicRef; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.util.RefList; @@ -245,24 +248,47 @@ public class InMemoryRepository extends DfsRepository { throws IOException { ObjectId id = newRef.getObjectId(); if (id != null) { - RevWalk rw = new RevWalk(getRepository()); - try { + try (RevWalk rw = new RevWalk(getRepository())) { // Validate that the target exists in a new RevWalk, as the RevWalk // from the RefUpdate might be reading back unflushed objects. rw.parseAny(id); - } finally { - rw.release(); } } String name = newRef.getName(); - if (oldRef == null || oldRef.getStorage() == Storage.NEW) + if (oldRef == null) + return refs.putIfAbsent(name, newRef) == null; + + synchronized (refs) { + 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)) + return refs.replace(name, cur, newRef); + } + } + + if (oldRef.getStorage() == Storage.NEW) return refs.putIfAbsent(name, newRef) == null; - Ref cur = refs.get(name); - if (cur != null && eq(cur, oldRef)) - return refs.replace(name, cur, newRef); - else - return false; + return false; } @Override @@ -276,11 +302,12 @@ public class InMemoryRepository extends DfsRepository { } private boolean eq(Ref a, Ref b) { - if (a.getObjectId() == null && b.getObjectId() == null) - return true; - if (a.getObjectId() != null) - return a.getObjectId().equals(b.getObjectId()); - return false; + 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()); } } }