git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 10/11] refs/files: remove unused "errno == EISDIR" code

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jul 26, 2021, 23:44 UTC
Message-ID
<patch-10.11-009abc9968-20210726T234237Z-avarab@gmail.com>
In-Reply-To
<cover-00.11-0000000000-20210726T234237Z-avarab@gmail.com>

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>
---
 refs/files-backend.c | 28 +++-------------------------
 1 file changed, 3 insertions(+), 25 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 98d9aa1601..28cd8853f5 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -883,7 +883,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);
@@ -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)) {
 		last_errno = errno;
 		if (last_errno != ENOTDIR ||
 		    !refs_verify_refname_available(&refs->base, refname,
-- 
2.32.0.956.g6b0c84ceda8
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 68 of 88 in “refs file backend: remove dead "errno == EISDIR" code”
  1. refs file backend: remove dead "errno == EISDIR" codeÆvar Arnfjörð Bjarmason, Jul 14, 2021
  2. Jeff KingJul 14, 2021
  3. Ævar Arnfjörð BjarmasonJul 14, 2021
  4. Jeff KingJul 14, 2021
  5. Ævar Arnfjörð BjarmasonJul 15, 2021
  6. Jeff KingJul 15, 2021
  7. Junio C HamanoJul 17, 2021
  8. Jeff KingJul 17, 2021
  9. Junio C HamanoJul 19, 2021
  10. Junio C HamanoJul 19, 2021
  11. Ævar Arnfjörð BjarmasonJul 17, 2021
  12. 00/11 fix "git reflog expire" race & get rid of EISDIR in refs APIÆvar Arnfjörð Bjarmason, Jul 16, 2021
  13. 01/11 refs/packet: add missing BUG() invocations to reflog callbacksÆvar Arnfjörð Bjarmason, Jul 16, 2021
  14. 02/11 refs/files: remove unused REF_DELETING in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Jul 16, 2021
  15. Jeff KingJul 17, 2021
  16. Junio C HamanoJul 19, 2021
  17. Jeff KingJul 20, 2021
  18. 03/11 refs/files: remove unused "extras/skip" in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Jul 16, 2021
  19. 04/11 refs/files: remove unused "skip" in lock_raw_ref() tooÆvar Arnfjörð Bjarmason, Jul 16, 2021
  20. 05/11 refs/debug: re-indent argument list for "prepare"Ævar Arnfjörð Bjarmason, Jul 16, 2021
  21. 06/11 refs API: pass the "lock OID" to reflog "prepare"Ævar Arnfjörð Bjarmason, Jul 16, 2021
  22. Jeff KingJul 17, 2021
  23. Junio C HamanoJul 19, 2021
  24. Ævar Arnfjörð BjarmasonJul 19, 2021
  25. 07/11 refs: make repo_dwim_log() accept a NULL oidÆvar Arnfjörð Bjarmason, Jul 16, 2021
  26. 08/11 refs/files: add a comment about refs_reflog_exists() callÆvar Arnfjörð Bjarmason, Jul 16, 2021
  27. Jeff KingJul 17, 2021
  28. Junio C HamanoJul 19, 2021
  29. Jeff KingJul 20, 2021
  30. 09/11 reflog expire: don't lock reflogs using previously seen OIDÆvar Arnfjörð Bjarmason, Jul 16, 2021
  31. Jeff KingJul 17, 2021
  32. Han-Wen NienhuysAug 17, 2021
  33. Junio C HamanoAug 18, 2021
  34. Carlo Marcelo Arenas BelónAug 19, 2021
  35. Junio C HamanoAug 20, 2021
  36. 10/11 refs/files: remove unused "oid" in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Jul 16, 2021
  37. Jeff KingJul 17, 2021
  38. 11/11 refs/files: remove unused "errno == EISDIR" codeÆvar Arnfjörð Bjarmason, Jul 16, 2021
  39. Jeff KingJul 17, 2021
  40. Jeff KingJul 17, 2021
  41. 00/12 fix "git reflog expire" race & get rid of EISDIR in refs APIÆvar Arnfjörð Bjarmason, Jul 20, 2021
  42. 10/12 refs/files: remove unused "oid" in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Jul 20, 2021
  43. 05/12 refs/debug: re-indent argument list for "prepare"Ævar Arnfjörð Bjarmason, Jul 20, 2021
  44. 08/12 refs/files: add a comment about refs_reflog_exists() callÆvar Arnfjörð Bjarmason, Jul 20, 2021
  45. 07/12 refs: make repo_dwim_log() accept a NULL oidÆvar Arnfjörð Bjarmason, Jul 20, 2021
  46. 02/12 refs/files: remove unused REF_DELETING in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Jul 20, 2021
  47. Junio C HamanoAug 2, 2021
  48. 01/12 refs/packet: add missing BUG() invocations to reflog callbacksÆvar Arnfjörð Bjarmason, Jul 20, 2021
  49. 11/12 refs/files: remove unused "errno == EISDIR" codeÆvar Arnfjörð Bjarmason, Jul 20, 2021
  50. 09/12 reflog expire: don't lock reflogs using previously seen OIDÆvar Arnfjörð Bjarmason, Jul 20, 2021
  51. 03/12 refs/files: remove unused "extras/skip" in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Jul 20, 2021
  52. 06/12 refs API: pass the "lock OID" to reflog "prepare"Ævar Arnfjörð Bjarmason, Jul 20, 2021
  53. Junio C HamanoJul 21, 2021
  54. Junio C HamanoJul 21, 2021
  55. 12/12 refs/files: remove unused "errno != ENOTDIR" conditionÆvar Arnfjörð Bjarmason, Jul 20, 2021
  56. 04/12 refs/files: remove unused "skip" in lock_raw_ref() tooÆvar Arnfjörð Bjarmason, Jul 20, 2021
  57. 00/11 fix "git reflog expire" race & get rid of EISDIR in refs APIÆvar Arnfjörð Bjarmason, Jul 26, 2021
  58. 02/11 refs/files: remove unused REF_DELETING in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Jul 26, 2021
  59. 01/11 refs/packet: add missing BUG() invocations to reflog callbacksÆvar Arnfjörð Bjarmason, Jul 26, 2021
  60. 03/11 refs/files: remove unused "extras/skip" in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Jul 26, 2021
  61. 05/11 refs/debug: re-indent argument list for "prepare"Ævar Arnfjörð Bjarmason, Jul 26, 2021
  62. 04/11 refs/files: remove unused "skip" in lock_raw_ref() tooÆvar Arnfjörð Bjarmason, Jul 26, 2021
  63. 07/11 refs/files: add a comment about refs_reflog_exists() callÆvar Arnfjörð Bjarmason, Jul 26, 2021
  64. 09/11 refs/files: remove unused "oid" in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Jul 26, 2021
  65. 08/11 reflog expire: don't lock reflogs using previously seen OIDÆvar Arnfjörð Bjarmason, Jul 26, 2021
  66. Junio C HamanoAug 2, 2021
  67. Ævar Arnfjörð BjarmasonAug 4, 2021
  68. 10/11 refs/files: remove unused "errno == EISDIR" codeÆvar Arnfjörð Bjarmason, Jul 26, 2021
  69. 11/11 refs/files: remove unused "errno != ENOTDIR" conditionÆvar Arnfjörð Bjarmason, Jul 26, 2021
  70. 06/11 refs: make repo_dwim_log() accept a NULL oidÆvar Arnfjörð Bjarmason, Jul 26, 2021
  71. 00/13 fix "git reflog expire" race & get rid of EISDIR in refs APIÆvar Arnfjörð Bjarmason, Aug 23, 2021
  72. 01/13 refs/packet: add missing BUG() invocations to reflog callbacksÆvar Arnfjörð Bjarmason, Aug 23, 2021
  73. 02/13 refs/files: remove unused REF_DELETING in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Aug 23, 2021
  74. 03/13 refs: drop unused "flags" parameter to lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Aug 23, 2021
  75. 04/13 refs/files: remove unused "extras/skip" in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Aug 23, 2021
  76. 05/13 refs/files: remove unused "skip" in lock_raw_ref() tooÆvar Arnfjörð Bjarmason, Aug 23, 2021
  77. 06/13 refs/debug: re-indent argument list for "prepare"Ævar Arnfjörð Bjarmason, Aug 23, 2021
  78. 07/13 refs: make repo_dwim_log() accept a NULL oidÆvar Arnfjörð Bjarmason, Aug 23, 2021
  79. 08/13 refs/files: add a comment about refs_reflog_exists() callÆvar Arnfjörð Bjarmason, Aug 23, 2021
  80. 09/13 reflog expire: don't lock reflogs using previously seen OIDÆvar Arnfjörð Bjarmason, Aug 23, 2021
  81. 10/13 refs API: remove OID argument to reflog_expire()Ævar Arnfjörð Bjarmason, Aug 23, 2021
  82. 11/13 refs/files: remove unused "oid" in lock_ref_oid_basic()Ævar Arnfjörð Bjarmason, Aug 23, 2021
  83. 12/13 refs/files: remove unused "errno == EISDIR" codeÆvar Arnfjörð Bjarmason, Aug 23, 2021
  84. 13/13 refs/files: remove unused "errno != ENOTDIR" conditionÆvar Arnfjörð Bjarmason, Aug 23, 2021
  85. Ævar Arnfjörð BjarmasonJul 23, 2021
  86. Junio C HamanoJul 23, 2021
  87. Ævar Arnfjörð BjarmasonJul 26, 2021
  88. Junio C HamanoJul 26, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.