Fix deleting symrefs

The RefDirectory implementation of doDelete never considered whether to
delete a symref or its leaf, because the detachingSymbolicRef bit was
never exposed from RefUpdate. The behavior was thus incorrectly to
always delete the symref, never the leaf.

There was no test for this behavior. The only thing that attempted to be
a test was testDeleteHeadInBareRepo, but this test was broken for
reasons unrelated to this bug. Specifically, it set the leaf to point to
a completely nonexistent object, and then asserted that deleting HEAD
resulted in NO_CHANGE. The only reason this test ever passed is because
of a quirk of updateImpl, which treats a missing object as the same as
null. This quirk aside, the test wasn't really testing the right thing.
Turn this into a real test by writing out a real object and pointing the
leaf at that.

Also, add a test for the detachingSymbolicRef case, i.e. deleting the
symref and leaving the leaf alone.

Change-Id: Ib96d2a35b4f99eba0734725486085fc6f9d78aa5

3 files changed, 78 insertions, 4 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java
index daef91d53c..e4e5d28f7d 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefUpdateTest.java
@@ -45,6 +45,7 @@
 
 package org.eclipse.jgit.internal.storage.file;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.eclipse.jgit.junit.Assert.assertEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -64,6 +65,7 @@ import java.util.Map.Entry;
 import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefRename;
@@ -240,14 +242,73 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
 	@Test
 	public void testDeleteHeadInBareRepo() throws IOException {
 		Repository bareRepo = createBareRepository();
+		String master = "refs/heads/master";
+		Ref head = bareRepo.exactRef(Constants.HEAD);
+		assertNotNull(head);
+		assertTrue(head.isSymbolic());
+		assertEquals(master, head.getLeaf().getName());
+		assertNull(head.getObjectId());
+		assertNull(bareRepo.exactRef(master));
+
+		ObjectId blobId;
+		try (ObjectInserter ins = bareRepo.newObjectInserter()) {
+			blobId = ins.insert(Constants.OBJ_BLOB, "contents".getBytes(UTF_8));
+			ins.flush();
+		}
+
+		// Create master via HEAD, so we delete it.
 		RefUpdate ref = bareRepo.updateRef(Constants.HEAD);
-		ref.setNewObjectId(ObjectId
-				.fromString("0123456789012345678901234567890123456789"));
-		// Create the HEAD ref so we can delete it.
+		ref.setNewObjectId(blobId);
 		assertEquals(Result.NEW, ref.update());
+
+		head = bareRepo.exactRef(Constants.HEAD);
+		assertTrue(head.isSymbolic());
+		assertEquals(master, head.getLeaf().getName());
+		assertEquals(blobId, head.getLeaf().getObjectId());
+		assertEquals(blobId, bareRepo.exactRef(master).getObjectId());
+
+		// Unlike in a non-bare repo, deleting the HEAD is allowed, and leaves HEAD
+		// back in a dangling state.
 		ref = bareRepo.updateRef(Constants.HEAD);
-		delete(bareRepo, ref, Result.NO_CHANGE, true, true);
+		ref.setExpectedOldObjectId(blobId);
+		ref.setForceUpdate(true);
+		delete(bareRepo, ref, Result.FORCED, true, true);
+
+		head = bareRepo.exactRef(Constants.HEAD);
+		assertNotNull(head);
+		assertTrue(head.isSymbolic());
+		assertEquals(master, head.getLeaf().getName());
+		assertNull(head.getObjectId());
+		assertNull(bareRepo.exactRef(master));
+	}
+
+	@Test
+	public void testDeleteSymref() throws IOException {
+		RefUpdate dst = updateRef("refs/heads/abc");
+		assertEquals(Result.NEW, dst.update());
+		ObjectId id = dst.getNewObjectId();
+
+		RefUpdate u = db.updateRef("refs/symref");
+		assertEquals(Result.NEW, u.link(dst.getName()));
+
+		Ref ref = db.exactRef(u.getName());
+		assertNotNull(ref);
+		assertTrue(ref.isSymbolic());
+		assertEquals(dst.getName(), ref.getLeaf().getName());
+		assertEquals(id, ref.getLeaf().getObjectId());
+
+		u = db.updateRef(u.getName());
+		u.setDetachingSymbolicRef();
+		u.setForceUpdate(true);
+		assertEquals(Result.FORCED, u.delete());
+
+		assertNull(db.exactRef(u.getName()));
+		ref = db.exactRef(dst.getName());
+		assertNotNull(ref);
+		assertFalse(ref.isSymbolic());
+		assertEquals(id, ref.getObjectId());
 	}
+
 	/**
 	 * Delete a loose ref and make sure the directory in refs is deleted too,
 	 * and the reflog dir too
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 e03bd8723f..c8c2dd5867 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
@@ -580,6 +580,9 @@ public class RefDirectory extends RefDatabase {
 
 	void delete(RefDirectoryUpdate update) throws IOException {
 		Ref dst = update.getRef();
+		if (!update.isDetachingSymbolicRef()) {
+			dst = dst.getLeaf();
+		}
 		String name = dst.getName();
 
 		// Write the packed-refs file using an atomic update. We might
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
index fc334f0275..61fda943c0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
@@ -278,6 +278,16 @@ public abstract class RefUpdate {
 	}
 
 	/**
+	 * Return whether this update is actually detaching a symbolic ref.
+	 *
+	 * @return true if detaching a symref.
+	 * @since 4.9
+	 */
+	public boolean isDetachingSymbolicRef() {
+		return detachingSymbolicRef;
+	}
+
+	/**
 	 * Set the new value the ref will update to.
 	 *
 	 * @param id