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

org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java

@@ -478,6 +478,61 @@ public class TestRepository<R extends Repository> {
 		}
 	}
 
+	/**
+	 * Soft-reset HEAD to a detached state.
+	 * <p>
+	 * @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.
+	 * <p>
+	 * 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.
 	 *

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<InMemoryRepository> 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());
+	}
 }

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 (RevWalk rw = new RevWalk(getRepository())) {
-				try {
 					// 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);
-			if (cur != null && eq(cur, oldRef))
+				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);
-			else
+				}
-				return false;
+			}
+
+			if (oldRef.getStorage() == Storage.NEW)
+				return refs.putIfAbsent(name, newRef) == null;
 
+			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)
+			if (!Objects.equals(a.getName(), b.getName()))
-				return true;
-			if (a.getObjectId() != null)
-				return a.getObjectId().equals(b.getObjectId());
 				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());
 		}
 	}
 }