Thread (2 messages) 2 messages, 2 authors, 2024-01-25

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help