Hello Junio,

Junio C Hamano [off-list ref] writes:

Karthik Nayak [off-list ref] writes:

quoted

We cannot directly add the new syntax checks to `is_pseudoref_syntax()`
because the function is also used by `is_current_worktree_ref()` and
making it stricter to match only known pseudorefs might have unintended
consequences due to files like 'BISECT_START' which isn't a pseudoref
but sometimes contains object ID.

Well described.

quoted

diff --git a/refs.c b/refs.c
index 20e8f1ff1f..4b6bfc66fb 100644
--- a/refs.c
+++ b/refs.c

@@ -859,6 +859,38 @@ static int is_pseudoref_syntax(const char *refname)
 	return 1;
 }

+int is_pseudoref(struct ref_store *refs, const char *refname)
+{
+	static const char *const irregular_pseudorefs[] = {
+		"AUTO_MERGE",
+		"BISECT_EXPECTED_REV",
+		"NOTES_MERGE_PARTIAL",
+		"NOTES_MERGE_REF",
+		"MERGE_AUTOSTASH"

Let's end an array's initializer with a trailing comma, to help
future patches to add entries to this array without unnecessary
patch noise.

Sure, will add!

quoted

+	};
+	size_t i;
+
+	if (!is_pseudoref_syntax(refname))
+		return 0;
+
+	if (ends_with(refname, "_HEAD"))
+		return refs_ref_exists(refs, refname);
+
+	for (i = 0; i < ARRAY_SIZE(irregular_pseudorefs); i++)
+		 if (!strcmp(refname, irregular_pseudorefs[i]))
+			 return refs_ref_exists(refs, refname);
+
+	return 0;
+}

The above uses refs_ref_exists() because we want these to
successfully resolve for reading.

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`.

I'll wait a day or two, before sending in the new version with the
fixes. The current diff is

diff --git a/refs.c b/refs.c
index b5e63f133a..4a1fd30ef2 100644
--- a/refs.c
+++ b/refs.c

@@ -866,7 +866,7 @@ int is_pseudoref(struct ref_store *refs, const

char *refname)
 		"BISECT_EXPECTED_REV",
 		"NOTES_MERGE_PARTIAL",
 		"NOTES_MERGE_REF",
-		"MERGE_AUTOSTASH"
+		"MERGE_AUTOSTASH",
 	};
 	size_t i;

@@ -885,10 +885,23 @@ int is_pseudoref(struct ref_store *refs, const

char *refname)

 int is_headref(struct ref_store *refs, const char *refname)
 {
-	if (!strcmp(refname, "HEAD"))
-		return refs_ref_exists(refs, refname);
+	struct object_id oid;
+	int flag;

-	return 0;
+	if (strcmp(refname, "HEAD"))
+		return 0;
+
+	/*
+	 * If HEAD doesn't exist, we don't have to die, but rather,
+	 * we simply return 0.
+	 */
+	if (read_ref_full("HEAD", RESOLVE_REF_READING, &oid, &flag))
+		return 0;
+
+	if (is_null_oid(&oid))
+		return 0;
+
+	return 1;
 }

 static int is_current_worktree_ref(const char *ref) {

- Karthik