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

[PATCH 01/20] get_ref_dir(): don't call read_loose_refs() for "refs/bisect"

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Mar 20, 2017, 16:33 UTC
Message-ID
<53e0eed5e13a04f502dcaeda83b2cbb3035f1396.1490026594.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1490026594.git.mhagger@alum.mit.edu>

Since references under "refs/bisect/" are per-worktree, they have to be sought in the worktree rather than in the main repository. But since loose references are found by traversing directories, the reference iterator won't even get the idea to look for a "refs/bisect/" directory in the worktree if there is not a directory with that name in the main repository. Thus `get_ref_dir()` manually inserts a dir_entry for "refs/bisect/" whenever it reads the entry for "refs/".

The current code then immediately calls `read_loose_refs()` on that directory. But since the dir_entry is created with its `incomplete` flag set, any traversal that gets to this point will read the directory automatically. So there is no need to call `read_loose_refs()` explicitly; the lazy mechanism suffices.

And in fact, the attempt to `read_loose_refs()` was broken anyway. That function needs its `dirname` argument to have a trailing `/` character, but the invocation here was passing it "refs/bisect" without a trailing slash. So `read_loose_refs()` would read `$GIT_DIR/refs/bisect" correctly, but if it found an entry "foo" in that directory, it would try to read "$GIT_DIR/refs/bisectfoo". Normally it wouldn't find anything at that path, but the failure was canceled out because `get_ref_dir()` *also* forgot to reset the `REF_INCOMPLETE` bit on the dir_entry. So the read was attempted again when it was accessed, via the lazy mechanism, and this time the read was done correctly.

This code has been broken since it was first introduced.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs/files-backend.c | 2 --
 1 file changed, 2 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 4242486118..e7a075e864 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -191,8 +191,6 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)
 							       "refs/bisect/",
 							       12, 1);
 				add_entry_to_dir(dir, child_entry);
-				read_loose_refs("refs/bisect",
-						&child_entry->u.subdir);
 			}
 		}
 		entry->flag &= ~REF_INCOMPLETE;
-- 
2.11.0
Previous: Michael HaggertyNext: Michael Haggerty
Message 2 of 38 in “Separate `ref_cache` into a separate module”
  1. 00/20 Separate `ref_cache` into a separate moduleMichael Haggerty, Mar 20, 2017
  2. 01/20 get_ref_dir(): don't call read_loose_refs() for "refs/bisect"Michael Haggerty, Mar 20, 2017
  3. 10/20 ref-cache: introduce a new type, ref_cacheMichael Haggerty, Mar 20, 2017
  4. 13/20 refs: handle "refs/bisect/" in `loose_fill_ref_dir()`Michael Haggerty, Mar 20, 2017
  5. 11/20 refs: record the ref_store in ref_cache, not ref_dirMichael Haggerty, Mar 20, 2017
  6. Jeff KingMar 20, 2017
  7. Michael HaggertyMar 20, 2017
  8. 08/20 Rename `remove_entry()` to `remove_entry_from_dir()`Michael Haggerty, Mar 20, 2017
  9. 18/20 commit_packed_refs(): use reference iterationMichael Haggerty, Mar 20, 2017
  10. Jeff KingMar 20, 2017
  11. Michael HaggertyMar 22, 2017
  12. Jeff KingMar 22, 2017
  13. 20/20 do_for_each_entry_in_dir(): delete functionMichael Haggerty, Mar 20, 2017
  14. 16/20 get_loose_ref_cache(): new functionMichael Haggerty, Mar 20, 2017
  15. 15/20 get_loose_ref_dir(): function renamed from get_loose_refs()Michael Haggerty, Mar 20, 2017
  16. 19/20 files_pack_refs(): use reference iterationMichael Haggerty, Mar 20, 2017
  17. 14/20 do_for_each_entry_in_dir(): eliminate `offset` argumentMichael Haggerty, Mar 20, 2017
  18. 03/20 refs_ref_iterator_begin(): new functionMichael Haggerty, Mar 20, 2017
  19. 17/20 cache_ref_iterator_begin(): make function smarterMichael Haggerty, Mar 20, 2017
  20. 07/20 Rename `find_ref()` to `find_ref_entry()`Michael Haggerty, Mar 20, 2017
  21. 12/20 ref-cache: use a callback function to fill the cacheMichael Haggerty, Mar 20, 2017
  22. 09/20 refs: split `ref_cache` code into separate filesMichael Haggerty, Mar 20, 2017
  23. Jeff KingMar 20, 2017
  24. Junio C HamanoMar 20, 2017
  25. Stefan BellerMar 20, 2017
  26. Junio C HamanoMar 20, 2017
  27. 04/20 refs_verify_refname_available(): implement once for all backendsMichael Haggerty, Mar 20, 2017
  28. Jeff KingMar 20, 2017
  29. Michael HaggertyMar 20, 2017
  30. 02/20 refs_read_raw_ref(): new functionMichael Haggerty, Mar 20, 2017
  31. 06/20 Rename `add_ref()` to `add_ref_entry()`Michael Haggerty, Mar 20, 2017
  32. 05/20 refs_verify_refname_available(): use function in more placesMichael Haggerty, Mar 20, 2017
  33. Junio C HamanoMar 20, 2017
  34. Jeff KingMar 20, 2017
  35. Ævar Arnfjörð BjarmasonMar 20, 2017
  36. Jeff KingMar 20, 2017
  37. Junio C HamanoMar 20, 2017
  38. Michael HaggertyMar 20, 2017

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.