refs/files: remove unused "errno == EISDIR" code

When we lock a reference like "foo" we need to handle the case where
"foo" exists, but is an empty directory. That's what this code added
in bc7127ef0f (ref locking: allow 'foo' when 'foo/bar' used to exist
but not anymore., 2006-09-30) seems like it should be dealing with.

Except it doesn't, and we never take this branch. The reason is that
when bc7127ef0f was written this looked like:

	ref = resolve_ref([...]);
	if (!ref && errno == EISDIR) {
	[...]

And in resolve_ref() we had this code:

	fd = open(path, O_RDONLY);
	if (fd < 0)
		return NULL;

I.e. we would attempt to read "foo" with open(), which would fail with
EISDIR and we'd return NULL. We'd then take this branch, call
remove_empty_directories() and continue.

Since a1c1d8170d (refs_resolve_ref_unsafe: handle d/f conflicts for
writes, 2017-10-06) we don't. E.g. in the case of
files_copy_or_rename_ref() our callstack will look something like:

	[...] ->
	files_copy_or_rename_ref() ->
	lock_ref_oid_basic() ->
	refs_resolve_ref_unsafe()

At that point the first (now only) refs_resolve_ref_unsafe() call in
lock_ref_oid_basic() would do the equivalent of this in the resulting
call to refs_read_raw_ref() in refs_resolve_ref_unsafe():

	/* Via refs_read_raw_ref() */
	fd = open(path, O_RDONLY);
	if (fd < 0)
		/* get errno == EISDIR */
	/* later, in refs_resolve_ref_unsafe() */
	if ([...] && errno != EISDIR)
		return NULL;
	[...]
	/* returns the refs/heads/foo to the caller, even though it's a directory */
	return refname;

I.e. even though we got an "errno == EISDIR" we won't take this
branch, since in cases of EISDIR "resolved" is always
non-NULL. I.e. we pretend at this point as though everything's OK and
there is no "foo" directory.

We then proceed with the entire ref update and don't call
remove_empty_directories() until we call commit_ref_update(). See
5387c0d883 (commit_ref(): if there is an empty dir in the way, delete
it, 2016-05-05) for the addition of that code, and
a1c1d8170db (refs_resolve_ref_unsafe: handle d/f conflicts for writes,
2017-10-06) for the commit that changed the original codepath added in
bc7127ef0f to use this "EISDIR" handling.

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:

	if (resolve_flags & RESOLVE_REF_READING)
		return NULL;

There may have been some case where this ended up mattering and we
couldn't safely make this change before we removed the "oid"
parameter, but I don't think there was, see [1] for some discussion on
that.

In any case, now that we've removed the "oid" parameter in a preceding
commit we can be sure that this code is redundant, so let's remove it.

1. http://lore.kernel.org/git/871r801yp6.fsf@evledraar.gmail.com

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 4f2d907597a8dc39533f21cdf7469e7dbe2e2f25..bed2ab25c3a641f3c7f01aff74129a5697edca0a 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -882,7 +882,6 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
        struct strbuf ref_file = STRBUF_INIT;
        struct ref_lock *lock;
        int last_errno = 0;
-       int resolved;
 
        files_assert_main_repository(refs, "lock_ref_oid_basic");
        assert(err);
@@ -890,30 +889,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)) {
                last_errno = errno;
                if (last_errno != ENOTDIR ||
                    !refs_verify_refname_available(&refs->base, refname,