git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Patrick Steinhardt <ps@pks.im>
Subject: [PATCH v2 2/8] builtin/repack.c: extract marking packs for deletion
Date: Wed, 13 Sep 2023 15:17:46 -0400	[thread overview]
Message-ID: <62d916169d33a7569d83b72dfb1cb90d56568f01.1694632644.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1694632644.git.me@ttaylorr.com>

At the end of a repack (when given `-d`), Git attempts to remove any
packs which have been made "redundant" as a result of the repacking
operation. For example, an all-into-one (`-A` or `-a`) repack makes
every pre-existing pack which is not marked as kept redundant. Geometric
repacks (with `--geometric=<n>`) make any packs which were rolled up
redundant, and so on.

But before deleting the set of packs we think are redundant, we first
check to see whether or not we just wrote a pack which is identical to
any one of the packs we were going to delete. When this is the case, Git
must avoid deleting that pack, since it matches a pack we just wrote
(so deleting it may cause the repository to become corrupt).

Right now we only process the list of non-kept packs in a single pass.
But a future change will split the existing non-kept packs further into
two lists: one for cruft packs, and another for non-cruft packs.

Factor out this routine to prepare for calling it twice on two separate
lists in a future patch.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 builtin/repack.c | 50 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index 67bf86567f..0eee430951 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -105,6 +105,36 @@ struct existing_packs {
 	.non_kept_packs = STRING_LIST_INIT_DUP, \
 }
 
+static void mark_packs_for_deletion_1(struct string_list *names,
+				      struct string_list *list)
+{
+	struct string_list_item *item;
+	const int hexsz = the_hash_algo->hexsz;
+
+	for_each_string_list_item(item, list) {
+		char *sha1;
+		size_t len = strlen(item->string);
+		if (len < hexsz)
+			continue;
+		sha1 = item->string + len - hexsz;
+		/*
+		 * Mark this pack for deletion, which ensures that this
+		 * pack won't be included in a MIDX (if `--write-midx`
+		 * was given) and that we will actually delete this pack
+		 * (if `-d` was given).
+		 */
+		if (!string_list_has_string(names, sha1))
+			item->util = (void*)(uintptr_t)((size_t)item->util | DELETE_PACK);
+	}
+}
+
+static void mark_packs_for_deletion(struct existing_packs *existing,
+				    struct string_list *names)
+
+{
+	mark_packs_for_deletion_1(names, &existing->non_kept_packs);
+}
+
 static void existing_packs_release(struct existing_packs *existing)
 {
 	string_list_clear(&existing->kept_packs, 0);
@@ -1142,24 +1172,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 	}
 	/* End of pack replacement. */
 
-	if (delete_redundant && pack_everything & ALL_INTO_ONE) {
-		const int hexsz = the_hash_algo->hexsz;
-		for_each_string_list_item(item, &existing.non_kept_packs) {
-			char *sha1;
-			size_t len = strlen(item->string);
-			if (len < hexsz)
-				continue;
-			sha1 = item->string + len - hexsz;
-			/*
-			 * Mark this pack for deletion, which ensures that this
-			 * pack won't be included in a MIDX (if `--write-midx`
-			 * was given) and that we will actually delete this pack
-			 * (if `-d` was given).
-			 */
-			if (!string_list_has_string(&names, sha1))
-				item->util = (void*)(uintptr_t)((size_t)item->util | DELETE_PACK);
-		}
-	}
+	if (delete_redundant && pack_everything & ALL_INTO_ONE)
+		mark_packs_for_deletion(&existing, &names);
 
 	if (write_midx) {
 		struct string_list include = STRING_LIST_INIT_NODUP;
-- 
2.42.0.166.g68748eb9c8


  parent reply	other threads:[~2023-09-13 19:17 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-05 20:36 [PATCH 0/8] repack: refactor pack snapshot-ing logic Taylor Blau
2023-09-05 20:36 ` [PATCH 1/8] builtin/repack.c: extract structure to store existing packs Taylor Blau
2023-09-07  7:54   ` Jeff King
2023-09-05 20:36 ` [PATCH 2/8] builtin/repack.c: extract marking packs for deletion Taylor Blau
2023-09-07  7:59   ` Jeff King
2023-09-07 22:10     ` Taylor Blau
2023-09-05 20:36 ` [PATCH 3/8] builtin/repack.c: extract redundant pack cleanup for --geometric Taylor Blau
2023-09-07  8:00   ` Jeff King
2023-09-05 20:36 ` [PATCH 4/8] builtin/repack.c: extract redundant pack cleanup for existing packs Taylor Blau
2023-09-07  8:02   ` Jeff King
2023-09-05 20:36 ` [PATCH 5/8] builtin/repack.c: extract `has_existing_non_kept_packs()` Taylor Blau
2023-09-07  8:04   ` Jeff King
2023-09-05 20:36 ` [PATCH 6/8] builtin/repack.c: store existing cruft packs separately Taylor Blau
2023-09-07  8:09   ` Jeff King
2023-09-05 20:36 ` [PATCH 7/8] builtin/repack.c: drop `DELETE_PACK` macro Taylor Blau
2023-09-06 17:05   ` Junio C Hamano
2023-09-06 17:22     ` Taylor Blau
2023-09-07  8:19       ` Patrick Steinhardt
2023-09-07 18:16         ` Junio C Hamano
2023-09-07  8:58       ` Jeff King
2023-09-05 20:37 ` [PATCH 8/8] builtin/repack.c: extract common cruft pack loop Taylor Blau
2023-09-07  9:00 ` [PATCH 0/8] repack: refactor pack snapshot-ing logic Jeff King
2023-09-13 19:17 ` [PATCH v2 " Taylor Blau
2023-09-13 19:17   ` [PATCH v2 1/8] builtin/repack.c: extract structure to store existing packs Taylor Blau
2023-09-13 19:17   ` Taylor Blau [this message]
2023-09-13 19:17   ` [PATCH v2 3/8] builtin/repack.c: extract redundant pack cleanup for --geometric Taylor Blau
2023-09-13 19:17   ` [PATCH v2 4/8] builtin/repack.c: extract redundant pack cleanup for existing packs Taylor Blau
2023-09-13 19:17   ` [PATCH v2 5/8] builtin/repack.c: extract `has_existing_non_kept_packs()` Taylor Blau
2023-09-15 10:02     ` Christian Couder
2023-09-13 19:17   ` [PATCH v2 6/8] builtin/repack.c: store existing cruft packs separately Taylor Blau
2023-09-13 19:18   ` [PATCH v2 7/8] builtin/repack.c: avoid directly inspecting "util" Taylor Blau
2023-09-13 19:18   ` [PATCH v2 8/8] builtin/repack.c: extract common cruft pack loop Taylor Blau
2023-09-13 19:44   ` [PATCH v2 0/8] repack: refactor pack snapshot-ing logic Junio C Hamano
2023-09-14  0:07     ` Jeff King
2023-09-14 10:33       ` Patrick Steinhardt
2023-09-14 11:10     ` Christian Couder
2023-09-14 18:01       ` Taylor Blau
2023-09-15  5:56         ` Christian Couder
2023-09-15 10:09   ` Christian Couder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=62d916169d33a7569d83b72dfb1cb90d56568f01.1694632644.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).