Re: [PATCH v2 1/4] refs: introduce `is_pseudoref()` and `is_headref()`
From: Junio C Hamano <hidden>
Date: 2024-01-25 16:28:27
Karthik Nayak [off-list ref] writes:
quoted
quoted
+int is_headref(struct ref_store *refs, const char *refname) +{ + if (!strcmp(refname, "HEAD")) + return refs_ref_exists(refs, refname);Given that "git for-each-ref refs/remotes" does not show "refs/remotes/origin/HEAD" in the output when we do not have the remote-tracking branch that symref points at, we probably do want to omit "HEAD" from the output when the HEAD symref points at an unborn branch. If refs_ref_exists() says "no, it does not exist" in such a case, we are perfectly fine with this code. We do not have to worry about the unborn state for pseudorefs because they would never be symbolic. But that in turn makes me suspect that the check done with refs_ref_exists() in the is_pseudoref() helper is a bit too lenient by allowing it to be a symbolic ref. Shouldn't we be using a check based on read_ref_full(), like we did in another topic recently [*]? [Reference] * https://lore.kernel.org/git/xmqqzfxa9usx.fsf@gitster.g/ (local)Thanks, this makes sense and the link is helpful. I'll do something similar, but since HEAD can be a symref, I'll drop the `RESOLVE_REF_NO_RECURSE` flag and only use `RESOLVE_REF_READING`.
Just to make sure there is no misunderstanding, I think how is_headref() does what it does in the patch is perfectly fine, including its use of refs_ref_exists(). The side I was referring to with "in turn makes me suspect" is the other helper function that will never have to deal with a symref. Use of refs_ref_exists() in that function is too loose.