On Fri, Oct 06, 2017 at 03:39:13AM -0400, Jeff King wrote:
> I got a chance to look at this again. I think the root of the problem is
> that resolve_ref() as it is implemented now is just totally unsuitable
> for asking the question "what does this symbolic link point to?".
>
> Because you end up with either:
>
> 1. If we pass RESOLVE_REF_READING, then we do not return the target
> refname for orphaned commits (which is why 31824d180d dropped it).
>
> 2. If not, then we do not return the target refname for commits with
> names that are not available for writing. The d/f conflict here is
> one example, but there may be others.
>
> So I think we need to teach resolve_ref() a new mode that's like
> "reading", but just follows the symref chain.
This analysis is not _quite_ right. The "not available for writing"
thing actually isn't intentionally enforced by the resolve_ref. It's
just that it's not careful enough about checking errno. We see EISDIR
instead of ENOENT when there's a d/f situation, but both have the same
practical effect: that ref doesn't exist.
I.e., this lookup has _always_ been broken, even in the "reading" case.
It's just that the fix from 31824d180d (correctly) made git-branch more
careful about handling the cases where we couldn't resolve a HEAD.
So this patch fixes the problem:
diff --git a/refs.c b/refs.c
index df075fcd06..2ba74720c8 100644
--- a/refs.c
+++ b/refs.c
@@ -1435,7 +1435,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
if (refs_read_raw_ref(refs, refname,
sha1, &sb_refname, &read_flags)) {
*flags |= read_flags;
- if (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))
+ if ((errno != ENOENT && errno != EISDIR) ||
+ (resolve_flags & RESOLVE_REF_READING))
return NULL;
hashclr(sha1);
if (*flags & REF_BAD_NAME)
but seems to stimulate a test failure in t3308. I have a suspicion that
I've just uncovered another bug, but I'll dig in that. In the meantime I
wanted to post this update in case anybody else was looking into it.
-Peff
