| Message ID | 20240129113527.607022-2-karthik.188@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 75fd01e7f476c527134d4058424ae20eca459a6a |
| Headers | show |
| Series | for-each-ref: print all refs on empty string pattern | expand |
On Mon, Jan 29, 2024 at 12:35:24PM +0100, Karthik Nayak wrote: > +int is_pseudoref(struct ref_store *refs, const char *refname) > [...] > + if (ends_with(refname, "_HEAD")) { > + read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, > + &oid, NULL); > + return !is_null_oid(&oid); > + } I was going to prepare a patch on top, but since it looks like this was reverted out of 'next' to be revamped, I thought I'd mention it now: -Wunused-parameter notices that we never use the "refs" parameter to the function. And indeed it looks like a (possible) bug, since read_ref_full() is going to use the_repository to find the ref store. I think you'd want something like this squashed in (note that it also fixes a slight indent problem in the first block): diff --git a/refs.c b/refs.c index 3190df8d81..0d65c31117 100644 --- a/refs.c +++ b/refs.c @@ -875,15 +875,17 @@ int is_pseudoref(struct ref_store *refs, const char *refname) return 0; if (ends_with(refname, "_HEAD")) { - read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, - &oid, NULL); - return !is_null_oid(&oid); + refs_resolve_ref_unsafe(refs, refname, + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, + &oid, NULL); + return !is_null_oid(&oid); } for (i = 0; i < ARRAY_SIZE(irregular_pseudorefs); i++) if (!strcmp(refname, irregular_pseudorefs[i])) { - read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, - &oid, NULL); + refs_resolve_ref_unsafe(refs, refname, + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, + &oid, NULL); return !is_null_oid(&oid); } -Peff
Jeff King <peff@peff.net> writes: > On Mon, Jan 29, 2024 at 12:35:24PM +0100, Karthik Nayak wrote: > >> +int is_pseudoref(struct ref_store *refs, const char *refname) >> [...] >> + if (ends_with(refname, "_HEAD")) { >> + read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, >> + &oid, NULL); >> + return !is_null_oid(&oid); >> + } > > I was going to prepare a patch on top, but since it looks like this was > reverted out of 'next' to be revamped, I thought I'd mention it now: > -Wunused-parameter notices that we never use the "refs" parameter to the > function. And indeed it looks like a (possible) bug, since > read_ref_full() is going to use the_repository to find the ref store. > > I think you'd want something like this squashed in (note that it also > fixes a slight indent problem in the first block): > > diff --git a/refs.c b/refs.c > index 3190df8d81..0d65c31117 100644 > --- a/refs.c > +++ b/refs.c > @@ -875,15 +875,17 @@ int is_pseudoref(struct ref_store *refs, const char *refname) > return 0; > > if (ends_with(refname, "_HEAD")) { > - read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, > - &oid, NULL); > - return !is_null_oid(&oid); > + refs_resolve_ref_unsafe(refs, refname, > + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, > + &oid, NULL); > + return !is_null_oid(&oid); > } > > for (i = 0; i < ARRAY_SIZE(irregular_pseudorefs); i++) > if (!strcmp(refname, irregular_pseudorefs[i])) { > - read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, > - &oid, NULL); > + refs_resolve_ref_unsafe(refs, refname, > + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, > + &oid, NULL); > return !is_null_oid(&oid); > } > > > -Peff Thanks Jeff, makes sense, will squash this in.
diff --git a/refs.c b/refs.c index 20e8f1ff1f..559f5aeea8 100644 --- a/refs.c +++ b/refs.c @@ -859,6 +859,45 @@ 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", + }; + struct object_id oid; + size_t i; + + if (!is_pseudoref_syntax(refname)) + return 0; + + if (ends_with(refname, "_HEAD")) { + read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, + &oid, NULL); + return !is_null_oid(&oid); + } + + for (i = 0; i < ARRAY_SIZE(irregular_pseudorefs); i++) + if (!strcmp(refname, irregular_pseudorefs[i])) { + read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, + &oid, NULL); + return !is_null_oid(&oid); + } + + return 0; +} + +int is_headref(struct ref_store *refs, const char *refname) +{ + if (!strcmp(refname, "HEAD")) + return refs_ref_exists(refs, refname); + + return 0; +} + static int is_current_worktree_ref(const char *ref) { return is_pseudoref_syntax(ref) || is_per_worktree_ref(ref); } diff --git a/refs.h b/refs.h index 11b3b6ccea..46b8085d63 100644 --- a/refs.h +++ b/refs.h @@ -1021,4 +1021,7 @@ extern struct ref_namespace_info ref_namespace[NAMESPACE__COUNT]; */ void update_ref_namespace(enum ref_namespace namespace, char *ref); +int is_pseudoref(struct ref_store *refs, const char *refname); +int is_headref(struct ref_store *refs, const char *refname); + #endif /* REFS_H */
Introduce two new functions `is_pseudoref()` and `is_headref()`. This provides the necessary functionality for us to add pseudorefs and HEAD to the loose ref cache in the files backend, allowing us to build tooling to print these refs. The `is_pseudoref()` function internally calls `is_pseudoref_syntax()` but adds onto it by also checking to ensure that the pseudoref either ends with a "_HEAD" suffix or matches a list of exceptions. After which we also parse the contents of the pseudoref to ensure that it conforms to the ref format. 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. Keeping this in mind, we leave `is_pseudoref_syntax()` as is and create `is_pseudoref()` which is stricter. Ideally we'd want to move the new syntax checks to `is_pseudoref_syntax()` but a prerequisite for this would be to actually remove the exception list by converting those pseudorefs to also contain a '_HEAD' suffix and perhaps move bisect related files like 'BISECT_START' to a new directory similar to the 'rebase-merge' directory. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> --- refs.c | 39 +++++++++++++++++++++++++++++++++++++++ refs.h | 3 +++ 2 files changed, 42 insertions(+)