[PATCH v3 7/8] refs: stop resolving ref corresponding to reflogs
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 21, 2024, 12:37 UTC
- Message-ID
- <fc96d5bbabe7986d08eb1645549b5591784521e4.1708518982.git.ps@pks.im>
- In-Reply-To
- <cover.1708518982.git.ps@pks.im>
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 <ps@pks.im> --- 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 c7aff6b331..6f98168a81 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 4998b676c2..6c11c4a5e3 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1616,11 +1616,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