The reflog iterator tries to resolve the corresponding ref for every reflog that it is about to yield. Historically, this was done due to multiple reasons: - It ensures that the refname is safe because we end up calling `check_refname_format()`. Also, non-conformant refnames are skipped altogether. - The iterator used to yield the resolved object ID as well as its flags to the callback. This info was never used though, and the corresponding parameters were dropped in the preceding commit. - When a ref is corrupt then the reflog is not emitted at all. We're about to introduce a new `git reflog list` subcommand that will print all reflogs that the refdb knows about. Skipping over reflogs whose refs are corrupted would be quite counterproductive in this case as the user would have no way to learn about reflogs which may still exist in their repository to help and rescue such a corrupted ref. Thus, the only remaining reason for why we'd want to resolve the ref is to verify its refname. Refactor the code to call `check_refname_format()` directly instead of trying to resolve the ref. This is significantly more efficient given that we don't have to hit the object database anymore to list reflogs. And second, it ensures that we end up showing reflogs of broken refs, which will help to make the reflog more useful. Note that this really only impacts the case where the corresponding ref is corrupt. Reflogs for nonexistent refs would have been returned to the caller beforehand already as we did not pass `RESOLVE_REF_READING` to the function, and thus `refs_resolve_ref_unsafe()` would have returned successfully in that case. Signed-off-by: Patrick Steinhardt --- refs/files-backend.c | 12 ++---------- refs/reftable-backend.c | 6 ++---- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 48cc60d71b..4726b04baa 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2129,17 +2129,9 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator) while ((ok = dir_iterator_advance(diter)) == ITER_OK) { if (!S_ISREG(diter->st.st_mode)) continue; - if (diter->basename[0] == '.') + if (check_refname_format(diter->basename, + REFNAME_ALLOW_ONELEVEL)) continue; - if (ends_with(diter->basename, ".lock")) - continue; - - if (!refs_resolve_ref_unsafe(iter->ref_store, - diter->relative_path, 0, - NULL, NULL)) { - error("bad ref for %s", diter->path.buf); - continue; - } iter->base.refname = diter->relative_path; return ITER_OK; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 5247e09d58..f3200a1886 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1659,11 +1659,9 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator) if (iter->last_name && !strcmp(iter->log.refname, iter->last_name)) continue; - if (!refs_resolve_ref_unsafe(&iter->refs->base, iter->log.refname, - 0, NULL, NULL)) { - error(_("bad ref for %s"), iter->log.refname); + if (check_refname_format(iter->log.refname, + REFNAME_ALLOW_ONELEVEL)) continue; - } free(iter->last_name); iter->last_name = xstrdup(iter->log.refname); -- 2.44.0-rc1