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

[PATCH 6/8] builtin/repack.c: store existing cruft packs separately

From
Taylor Blau <me@ttaylorr.com>
Date
Sep 5, 2023, 20:36 UTC
Message-ID
<414a558883830a29924710126960074b37ab97fc.1693946195.git.me@ttaylorr.com>
In-Reply-To
<cover.1693946195.git.me@ttaylorr.com>

When repacking with the `--write-midx` option, we invoke the function `midx_included_packs()` in order to produce the list of packs we want to include in the resulting MIDX.

This list is comprised of:
  - existing .keep packs
  - any pack(s) which were written earlier in the same process
  - any unchanged packs when doing a `--geometric` repack
  - any cruft packs

Prior to this patch, we stored pre-existing cruft and non-cruft packs together (provided those packs are non-kept). This meant we needed an additional bit to indicate which non-kept pack(s) were cruft versus those that aren't.

But alternatively we can store cruft packs in a separate list, avoiding the need for this extra bit, and simplifying the code below.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/repack.c | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 3f0789ff89..478fab96c9 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -27,7 +27,6 @@
 #define PACK_CRUFT 4
 
 #define DELETE_PACK 1
-#define CRUFT_PACK 2
 
 static int pack_everything;
 static int delta_base_offset = 1;
@@ -98,16 +97,18 @@ static int repack_config(const char *var, const char *value,
 struct existing_packs {
 	struct string_list kept_packs;
 	struct string_list non_kept_packs;
+	struct string_list cruft_packs;
 };
 
 #define EXISTING_PACKS_INIT { \
 	.kept_packs = STRING_LIST_INIT_DUP, \
 	.non_kept_packs = STRING_LIST_INIT_DUP, \
+	.cruft_packs = STRING_LIST_INIT_DUP, \
 }
 
 static int has_existing_non_kept_packs(const struct existing_packs *existing)
 {
-	return existing->non_kept_packs.nr;
+	return existing->non_kept_packs.nr || existing->cruft_packs.nr;
 }
 
 static void mark_packs_for_deletion_1(struct string_list *names,
@@ -138,6 +139,7 @@ static void mark_packs_for_deletion(struct existing_packs *existing,
 
 {
 	mark_packs_for_deletion_1(names, &existing->non_kept_packs);
+	mark_packs_for_deletion_1(names, &existing->cruft_packs);
 }
 
 static void remove_redundant_pack(const char *dir_name, const char *base_name)
@@ -165,12 +167,14 @@ static void remove_redundant_packs_1(struct string_list *packs)
 static void remove_redundant_existing_packs(struct existing_packs *existing)
 {
 	remove_redundant_packs_1(&existing->non_kept_packs);
+	remove_redundant_packs_1(&existing->cruft_packs);
 }
 
 static void existing_packs_release(struct existing_packs *existing)
 {
 	string_list_clear(&existing->kept_packs, 0);
 	string_list_clear(&existing->non_kept_packs, 0);
+	string_list_clear(&existing->cruft_packs, 0);
 }
 
 /*
@@ -204,12 +208,10 @@ static void collect_pack_filenames(struct existing_packs *existing,
 
 		if ((extra_keep->nr > 0 && i < extra_keep->nr) || p->pack_keep)
 			string_list_append(&existing->kept_packs, buf.buf);
-		else {
-			struct string_list_item *item;
-			item = string_list_append(&existing->non_kept_packs, buf.buf);
-			if (p->is_cruft)
-				item->util = (void*)(uintptr_t)CRUFT_PACK;
-		}
+		else if (p->is_cruft)
+			string_list_append(&existing->cruft_packs, buf.buf);
+		else
+			string_list_append(&existing->non_kept_packs, buf.buf);
 	}
 
 	string_list_sort(&existing->kept_packs);
@@ -691,14 +693,11 @@ static void midx_included_packs(struct string_list *include,
 			string_list_insert(include, strbuf_detach(&buf, NULL));
 		}
 
-		for_each_string_list_item(item, &existing->non_kept_packs) {
-			if (!((uintptr_t)item->util & CRUFT_PACK)) {
-				/*
-				 * no need to check DELETE_PACK, since we're not
-				 * doing an ALL_INTO_ONE repack
-				 */
-				continue;
-			}
+		for_each_string_list_item(item, &existing->cruft_packs) {
+			/*
+			 * no need to check DELETE_PACK, since we're not
+			 * doing an ALL_INTO_ONE repack
+			 */
 			string_list_insert(include, xstrfmt("%s.idx", item->string));
 		}
 	} else {
@@ -707,6 +706,12 @@ static void midx_included_packs(struct string_list *include,
 				continue;
 			string_list_insert(include, xstrfmt("%s.idx", item->string));
 		}
+
+		for_each_string_list_item(item, &existing->cruft_packs) {
+			if ((uintptr_t)item->util & DELETE_PACK)
+				continue;
+			string_list_insert(include, xstrfmt("%s.idx", item->string));
+		}
 	}
 }
 
@@ -836,6 +841,8 @@ static int write_cruft_pack(const struct pack_objects_args *args,
 		fprintf(in, "%s-%s.pack\n", pack_prefix, item->string);
 	for_each_string_list_item(item, &existing->non_kept_packs)
 		fprintf(in, "-%s.pack\n", item->string);
+	for_each_string_list_item(item, &existing->cruft_packs)
+		fprintf(in, "-%s.pack\n", item->string);
 	for_each_string_list_item(item, &existing->kept_packs)
 		fprintf(in, "%s.pack\n", item->string);
 	fclose(in);
-- 
2.42.0.119.gca7d13e7bf
Previous: Jeff KingNext: Jeff King
Message 13 of 39 in “repack: refactor pack snapshot-ing logic”
  1. 0/8 repack: refactor pack snapshot-ing logicTaylor Blau, Sep 5, 2023
  2. 2/8 builtin/repack.c: extract marking packs for deletionTaylor Blau, Sep 5, 2023
  3. Jeff KingSep 7, 2023
  4. Taylor BlauSep 7, 2023
  5. 5/8 builtin/repack.c: extract `has_existing_non_kept_packs()`Taylor Blau, Sep 5, 2023
  6. Jeff KingSep 7, 2023
  7. 3/8 builtin/repack.c: extract redundant pack cleanup for --geometricTaylor Blau, Sep 5, 2023
  8. Jeff KingSep 7, 2023
  9. 1/8 builtin/repack.c: extract structure to store existing packsTaylor Blau, Sep 5, 2023
  10. Jeff KingSep 7, 2023
  11. 4/8 builtin/repack.c: extract redundant pack cleanup for existing packsTaylor Blau, Sep 5, 2023
  12. Jeff KingSep 7, 2023
  13. 6/8 builtin/repack.c: store existing cruft packs separatelyTaylor Blau, Sep 5, 2023
  14. Jeff KingSep 7, 2023
  15. 7/8 builtin/repack.c: drop `DELETE_PACK` macroTaylor Blau, Sep 5, 2023
  16. Junio C HamanoSep 6, 2023
  17. Taylor BlauSep 6, 2023
  18. Patrick SteinhardtSep 7, 2023
  19. Junio C HamanoSep 7, 2023
  20. Jeff KingSep 7, 2023
  21. 8/8 builtin/repack.c: extract common cruft pack loopTaylor Blau, Sep 5, 2023
  22. Jeff KingSep 7, 2023
  23. 0/8 repack: refactor pack snapshot-ing logicTaylor Blau, Sep 13, 2023
  24. 1/8 builtin/repack.c: extract structure to store existing packsTaylor Blau, Sep 13, 2023
  25. 2/8 builtin/repack.c: extract marking packs for deletionTaylor Blau, Sep 13, 2023
  26. 3/8 builtin/repack.c: extract redundant pack cleanup for --geometricTaylor Blau, Sep 13, 2023
  27. 4/8 builtin/repack.c: extract redundant pack cleanup for existing packsTaylor Blau, Sep 13, 2023
  28. 5/8 builtin/repack.c: extract `has_existing_non_kept_packs()`Taylor Blau, Sep 13, 2023
  29. Christian CouderSep 15, 2023
  30. 6/8 builtin/repack.c: store existing cruft packs separatelyTaylor Blau, Sep 13, 2023
  31. 7/8 builtin/repack.c: avoid directly inspecting "util"Taylor Blau, Sep 13, 2023
  32. 8/8 builtin/repack.c: extract common cruft pack loopTaylor Blau, Sep 13, 2023
  33. Junio C HamanoSep 13, 2023
  34. Jeff KingSep 14, 2023
  35. Patrick SteinhardtSep 14, 2023
  36. Christian CouderSep 14, 2023
  37. Taylor BlauSep 14, 2023
  38. Christian CouderSep 15, 2023
  39. Christian CouderSep 15, 2023

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.