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

[PATCH 19/33] refs: change how packed refs are deleted

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Apr 14, 2013, 12:54 UTC
Message-ID
<1365944088-10588-20-git-send-email-mhagger@alum.mit.edu>
In-Reply-To
<1365944088-10588-1-git-send-email-mhagger@alum.mit.edu>

Add a function remove_ref(), which removes a single entry from a reference cache.

Use this function to reimplement repack_without_ref(). The old version iterated over all refs, packing all of them except for the one to be deleted, then discarded the entire packed reference cache. The new version deletes the doomed reference from the cache *before* iterating.

This has two advantages:
* the code for writing packed-refs becomes simpler, because it doesn't
  have to exclude one of the references.
* it is no longer necessary to discard the packed refs cache after
  deleting a reference: symbolic refs cannot be packed, so packed
  references cannot depend on each other, so the rest of the packed
  refs cache remains valid after a reference is deleted.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 68 insertions(+), 16 deletions(-)
diff --git a/refs.c b/refs.c
index 0c0668b..3c20853 100644
--- a/refs.c
+++ b/refs.c
@@ -467,6 +467,57 @@ static struct ref_entry *find_ref(struct ref_dir *dir, const char *refname)
 }
 
 /*
+ * Remove the entry with the given name from dir, recursing into
+ * subdirectories as necessary.  If refname is the name of a directory
+ * (i.e., ends with '/'), then remove the directory and its contents.
+ * If the removal was successful, return the number of entries
+ * remaining in the directory entry that contained the deleted entry.
+ * If the name was not found, return -1.  Please note that this
+ * function only deletes the entry from the cache; it does not delete
+ * it from the filesystem or ensure that other cache entries (which
+ * might be symbolic references to the removed entry) are updated.
+ * Nor does it remove any containing dir entries that might be made
+ * empty by the removal.  dir must represent the top-level directory
+ * and must already be complete.
+ */
+static int remove_entry(struct ref_dir *dir, const char *refname)
+{
+	int refname_len = strlen(refname);
+	int entry_index;
+	struct ref_entry *entry;
+	int is_dir = refname[refname_len - 1] == '/';
+	if (is_dir) {
+		/*
+		 * refname represents a reference directory.  Remove
+		 * the trailing slash; otherwise we will get the
+		 * directory *representing* refname rather than the
+		 * one *containing* it.
+		 */
+		char *dirname = xmemdupz(refname, refname_len - 1);
+		dir = find_containing_dir(dir, dirname, 0);
+		free(dirname);
+	} else {
+		dir = find_containing_dir(dir, refname, 0);
+	}
+	if (!dir)
+		return -1;
+	entry_index = search_ref_dir(dir, refname, refname_len);
+	if (entry_index == -1)
+		return -1;
+	entry = dir->entries[entry_index];
+
+	memmove(&dir->entries[entry_index],
+		&dir->entries[entry_index + 1],
+		(dir->nr - entry_index - 1) * sizeof(*dir->entries)
+		);
+	dir->nr--;
+	if (dir->sorted > entry_index)
+		dir->sorted--;
+	free_ref_entry(entry);
+	return dir->nr;
+}
+
+/*
  * Add a ref_entry to the ref_dir (unsorted), recursing into
  * subdirectories as necessary.  dir must represent the top-level
  * directory.  Return 0 on success.
@@ -1892,19 +1943,12 @@ struct ref_lock *lock_any_ref_for_update(const char *refname,
 	return lock_ref_sha1_basic(refname, old_sha1, flags, NULL);
 }
 
-struct repack_without_ref_sb {
-	const char *refname;
-	int fd;
-};
-
-static int repack_without_ref_fn(struct ref_entry *entry, void *cb_data)
+static int repack_ref_fn(struct ref_entry *entry, void *cb_data)
 {
-	struct repack_without_ref_sb *data = cb_data;
+	int *fd = cb_data;
 	char line[PATH_MAX + 100];
 	int len;
 
-	if (!strcmp(data->refname, entry->name))
-		return 0;
 	/* Silently skip broken refs: */
 	if (!ref_resolves_to_object(entry, 0))
 		return 0;
@@ -1913,7 +1957,7 @@ static int repack_without_ref_fn(struct ref_entry *entry, void *cb_data)
 	/* this should not happen but just being defensive */
 	if (len > sizeof(line))
 		die("too long a refname '%s'", entry->name);
-	write_or_die(data->fd, line, len);
+	write_or_die(*fd, line, len);
 	return 0;
 }
 
@@ -1921,22 +1965,30 @@ static struct lock_file packlock;
 
 static int repack_without_ref(const char *refname)
 {
-	struct repack_without_ref_sb data;
+	int fd;
 	struct ref_cache *refs = get_ref_cache(NULL);
 	struct ref_dir *packed;
 
 	if (!get_packed_ref(refname))
 		return 0; /* refname does not exist in packed refs */
 
-	data.refname = refname;
-	data.fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
-	if (data.fd < 0) {
+	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
+	if (fd < 0) {
 		unable_to_lock_error(git_path("packed-refs"), errno);
 		return error("cannot delete '%s' from packed refs", refname);
 	}
 	clear_packed_ref_cache(refs);
 	packed = get_packed_refs(refs);
-	do_for_each_entry_in_dir(packed, 0, repack_without_ref_fn, &data);
+	/* Remove refname from the cache. */
+	if (remove_entry(packed, refname) == -1) {
+		/*
+		 * The packed entry disappeared while we were
+		 * acquiring the lock.
+		 */
+		rollback_lock_file(&packlock);
+		return 0;
+	}
+	do_for_each_entry_in_dir(packed, 0, repack_ref_fn, &fd);
 	return commit_lock_file(&packlock);
 }
 
@@ -1965,7 +2017,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 	ret |= repack_without_ref(lock->ref_name);
 
 	unlink_or_warn(git_path("logs/%s", lock->ref_name));
-	invalidate_ref_cache(NULL);
+	clear_loose_ref_cache(get_ref_cache(NULL));
 	unlock_ref(lock);
 	return ret;
 }
-- 
1.8.2.1
Previous: Michael HaggertyNext: Michael Haggerty
Message 48 of 64 in “Various cleanups around reference packing and peeling”
  1. 00/33 Various cleanups around reference packing and peelingMichael Haggerty, Apr 14, 2013
  2. 01/33 refs: document flags constants REF_*Michael Haggerty, Apr 14, 2013
  3. 02/33 refs: document the fields of struct ref_valueMichael Haggerty, Apr 14, 2013
  4. 03/33 refs: document do_for_each_ref() and do_one_ref()Michael Haggerty, Apr 14, 2013
  5. Junio C HamanoApr 15, 2013
  6. Michael HaggertyApr 16, 2013
  7. 04/33 refs: document how current_ref is usedMichael Haggerty, Apr 14, 2013
  8. 05/33 refs: define constant PEELED_LINE_LENGTHMichael Haggerty, Apr 14, 2013
  9. 06/33 do_for_each_ref_in_dirs(): remove dead codeMichael Haggerty, Apr 14, 2013
  10. 07/33 get_packed_ref(): return a ref_entryMichael Haggerty, Apr 14, 2013
  11. 08/33 peel_ref(): use function get_packed_ref()Michael Haggerty, Apr 14, 2013
  12. 09/33 repack_without_ref(): use function get_packed_ref()Michael Haggerty, Apr 14, 2013
  13. 10/33 refs: extract a function ref_resolves_to_object()Michael Haggerty, Apr 14, 2013
  14. Junio C HamanoApr 15, 2013
  15. Michael HaggertyApr 16, 2013
  16. Junio C HamanoApr 16, 2013
  17. 11/33 refs: extract function peel_object()Michael Haggerty, Apr 14, 2013
  18. 12/33 peel_object(): give more specific information in return valueMichael Haggerty, Apr 14, 2013
  19. Junio C HamanoApr 15, 2013
  20. 13/33 peel_ref(): fix return value for non-peelable, not-current referenceMichael Haggerty, Apr 14, 2013
  21. Junio C HamanoApr 15, 2013
  22. Michael HaggertyApr 16, 2013
  23. 14/33 refs: extract a function peel_entry()Michael Haggerty, Apr 14, 2013
  24. Junio C HamanoApr 15, 2013
  25. Michael HaggertyApr 16, 2013
  26. Junio C HamanoApr 16, 2013
  27. Michael HaggertyApr 16, 2013
  28. 15/33 refs: change the internal reference-iteration APIMichael Haggerty, Apr 14, 2013
  29. Junio C HamanoApr 15, 2013
  30. Michael HaggertyApr 16, 2013
  31. Junio C HamanoApr 16, 2013
  32. 16/33 t3210: test for spurious error messages for dangling packed refsMichael Haggerty, Apr 14, 2013
  33. Junio C HamanoApr 15, 2013
  34. Michael HaggertyApr 16, 2013
  35. Junio C HamanoApr 16, 2013
  36. Jeff KingApr 16, 2013
  37. Junio C HamanoApr 17, 2013
  38. Junio C HamanoApr 17, 2013
  39. 0/2 Add documentation for new expiry option valuesMichael Haggerty, Apr 18, 2013
  40. 1/2 git-gc.txt, git-reflog.txt: document new expiry optionsMichael Haggerty, Apr 18, 2013
  41. 2/2 api-parse-options.txt: document "no-" for non-boolean optionsMichael Haggerty, Apr 18, 2013
  42. prune: introduce OPT_EXPIRY_DATE() and use itJunio C Hamano, Apr 25, 2013
  43. Michael HaggertyApr 17, 2013
  44. 17/33 repack_without_ref(): silence errors for dangling packed refsMichael Haggerty, Apr 14, 2013
  45. Junio C HamanoApr 15, 2013
  46. Michael HaggertyApr 17, 2013
  47. 18/33 search_ref_dir(): return an index rather than a pointerMichael Haggerty, Apr 14, 2013
  48. 19/33 refs: change how packed refs are deletedMichael Haggerty, Apr 14, 2013
  49. 20/33 t3211: demonstrate loss of peeled refs if a packed ref is deletedMichael Haggerty, Apr 14, 2013
  50. 21/33 repack_without_ref(): write peeled refs in the rewritten fileMichael Haggerty, Apr 14, 2013
  51. Junio C HamanoApr 15, 2013
  52. 22/33 refs: extract a function write_packed_entry()Michael Haggerty, Apr 14, 2013
  53. 23/33 pack-refs: rename handle_one_ref() to pack_one_ref()Michael Haggerty, Apr 14, 2013
  54. 24/33 pack-refs: merge code from pack-refs.{c,h} into refs.{c,h}Michael Haggerty, Apr 14, 2013
  55. 25/33 pack_one_ref(): rename "path" parameter to "refname"Michael Haggerty, Apr 14, 2013
  56. 26/33 refs: use same lock_file object for both ref-packing functionsMichael Haggerty, Apr 14, 2013
  57. 27/33 pack_refs(): change to use do_for_each_entry()Michael Haggerty, Apr 14, 2013
  58. 28/33 refs: inline function do_not_prune()Michael Haggerty, Apr 14, 2013
  59. 29/33 pack_one_ref(): use function peel_entry()Michael Haggerty, Apr 14, 2013
  60. 30/33 pack_one_ref(): use write_packed_entry() to do the writingMichael Haggerty, Apr 14, 2013
  61. 31/33 pack_one_ref(): do some cheap tests before a more expensive oneMichael Haggerty, Apr 14, 2013
  62. 32/33 refs: change do_for_each_*() functions to take ref_cache argumentsMichael Haggerty, Apr 14, 2013
  63. Junio C HamanoApr 15, 2013
  64. 33/33 refs: handle the main ref_cache speciallyMichael Haggerty, Apr 14, 2013

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.