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

[PATCH 01/12] repack_without_ref(): split list curation and entry writing

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Jun 11, 2013, 21:48 UTC
Message-ID
<1370987312-6761-2-git-send-email-mhagger@alum.mit.edu>
In-Reply-To
<1370987312-6761-1-git-send-email-mhagger@alum.mit.edu>
Split repack_without_ref() into multiple passes:
* collect the list of refnames that should be deleted from packed_refs
* delete those refnames from the cache
* write the remainder to the packed-refs file

The purpose of this change is to make the "write the remainder" part reusable.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c | 48 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/refs.c b/refs.c
index d17931a..dccfe14 100644
--- a/refs.c
+++ b/refs.c
@@ -1994,6 +1994,23 @@ static void write_packed_entry(int fd, char *refname, unsigned char *sha1,
 	}
 }
 
+/*
+ * An each_ref_entry_fn that writes the entry to a packed-refs file.
+ */
+static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
+{
+	int *fd = cb_data;
+	enum peel_status peel_status = peel_entry(entry, 0);
+
+	if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
+		error("internal error: %s is not a valid packed reference!",
+		      entry->name);
+	write_packed_entry(*fd, entry->name, entry->u.value.sha1,
+			   peel_status == PEEL_PEELED ?
+			   entry->u.value.peeled : NULL);
+	return 0;
+}
+
 struct ref_to_prune {
 	struct ref_to_prune *next;
 	unsigned char sha1[20];
@@ -2113,14 +2130,18 @@ int pack_refs(unsigned int flags)
 	return 0;
 }
 
-static int repack_ref_fn(struct ref_entry *entry, void *cb_data)
+/*
+ * If entry should be deleted from packed-refs, add it to the string
+ * list pointed to by cb_data.
+ */
+static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)
 {
-	int *fd = cb_data;
-	enum peel_status peel_status;
+	struct string_list *refs_to_delete = cb_data;
 
 	if (entry->flag & REF_ISBROKEN) {
 		/* This shouldn't happen to packed refs. */
 		error("%s is broken!", entry->name);
+		string_list_append(refs_to_delete, entry->name);
 		return 0;
 	}
 	if (!has_sha1_file(entry->u.value.sha1)) {
@@ -2130,7 +2151,7 @@ static int repack_ref_fn(struct ref_entry *entry, void *cb_data)
 		if (read_ref_full(entry->name, sha1, 0, &flags))
 			/* We should at least have found the packed ref. */
 			die("Internal error");
-		if ((flags & REF_ISSYMREF) || !(flags & REF_ISPACKED))
+		if ((flags & REF_ISSYMREF) || !(flags & REF_ISPACKED)) {
 			/*
 			 * This packed reference is overridden by a
 			 * loose reference, so it is OK that its value
@@ -2139,9 +2160,11 @@ static int repack_ref_fn(struct ref_entry *entry, void *cb_data)
 			 * collected.  For this purpose we don't even
 			 * care whether the loose reference itself is
 			 * invalid, broken, symbolic, etc.  Silently
-			 * omit the packed reference from the output.
+			 * remove the packed reference.
 			 */
+			string_list_append(refs_to_delete, entry->name);
 			return 0;
+		}
 		/*
 		 * There is no overriding loose reference, so the fact
 		 * that this reference doesn't refer to a valid object
@@ -2150,14 +2173,10 @@ static int repack_ref_fn(struct ref_entry *entry, void *cb_data)
 		 * the output.
 		 */
 		error("%s does not point to a valid object!", entry->name);
+		string_list_append(refs_to_delete, entry->name);
 		return 0;
 	}
 
-	peel_status = peel_entry(entry, 0);
-	write_packed_entry(*fd, entry->name, entry->u.value.sha1,
-			   peel_status == PEEL_PEELED ?
-			   entry->u.value.peeled : NULL);
-
 	return 0;
 }
 
@@ -2165,6 +2184,8 @@ static int repack_without_ref(const char *refname)
 {
 	int fd;
 	struct ref_dir *packed;
+	struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
+	struct string_list_item *ref_to_delete;
 
 	if (!get_packed_ref(refname))
 		return 0; /* refname does not exist in packed refs */
@@ -2185,8 +2206,13 @@ static int repack_without_ref(const char *refname)
 		rollback_lock_file(&packlock);
 		return 0;
 	}
+	do_for_each_entry_in_dir(packed, 0, curate_packed_ref_fn, &refs_to_delete);
+	for_each_string_list_item(ref_to_delete, &refs_to_delete) {
+		if (remove_entry(packed, ref_to_delete->string) == -1)
+			die("internal error");
+	}
 	write_or_die(fd, PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
-	do_for_each_entry_in_dir(packed, 0, repack_ref_fn, &fd);
+	do_for_each_entry_in_dir(packed, 0, write_packed_entry_fn, &fd);
 	return commit_lock_file(&packlock);
 }
 
-- 
1.8.3
Previous: Michael HaggertyNext: Jeff King
Message 2 of 22 in “Fix some reference-related races”
  1. 00/12 Fix some reference-related racesMichael Haggerty, Jun 11, 2013
  2. 01/12 repack_without_ref(): split list curation and entry writingMichael Haggerty, Jun 11, 2013
  3. Jeff KingJun 12, 2013
  4. Michael HaggertyJun 12, 2013
  5. 02/12 pack_refs(): split creation of packed refs and entry writingMichael Haggerty, Jun 11, 2013
  6. 03/12 refs: wrap the packed refs cache in a level of indirectionMichael Haggerty, Jun 11, 2013
  7. 04/12 refs: implement simple transactions for the packed-refs fileMichael Haggerty, Jun 11, 2013
  8. Jeff KingJun 12, 2013
  9. 05/12 refs: manage lifetime of packed refs cache via reference countingMichael Haggerty, Jun 11, 2013
  10. 06/12 do_for_each_entry(): increment the packed refs cache refcountMichael Haggerty, Jun 11, 2013
  11. 07/12 packed_ref_cache: increment refcount when lockedMichael Haggerty, Jun 11, 2013
  12. 08/12 Extract a struct stat_data from cache_entryMichael Haggerty, Jun 11, 2013
  13. 09/12 add a stat_validity structMichael Haggerty, Jun 11, 2013
  14. 10/12 get_packed_ref_cache: reload packed-refs file when it changesMichael Haggerty, Jun 11, 2013
  15. 11/12 for_each_ref: load all loose refs before packed refsMichael Haggerty, Jun 11, 2013
  16. 12/12 refs: do not invalidate the packed-refs cache unnecessarilyMichael Haggerty, Jun 11, 2013
  17. Jeff KingJun 12, 2013
  18. Jeff KingJun 12, 2013
  19. Ramsay JonesJun 15, 2013
  20. Michael HaggertyJun 16, 2013
  21. Ramsay JonesJun 18, 2013
  22. Michael HaggertyJun 19, 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.