From 1e47c7058d930db920cb897faa2df14a4ae87c00 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 10 Nov 2015 16:34:00 -0800 Subject: RefDirectory.getRef: Treat fake missing symrefs like real ones getRef() loops over its search path to find a ref: Ref ref = null; for (String prefix : SEARCH_PATH) { ref = readRef(prefix + needle, packed); if (ref != null) { ref = resolve(ref, 0, null, null, packed); break; } } fireRefsChanged(); return ref; If readRef returns null (indicating that the ref does not exist), the loop continues so we can find the ref later in the search path. And resolve should never return null, so if we return null it should mean we exhausted the entire search path and didn't find the ref. ... except that resolve can return null: it does so when it has followed too many symrefs and concluded that there is a symref loop: if (MAX_SYMBOLIC_REF_DEPTH <= depth) return null; // claim it doesn't exist Continue the loop instead of returning null immediately. This makes the behavior more consistent. Arguably getRef should throw an exception when a symref loop is detected. That would be a more invasive change, so if it's a good idea it will have to wait for another patch. Change-Id: Icb1c7fafd4f1e34c9b43538e27ab5bbc17ad9eef Signed-off-by: Jonathan Nieder --- .../internal/storage/file/RefDirectoryTest.java | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'org.eclipse.jgit.test/tst/org/eclipse/jgit') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java index d66753da08..52e181bc80 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/RefDirectoryTest.java @@ -857,6 +857,36 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase { assertNull("mising 1 due to cycle", r); } + @Test + public void testGetRef_CycleInSymbolicRef() throws IOException { + Ref r; + + writeLooseRef("refs/1", "ref: refs/2\n"); + writeLooseRef("refs/2", "ref: refs/3\n"); + writeLooseRef("refs/3", "ref: refs/4\n"); + writeLooseRef("refs/4", "ref: refs/5\n"); + writeLooseRef("refs/5", "ref: refs/end\n"); + writeLooseRef("refs/end", A); + + r = refdir.getRef("1"); + assertEquals("refs/1", r.getName()); + assertEquals(A, r.getObjectId()); + assertTrue(r.isSymbolic()); + + writeLooseRef("refs/5", "ref: refs/6\n"); + writeLooseRef("refs/6", "ref: refs/end\n"); + + r = refdir.getRef("1"); + assertNull("missing 1 due to cycle", r); + + writeLooseRef("refs/heads/1", B); + + r = refdir.getRef("1"); + assertEquals("refs/heads/1", r.getName()); + assertEquals(B, r.getObjectId()); + assertFalse(r.isSymbolic()); + } + @Test public void testGetRefs_PackedNotPeeled_Sorted() throws IOException { Map all; -- cgit v1.2.3