On Fri, Jul 16, 2021 at 04:13:07PM +0200, Ævar Arnfjörð Bjarmason wrote:
> Further historical commentary:
>
> Before the two preceding commits the caller in files_reflog_expire()
> was the only one out of our 4 callers that would pass non-NULL as an
> oid. We would then set a (now gone) "resolve_flags" to
> "RESOLVE_REF_READING" and just before that "errno != EISDIR" check do:
> [...]
Right, thanks for digging into this situation in the commit message.
> @@ -891,30 +890,9 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
> CALLOC_ARRAY(lock, 1);
>
> files_ref_path(refs, &ref_file, refname);
> - resolved = !!refs_resolve_ref_unsafe(&refs->base, refname,
> - RESOLVE_REF_NO_RECURSE,
> - &lock->old_oid, type);
> - if (!resolved && errno == EISDIR) {
> - /*
> - * we are trying to lock foo but we used to
> - * have foo/bar which now does not exist;
> - * it is normal for the empty directory 'foo'
> - * to remain.
> - */
> - if (remove_empty_directories(&ref_file)) {
> - last_errno = errno;
> - if (!refs_verify_refname_available(
> - &refs->base,
> - refname, NULL, NULL, err))
> - strbuf_addf(err, "there are still refs under '%s'",
> - refname);
> - goto error_return;
> - }
> - resolved = !!refs_resolve_ref_unsafe(&refs->base, refname,
> - RESOLVE_REF_NO_RECURSE,
> - &lock->old_oid, type);
> - }
> - if (!resolved) {
> + if (!refs_resolve_ref_unsafe(&refs->base, refname,
> + RESOLVE_REF_NO_RECURSE,
> + &lock->old_oid, type)) {
I agree that this is all trivially dead code now, and can be removed.
> last_errno = errno;
> if (last_errno != ENOTDIR ||
> !refs_verify_refname_available(&refs->base, refname,
Not necessary, but I think "last_errno != ENOTDIR" will always be true
now, too. We treat it the same as EISDIR in refs_resolve_ref_unsafe().
It's not that big a cleanup, but it could easily go on top.
-Peff
