From patchwork Wed Sep 13 19:17:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13383698 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CDE36EE020A for ; Wed, 13 Sep 2023 19:17:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232036AbjIMTRu (ORCPT ); Wed, 13 Sep 2023 15:17:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34468 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230311AbjIMTRs (ORCPT ); Wed, 13 Sep 2023 15:17:48 -0400 Received: from mail-yb1-xb29.google.com (mail-yb1-xb29.google.com [IPv6:2607:f8b0:4864:20::b29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BE7F3170F for ; Wed, 13 Sep 2023 12:17:44 -0700 (PDT) Received: by mail-yb1-xb29.google.com with SMTP id 3f1490d57ef6-d7b9de8139fso198241276.1 for ; Wed, 13 Sep 2023 12:17:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20230601.gappssmtp.com; s=20230601; t=1694632664; x=1695237464; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=9/zBrG9vaXf0P4savWUkFGQJmWWw6bQFT/Un8wPq6MY=; b=VP60ub/CXVpkfy+bZsrOd3vZdWMIZhMB4K3WWr7xJDzHbBGPy+suKiLHsRpEIA8F/n lMOBgOS0DdqTdbj8/o0bcg1Rmkdl7b4zj1zckAzgfmC/3OyozbkNi7oEjhdtFRk4FE1U 6Xl1XEpMuum/nsOjlW6NRnKeL/F17SibXUVW4kInOEiJFEBTAQ129FMi6TC+8Pk/diMx WopCa6Bt/oN0BlUfu+m1o+9eC3PtZ5SCuH5WWlKTmSI9XmTSVWyltC4CqhSC9YQ+FwgN fyIP/Nef2Tx+v/kVA/2W8P1wLkinPZKYYw54+XPwXzAlZ8+CosuNyZttJTWZX1k7jUbM mBng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694632664; x=1695237464; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=9/zBrG9vaXf0P4savWUkFGQJmWWw6bQFT/Un8wPq6MY=; b=rkuxBnEdxXyKP5GOFzJ7tjLXBQ/42OD9xq4RMPnKq7LBKNQEHjBRUsBuOyToUb205g ABhnDzOlLW16Kckrerf8P5NFnhGzYOytYFz8PCk2jagJmlwOnlPMPMf5BnZPWFunGDLv lRb7PFoiy2lssaOv0Cq76FMswxXZaBoEjAbP5ZfsgrTyltHZqdkkUMUBsEqKjEty6FGw /B+fENybG01n9gmlfBhG6xffESGVEkXMCCeTvaHxo+v9OIOQNMF9fDRbrM/XcJWYfK/O 7VZZ/1mxibUwXKw1WqsnioQZYf7VGTFKt4KMaZLMdd1LPRpZ0FdyPt195Ak+4ZF8dzSx lpvA== X-Gm-Message-State: AOJu0Yz43gq0FVZMAJv/yMPvWWiXInOql6hGki0mDR0VKQAOFqmeSOnp gzHXfWLXgYYsDdR1wzaoj9sEHC8w7cMTkXRvq2O2eA== X-Google-Smtp-Source: AGHT+IHe/GdCplNXLk+pSSBln5RjENV8uIQ6pfluSVyXv2TZx0F2NE7TOWq/3mvjI0qDZq/THnppJg== X-Received: by 2002:a25:be8b:0:b0:d74:6188:19e7 with SMTP id i11-20020a25be8b000000b00d74618819e7mr3250936ybk.35.1694632663528; Wed, 13 Sep 2023 12:17:43 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id l7-20020a252507000000b00d7badcab84esm2883130ybl.9.2023.09.13.12.17.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 12:17:43 -0700 (PDT) Date: Wed, 13 Sep 2023 15:17:35 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Patrick Steinhardt Subject: [PATCH v2 1/8] builtin/repack.c: extract structure to store existing packs Message-ID: <2e26beff22414282ecbbe846113d55845c7d0df8.1694632644.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The repack machinery needs to keep track of which packfiles were present in the repository at the beginning of a repack, segmented by whether or not each pack is marked as kept. The names of these packs are stored in two `string_list`s, corresponding to kept- and non-kept packs, respectively. As a consequence, many functions within the repack code need to take both `string_list`s as arguments, leading to code like this: ret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix, cruft_expiration, &names, &existing_nonkept_packs, /* <- */ &existing_kept_packs); /* <- */ Wrap up this pair of `string_list`s into a single structure that stores both. This saves us from having to pass both string lists separately, and prepares for adding additional fields to this structure. Signed-off-by: Taylor Blau --- builtin/repack.c | 90 ++++++++++++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 41 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index 6943c5ba11..67bf86567f 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -95,14 +95,29 @@ static int repack_config(const char *var, const char *value, return git_default_config(var, value, ctx, cb); } +struct existing_packs { + struct string_list kept_packs; + struct string_list non_kept_packs; +}; + +#define EXISTING_PACKS_INIT { \ + .kept_packs = STRING_LIST_INIT_DUP, \ + .non_kept_packs = STRING_LIST_INIT_DUP, \ +} + +static void existing_packs_release(struct existing_packs *existing) +{ + string_list_clear(&existing->kept_packs, 0); + string_list_clear(&existing->non_kept_packs, 0); +} + /* - * Adds all packs hex strings (pack-$HASH) to either fname_nonkept_list - * or fname_kept_list based on whether each pack has a corresponding + * Adds all packs hex strings (pack-$HASH) to either packs->non_kept + * or packs->kept based on whether each pack has a corresponding * .keep file or not. Packs without a .keep file are not to be kept * if we are going to pack everything into one file. */ -static void collect_pack_filenames(struct string_list *fname_nonkept_list, - struct string_list *fname_kept_list, +static void collect_pack_filenames(struct existing_packs *existing, const struct string_list *extra_keep) { struct packed_git *p; @@ -126,16 +141,16 @@ static void collect_pack_filenames(struct string_list *fname_nonkept_list, strbuf_strip_suffix(&buf, ".pack"); if ((extra_keep->nr > 0 && i < extra_keep->nr) || p->pack_keep) - string_list_append(fname_kept_list, buf.buf); + string_list_append(&existing->kept_packs, buf.buf); else { struct string_list_item *item; - item = string_list_append(fname_nonkept_list, buf.buf); + item = string_list_append(&existing->non_kept_packs, buf.buf); if (p->is_cruft) item->util = (void*)(uintptr_t)CRUFT_PACK; } } - string_list_sort(fname_kept_list); + string_list_sort(&existing->kept_packs); strbuf_release(&buf); } @@ -327,7 +342,7 @@ static int geometry_cmp(const void *va, const void *vb) } static void init_pack_geometry(struct pack_geometry *geometry, - struct string_list *existing_kept_packs, + struct existing_packs *existing, const struct pack_objects_args *args) { struct packed_git *p; @@ -344,23 +359,24 @@ static void init_pack_geometry(struct pack_geometry *geometry, if (!pack_kept_objects) { /* - * Any pack that has its pack_keep bit set will appear - * in existing_kept_packs below, but this saves us from - * doing a more expensive check. + * Any pack that has its pack_keep bit set will + * appear in existing->kept_packs below, but + * this saves us from doing a more expensive + * check. */ if (p->pack_keep) continue; /* - * The pack may be kept via the --keep-pack option; - * check 'existing_kept_packs' to determine whether to - * ignore it. + * The pack may be kept via the --keep-pack + * option; check 'existing->kept_packs' to + * determine whether to ignore it. */ strbuf_reset(&buf); strbuf_addstr(&buf, pack_basename(p)); strbuf_strip_suffix(&buf, ".pack"); - if (string_list_has_string(existing_kept_packs, buf.buf)) + if (string_list_has_string(&existing->kept_packs, buf.buf)) continue; } if (p->is_cruft) @@ -565,14 +581,13 @@ static void midx_snapshot_refs(struct tempfile *f) } static void midx_included_packs(struct string_list *include, - struct string_list *existing_nonkept_packs, - struct string_list *existing_kept_packs, + struct existing_packs *existing, struct string_list *names, struct pack_geometry *geometry) { struct string_list_item *item; - for_each_string_list_item(item, existing_kept_packs) + for_each_string_list_item(item, &existing->kept_packs) string_list_insert(include, xstrfmt("%s.idx", item->string)); for_each_string_list_item(item, names) string_list_insert(include, xstrfmt("pack-%s.idx", item->string)); @@ -600,7 +615,7 @@ static void midx_included_packs(struct string_list *include, string_list_insert(include, strbuf_detach(&buf, NULL)); } - for_each_string_list_item(item, existing_nonkept_packs) { + 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 @@ -611,7 +626,7 @@ static void midx_included_packs(struct string_list *include, string_list_insert(include, xstrfmt("%s.idx", item->string)); } } else { - for_each_string_list_item(item, existing_nonkept_packs) { + for_each_string_list_item(item, &existing->non_kept_packs) { if ((uintptr_t)item->util & DELETE_PACK) continue; string_list_insert(include, xstrfmt("%s.idx", item->string)); @@ -700,8 +715,7 @@ static int write_cruft_pack(const struct pack_objects_args *args, const char *pack_prefix, const char *cruft_expiration, struct string_list *names, - struct string_list *existing_packs, - struct string_list *existing_kept_packs) + struct existing_packs *existing) { struct child_process cmd = CHILD_PROCESS_INIT; struct strbuf line = STRBUF_INIT; @@ -743,9 +757,9 @@ static int write_cruft_pack(const struct pack_objects_args *args, in = xfdopen(cmd.in, "w"); for_each_string_list_item(item, names) fprintf(in, "%s-%s.pack\n", pack_prefix, item->string); - for_each_string_list_item(item, existing_packs) + for_each_string_list_item(item, &existing->non_kept_packs) fprintf(in, "-%s.pack\n", item->string); - for_each_string_list_item(item, existing_kept_packs) + for_each_string_list_item(item, &existing->kept_packs) fprintf(in, "%s.pack\n", item->string); fclose(in); @@ -777,8 +791,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) struct child_process cmd = CHILD_PROCESS_INIT; struct string_list_item *item; struct string_list names = STRING_LIST_INIT_DUP; - struct string_list existing_nonkept_packs = STRING_LIST_INIT_DUP; - struct string_list existing_kept_packs = STRING_LIST_INIT_DUP; + struct existing_packs existing = EXISTING_PACKS_INIT; struct pack_geometry geometry = { 0 }; struct strbuf line = STRBUF_INIT; struct tempfile *refs_snapshot = NULL; @@ -914,13 +927,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix) packtmp_name = xstrfmt(".tmp-%d-pack", (int)getpid()); packtmp = mkpathdup("%s/%s", packdir, packtmp_name); - collect_pack_filenames(&existing_nonkept_packs, &existing_kept_packs, - &keep_pack_list); + collect_pack_filenames(&existing, &keep_pack_list); if (geometry.split_factor) { if (pack_everything) die(_("options '%s' and '%s' cannot be used together"), "--geometric", "-A/-a"); - init_pack_geometry(&geometry, &existing_kept_packs, &po_args); + init_pack_geometry(&geometry, &existing, &po_args); split_pack_geometry(&geometry); } @@ -964,7 +976,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (pack_everything & ALL_INTO_ONE) { repack_promisor_objects(&po_args, &names); - if (existing_nonkept_packs.nr && delete_redundant && + if (existing.non_kept_packs.nr && delete_redundant && !(pack_everything & PACK_CRUFT)) { for_each_string_list_item(item, &names) { strvec_pushf(&cmd.args, "--keep-pack=%s-%s.pack", @@ -1055,8 +1067,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) ret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix, cruft_expiration, &names, - &existing_nonkept_packs, - &existing_kept_packs); + &existing); if (ret) goto cleanup; @@ -1087,8 +1098,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) pack_prefix, NULL, &names, - &existing_nonkept_packs, - &existing_kept_packs); + &existing); if (ret) goto cleanup; } @@ -1134,7 +1144,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (delete_redundant && pack_everything & ALL_INTO_ONE) { const int hexsz = the_hash_algo->hexsz; - for_each_string_list_item(item, &existing_nonkept_packs) { + for_each_string_list_item(item, &existing.non_kept_packs) { char *sha1; size_t len = strlen(item->string); if (len < hexsz) @@ -1153,8 +1163,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (write_midx) { struct string_list include = STRING_LIST_INIT_NODUP; - midx_included_packs(&include, &existing_nonkept_packs, - &existing_kept_packs, &names, &geometry); + midx_included_packs(&include, &existing, &names, &geometry); ret = write_midx_included_packs(&include, &geometry, refs_snapshot ? get_tempfile_path(refs_snapshot) : NULL, @@ -1173,7 +1182,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (delete_redundant) { int opts = 0; - for_each_string_list_item(item, &existing_nonkept_packs) { + for_each_string_list_item(item, &existing.non_kept_packs) { if (!((uintptr_t)item->util & DELETE_PACK)) continue; remove_redundant_pack(packdir, item->string); @@ -1194,7 +1203,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) strbuf_strip_suffix(&buf, ".pack"); if ((p->pack_keep) || - (string_list_has_string(&existing_kept_packs, + (string_list_has_string(&existing.kept_packs, buf.buf))) continue; @@ -1225,8 +1234,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) cleanup: string_list_clear(&names, 1); - string_list_clear(&existing_nonkept_packs, 0); - string_list_clear(&existing_kept_packs, 0); + existing_packs_release(&existing); free_pack_geometry(&geometry); return ret; From patchwork Wed Sep 13 19:17:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13383699 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 27D8DEE020B for ; Wed, 13 Sep 2023 19:17:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232090AbjIMTRx (ORCPT ); Wed, 13 Sep 2023 15:17:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34490 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230311AbjIMTRw (ORCPT ); Wed, 13 Sep 2023 15:17:52 -0400 Received: from mail-yw1-x112e.google.com (mail-yw1-x112e.google.com [IPv6:2607:f8b0:4864:20::112e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3EC15170F for ; Wed, 13 Sep 2023 12:17:48 -0700 (PDT) Received: by mail-yw1-x112e.google.com with SMTP id 00721157ae682-592976e5b6dso1841407b3.2 for ; Wed, 13 Sep 2023 12:17:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20230601.gappssmtp.com; s=20230601; t=1694632667; x=1695237467; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=4Q8EDzEHrVv485b+ivCzs2soqfK0oEq7Q7hDwtxBeGo=; b=df7U2+mIh8NTZqzGiI/yJS9RDqzTirnSJAVNei6Oj0qAr3I1hIwyRx2HHp1OXCZJO2 hYiEXK4eJSDyq/hVsxsH9uUnRftcI78TbSEXQnUP8WfFZVv+mC/4O4bf/lBwhoH9d+Oy 56h6ZBl6lYCLfrj37SKs5uCZNBTabstAItq48QIOW0lYIpkzrJOlBuPfwTyGno3Kwr4T zLXXpxMwbOW7+x0v60+z+/y/9VPqY7TL6qugNtUy1M+zKeTJlskNtjaMhY4+u+9YymoG bAjEFXRlpsCwqWO3iczl32rIgh0eKu7ZzTIxa8KOCDcmOxnv1oBeMz92VMg8relPPlnp fiZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694632667; x=1695237467; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=4Q8EDzEHrVv485b+ivCzs2soqfK0oEq7Q7hDwtxBeGo=; b=inlaXLAMQwtsm0ln0uI8Sau/MewYvnd2rljL6EjTj8X0lL9wsrXpoxHRxs1EKtRQHB 6VV7aizS18gIPkxo3LXw6jc902rkEmdr0I5d7kiIpGYTw3OXiNNERMnxBjxj9BcawN5a I+qD1k/bLqkvcKeTiS7DnK/Rp4V29UH0+hezmi8LtnN9HZaS7lSC8J4oSPMWKypneRnU Pkpkiqzc6hYGjCka3lSMFCmMxX/mkESUoh3pyGzARWpGs4d95uBFpOdXQGtpu3xV3a8w w2tq3xZ2kpbshAGWWO0TobX4OZeQn/O66p07Gp/qZtzPY1bspPpY4kutl4/IH3A8uJSo D4Cg== X-Gm-Message-State: AOJu0YwZKr2ZH4x0M6FofKX4VC/o2GZ0ncqsP4BTNPMyKNfcjXG22Ban LU6EDSXHGVfWRlvlAmI04GY3Mxjq7f50+ORizLVGqQ== X-Google-Smtp-Source: AGHT+IHgVcZMifWSurEDDobaULahvwPBYefgoj1Xlck4KzENP10mRIMofyLDug6TOiqI5wbFbBZ4TQ== X-Received: by 2002:a0d:ef86:0:b0:59b:2458:f608 with SMTP id y128-20020a0def86000000b0059b2458f608mr3713386ywe.30.1694632667303; Wed, 13 Sep 2023 12:17:47 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id x83-20020a0dd556000000b0058c4e33b2d6sm3295731ywd.90.2023.09.13.12.17.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 12:17:47 -0700 (PDT) Date: Wed, 13 Sep 2023 15:17:46 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Patrick Steinhardt Subject: [PATCH v2 2/8] builtin/repack.c: extract marking packs for deletion Message-ID: <62d916169d33a7569d83b72dfb1cb90d56568f01.1694632644.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org 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=`) 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 --- 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; From patchwork Wed Sep 13 19:17:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13383700 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7AB68EE0207 for ; Wed, 13 Sep 2023 19:17:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232098AbjIMTR6 (ORCPT ); Wed, 13 Sep 2023 15:17:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36592 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232071AbjIMTRz (ORCPT ); Wed, 13 Sep 2023 15:17:55 -0400 Received: from mail-yw1-x112d.google.com (mail-yw1-x112d.google.com [IPv6:2607:f8b0:4864:20::112d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0EF9B199F for ; Wed, 13 Sep 2023 12:17:51 -0700 (PDT) Received: by mail-yw1-x112d.google.com with SMTP id 00721157ae682-59bbbeea69fso2107677b3.0 for ; Wed, 13 Sep 2023 12:17:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20230601.gappssmtp.com; s=20230601; t=1694632670; x=1695237470; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=GCbI8tom2x+w7tUloBiTiRgIwhdRrRfsvucxp79vtW4=; b=tgcHaKV1CC7wyCwNhw8AicGu4OpakbqvzSrPw9QmesaiCd4+hQgeHR0Uz/QJoTY34h hgS7WZon20j87PUFKC1gWMlRgpWCP8Csb2qbtGMTd4xzRaclB7+9JpDg7eO3TiOnQQOK qo1adhAqjY7bd+VjFgzxAz7RIso1WTr1gh+r/sytv/mIP5JvRBttfOUqTKieXz3wiZX8 gcICk6N6yV0OcI3+0caOGCJyg8XHCxP5pwUOZXtNk40smuwYz3RUzVwL2QM79GEqemou fWGg4GGS/T4+bFq/sescBrPdtmJWKZwjtwjMJ7zWiPLVEj67rs8BOZnHKTAcYKnLMQdD eZsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694632670; x=1695237470; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=GCbI8tom2x+w7tUloBiTiRgIwhdRrRfsvucxp79vtW4=; b=ZJJXdYL/ML0ztaAFZVezvwofLM00EjMChBoBrVTDQBGgHHpW2x9N54ZnH9I6EmA+uS z7nS9N8zWuOZgMbySkgkE3elAunn+Vp96C7CRGv6Lqa4//vVav12xiZDWqhaARR4yj+M e3IV/AZttrtLi9A/my1zTM5pGcj3JhO49+YJiYvLMYjAy6TkptGg9LZwqZnd+wn8TIla KnAw9RSztOEmJ1jFnExetuBBljcbB/oOdOk0MYUpokszwEy+wRv58j909lwsyz39q7RO mf3rxzWbRLDUKs+4C5SuI4pWwhFK3tvZERhUKXkQSa12LzJOptfvRdKGMs/YaXML8vKI mNRA== X-Gm-Message-State: AOJu0YytX2dCJc7DfHdVNJP6fKgv66jUxU2dMhqc6ebagwSZAWdvgiM6 cxWh15pf5sLRAitHFctm25qUVipCU+FbycYtFGlUcA== X-Google-Smtp-Source: AGHT+IEYLuvYlOliX2JzG/k+E/7Vtot3+9u0lmD19q37dWSyHJoVIp0fDogRhqY59YbvY9c0PPV/dg== X-Received: by 2002:a0d:e2c3:0:b0:576:7dfc:e73e with SMTP id l186-20020a0de2c3000000b005767dfce73emr3946795ywe.32.1694632670104; Wed, 13 Sep 2023 12:17:50 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id q62-20020a0dce41000000b005463e45458bsm3271191ywd.123.2023.09.13.12.17.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 12:17:49 -0700 (PDT) Date: Wed, 13 Sep 2023 15:17:49 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Patrick Steinhardt Subject: [PATCH v2 3/8] builtin/repack.c: extract redundant pack cleanup for --geometric Message-ID: <7ed45804ea02cdb709de1776daac857425f744fd.1694632644.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org To reduce the complexity of the already quite-long `cmd_repack()` implementation, extract out the parts responsible for deleting redundant packs from a geometric repack out into its own sub-routine. Signed-off-by: Taylor Blau --- builtin/repack.c | 52 +++++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index 0eee430951..71366811e9 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -540,6 +540,32 @@ static struct packed_git *get_preferred_pack(struct pack_geometry *geometry) return NULL; } +static void geometry_remove_redundant_packs(struct pack_geometry *geometry, + struct string_list *names, + struct existing_packs *existing) +{ + struct strbuf buf = STRBUF_INIT; + uint32_t i; + + for (i = 0; i < geometry->split; i++) { + struct packed_git *p = geometry->pack[i]; + if (string_list_has_string(names, hash_to_hex(p->hash))) + continue; + + strbuf_reset(&buf); + strbuf_addstr(&buf, pack_basename(p)); + strbuf_strip_suffix(&buf, ".pack"); + + if ((p->pack_keep) || + (string_list_has_string(&existing->kept_packs, buf.buf))) + continue; + + remove_redundant_pack(packdir, buf.buf); + } + + strbuf_release(&buf); +} + static void free_pack_geometry(struct pack_geometry *geometry) { if (!geometry) @@ -1202,29 +1228,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix) remove_redundant_pack(packdir, item->string); } - if (geometry.split_factor) { - struct strbuf buf = STRBUF_INIT; - - uint32_t i; - for (i = 0; i < geometry.split; i++) { - struct packed_git *p = geometry.pack[i]; - if (string_list_has_string(&names, - hash_to_hex(p->hash))) - continue; - - strbuf_reset(&buf); - strbuf_addstr(&buf, pack_basename(p)); - strbuf_strip_suffix(&buf, ".pack"); - - if ((p->pack_keep) || - (string_list_has_string(&existing.kept_packs, - buf.buf))) - continue; - - remove_redundant_pack(packdir, buf.buf); - } - strbuf_release(&buf); - } + if (geometry.split_factor) + geometry_remove_redundant_packs(&geometry, &names, + &existing); if (show_progress) opts |= PRUNE_PACKED_VERBOSE; prune_packed_objects(opts); From patchwork Wed Sep 13 19:17:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13383701 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BF109EE020A for ; Wed, 13 Sep 2023 19:17:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232119AbjIMTR7 (ORCPT ); Wed, 13 Sep 2023 15:17:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36668 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232157AbjIMTR5 (ORCPT ); Wed, 13 Sep 2023 15:17:57 -0400 Received: from mail-yw1-x112e.google.com (mail-yw1-x112e.google.com [IPv6:2607:f8b0:4864:20::112e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B6F3B170F for ; Wed, 13 Sep 2023 12:17:53 -0700 (PDT) Received: by mail-yw1-x112e.google.com with SMTP id 00721157ae682-5925e580f12so1855137b3.3 for ; Wed, 13 Sep 2023 12:17:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20230601.gappssmtp.com; s=20230601; t=1694632673; x=1695237473; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=RApe70Ddk4aFbpYXvaraueGlEryEzeF7P3fitACvKPI=; b=HiWSmG+/iOWOZjSQb7hX4MeQ2wwpiQIcAFDdS0cA3YUsGO6V/kmxB7Xg9YbFQHwiKq uglvXe0euDxgenkO2s2nTGes9fV7HC7u0CR1KzXMnaTCDUbtdAoNBPXWfiywsgZFCPye CZoDeBZ0GcxzE4PgA6Ml1J3VMZDnQrHgXvSol5+nrgYIjuNLEPHHyTBRVuzdvS7xhTpq EE4z3RK+EujPcZFZOuQtK4zjEutNM1D2Kk4sC3iV1CZqy/zXJ7Ce01OwukKq5X7JHolO gSUQ/lXZrG3qIzp6K5oM0rBGaHENvSsNGgYzpGCalRa4DaQ9VgR7BQPXDsF1DXiMh+82 1gAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694632673; x=1695237473; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=RApe70Ddk4aFbpYXvaraueGlEryEzeF7P3fitACvKPI=; b=fjoQpxbwVHiSjDRRMabUrzGepYAnOOby6mGQKfz9VafoxihNBfPQp0h7BFbLIwyZkj hD2PBx61QujVIXlIDJ0xlJOmGVdZN0z6gD06oM+c7rDew4IVea8B7utRWpASt4s/ZMC2 8XvRFloXDen0+GwITLGrhAVCkf6l/qJFFsHhRmwD2h2kQBep2R99IsMQ99PNZHs0UVcd HMF7U5TkOCM43qLbPJk6B+RQfS625z3WkDCufKQigQdSjVPeA/AGuV4le9CXetjCnK+2 2SI0ksfprJAHRobTPSXKSgBe0DNLVG4Ulql8AKSaVWfoqQ3IT/mr96FWjhspfXi+WUXo 67uw== X-Gm-Message-State: AOJu0YwGDsFdamW5EcibR1ZjFXrDzv10pHaTVb8qy7AmdxvunFy7vT53 BSr0zuqr/n7NAPYjyUO3gXVsD6vBXmOCxYfs0dLdbQ== X-Google-Smtp-Source: AGHT+IExVRO1+kzMUDoUuFIlx4XcBPDvavcE13W0hqKp7T4w3n6JiufAbMQZ93vA9b4zBgfJHaC3Pw== X-Received: by 2002:a81:6c50:0:b0:583:af98:6fb3 with SMTP id h77-20020a816c50000000b00583af986fb3mr3182566ywc.15.1694632672797; Wed, 13 Sep 2023 12:17:52 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id m128-20020a0de386000000b005837b48d16csm3297712ywe.84.2023.09.13.12.17.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 12:17:52 -0700 (PDT) Date: Wed, 13 Sep 2023 15:17:51 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Patrick Steinhardt Subject: [PATCH v2 4/8] builtin/repack.c: extract redundant pack cleanup for existing packs Message-ID: <82057de4cf3cbba67eb2cbbd8199909ed2108b82.1694632644.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org To remove redundant packs at the end of a repacking operation, Git uses its `remove_redundant_pack()` function in a loop over the set of pre-existing, non-kept packs. In a later commit, we will split this list into two, one for pre-existing cruft pack(s), and another for non-cruft pack(s). Prepare for this by factoring out the routine to loop over and delete redundant packs into its own function. Instead of calling `remove_redundant_pack()` directly, we now will call `remove_redundant_existing_packs()`, which itself dispatches a call to `remove_redundant_packs_1()`. Note that the geometric repacking code will still call `remove_redundant_pack()` directly, but see the previous commit for more details. Having `remove_redundant_packs_1()` exist as a separate function may seem like overkill in this patch. However, a later patch will call `remove_redundant_packs_1()` once over two separate lists, so this refactoring sets us up for that. Signed-off-by: Taylor Blau --- builtin/repack.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index 71366811e9..b5fb14c017 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -135,6 +135,33 @@ static void mark_packs_for_deletion(struct existing_packs *existing, mark_packs_for_deletion_1(names, &existing->non_kept_packs); } +static void remove_redundant_pack(const char *dir_name, const char *base_name) +{ + struct strbuf buf = STRBUF_INIT; + struct multi_pack_index *m = get_local_multi_pack_index(the_repository); + strbuf_addf(&buf, "%s.pack", base_name); + if (m && midx_contains_pack(m, buf.buf)) + clear_midx_file(the_repository); + strbuf_insertf(&buf, 0, "%s/", dir_name); + unlink_pack_path(buf.buf, 1); + strbuf_release(&buf); +} + +static void remove_redundant_packs_1(struct string_list *packs) +{ + struct string_list_item *item; + for_each_string_list_item(item, packs) { + if (!((uintptr_t)item->util & DELETE_PACK)) + continue; + remove_redundant_pack(packdir, item->string); + } +} + +static void remove_redundant_existing_packs(struct existing_packs *existing) +{ + remove_redundant_packs_1(&existing->non_kept_packs); +} + static void existing_packs_release(struct existing_packs *existing) { string_list_clear(&existing->kept_packs, 0); @@ -184,18 +211,6 @@ static void collect_pack_filenames(struct existing_packs *existing, strbuf_release(&buf); } -static void remove_redundant_pack(const char *dir_name, const char *base_name) -{ - struct strbuf buf = STRBUF_INIT; - struct multi_pack_index *m = get_local_multi_pack_index(the_repository); - strbuf_addf(&buf, "%s.pack", base_name); - if (m && midx_contains_pack(m, buf.buf)) - clear_midx_file(the_repository); - strbuf_insertf(&buf, 0, "%s/", dir_name); - unlink_pack_path(buf.buf, 1); - strbuf_release(&buf); -} - static void prepare_pack_objects(struct child_process *cmd, const struct pack_objects_args *args, const char *out) @@ -1222,11 +1237,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (delete_redundant) { int opts = 0; - for_each_string_list_item(item, &existing.non_kept_packs) { - if (!((uintptr_t)item->util & DELETE_PACK)) - continue; - remove_redundant_pack(packdir, item->string); - } + remove_redundant_existing_packs(&existing); if (geometry.split_factor) geometry_remove_redundant_packs(&geometry, &names, From patchwork Wed Sep 13 19:17:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13383702 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0F0C9EE020A for ; Wed, 13 Sep 2023 19:18:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232206AbjIMTSD (ORCPT ); Wed, 13 Sep 2023 15:18:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36744 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232157AbjIMTSA (ORCPT ); Wed, 13 Sep 2023 15:18:00 -0400 Received: from mail-yw1-x112e.google.com (mail-yw1-x112e.google.com [IPv6:2607:f8b0:4864:20::112e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9F01D19A0 for ; Wed, 13 Sep 2023 12:17:56 -0700 (PDT) Received: by mail-yw1-x112e.google.com with SMTP id 00721157ae682-58caaedb20bso1845087b3.1 for ; Wed, 13 Sep 2023 12:17:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20230601.gappssmtp.com; s=20230601; t=1694632675; x=1695237475; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=0pkvMCa8t/uiE70KS12CZYizlDGkE14cjyVfvFeYCAk=; b=Ucce+UsiVUW5e2AIeEihTqrcDYPufI3J2epNLWZAN00U/dozudZSgsYInaHc1E4kZl i1hdvvZnemFI6HP7a7nnU9G89Pk5HgLo9VMqfsfk3F1QLA3BT96oR2Q/0gGoTyAU/mWZ BTRgog6v3QSq1ufRmmVkMriQk6STMvr+4/kHeoj8xCQmpR0mYD80NtsRmANMoH6H5eV+ 5BO1N8hqJF4DWn64aAV5pkKsm7daMM7fQTOJcxgElv2WwNAcqUITqFtXJAjBPF+N9gew oYe6XnkjLyw5X+Z9HUVmhnCq3FVw9SwCBT+QVxlYRFjuHAZpeotkkpM/jb3zWOTvKzA+ WGJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694632675; x=1695237475; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=0pkvMCa8t/uiE70KS12CZYizlDGkE14cjyVfvFeYCAk=; b=QJ9QeOkXoZZT4KZUiaL97wPuDMbhXxj4h7nIFVT8OBTV5Q54f6o7pCb3Z2MFJS1iJ1 niIv9lh1wPOpeSUN6NhSD9lf3M6djcfD1dCWShOrGgjoC38+fgVOix1OiwNeEk0fRtyt dd8yOi0mAhTGfwueilIHtj4A7Clc6p2rWjGwbdQLmtEpwOxhpXgIyinf3fZd/cBpNKr5 1sUXBWHsN3vKYLzM2UMKF8WBa3lH+IsGrZG7lUv4ClFE1lH/O03jOFQ+4RxCoUlwdUUy trPopXm88sdBy19wY+DsOLtIG4CHgB+BZLDsmKPVCzprbbgWgOI2ozQWUFzYAWLtblp2 Wl2Q== X-Gm-Message-State: AOJu0YyoqIJEKaHV/qR9ZgIdRWuq/QYQrtaWqxIK0SwkVePI3SRU+i+O SrqjK+iZlfQnRkZMfSsPpyhovS6sMO0QZ+wfEFp/cg== X-Google-Smtp-Source: AGHT+IHbXUs0qChBm9B2FskdIjruGSR428qIroSaMDNtLcFvN7mdoDRCitbiPkE5Tj4d+IGyuF2ejQ== X-Received: by 2002:a25:b288:0:b0:d81:817c:580 with SMTP id k8-20020a25b288000000b00d81817c0580mr830708ybj.25.1694632675602; Wed, 13 Sep 2023 12:17:55 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id 4-20020a251104000000b00d7ba4c5e31fsm2879661ybr.39.2023.09.13.12.17.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 12:17:55 -0700 (PDT) Date: Wed, 13 Sep 2023 15:17:54 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Patrick Steinhardt Subject: [PATCH v2 5/8] builtin/repack.c: extract `has_existing_non_kept_packs()` Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org When there is: - at least one pre-existing packfile (which is not marked as kept), - repacking with the `-d` flag, and - not doing a cruft repack , then we pass a handful of additional options to the inner `pack-objects` process, like `--unpack-unreachable`, `--keep-unreachable`, and `--pack-loose-unreachable`, in addition to marking any packs we just wrote for promisor remotes as kept in-core (with `--keep-pack`, as opposed to the presence of a ".keep" file on disk). Because we store both cruft and non-cruft packs together in the same `existing.non_kept_packs` list, it suffices to check its `nr` member to see if it is zero or not. But a following change will store cruft- and non-cruft packs separately, meaning this check would break as a result. Prepare for this by extracting this part of the check into a new helper function called `has_existing_non_kept_packs()`. This patch does not introduce any functional changes, but prepares us to make a more isolated change in a subsequent patch. Signed-off-by: Taylor Blau --- builtin/repack.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/builtin/repack.c b/builtin/repack.c index b5fb14c017..9ebc2e774b 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -105,6 +105,11 @@ struct existing_packs { .non_kept_packs = STRING_LIST_INIT_DUP, \ } +static int has_existing_non_kept_packs(const struct existing_packs *existing) +{ + return existing->non_kept_packs.nr; +} + static void mark_packs_for_deletion_1(struct string_list *names, struct string_list *list) { @@ -1047,7 +1052,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (pack_everything & ALL_INTO_ONE) { repack_promisor_objects(&po_args, &names); - if (existing.non_kept_packs.nr && delete_redundant && + if (has_existing_non_kept_packs(&existing) && + delete_redundant && !(pack_everything & PACK_CRUFT)) { for_each_string_list_item(item, &names) { strvec_pushf(&cmd.args, "--keep-pack=%s-%s.pack", From patchwork Wed Sep 13 19:17:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13383703 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 846A5EE020E for ; Wed, 13 Sep 2023 19:18:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232274AbjIMTSH (ORCPT ); Wed, 13 Sep 2023 15:18:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57294 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232157AbjIMTSD (ORCPT ); Wed, 13 Sep 2023 15:18:03 -0400 Received: from mail-yw1-x1130.google.com (mail-yw1-x1130.google.com [IPv6:2607:f8b0:4864:20::1130]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 855EA1BCD for ; Wed, 13 Sep 2023 12:17:59 -0700 (PDT) Received: by mail-yw1-x1130.google.com with SMTP id 00721157ae682-590685a3be5so2065117b3.0 for ; Wed, 13 Sep 2023 12:17:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20230601.gappssmtp.com; s=20230601; t=1694632678; x=1695237478; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=bx2exNJucV3lvU1J+WGOSw53yJledT2LKaXdlFpKl9s=; b=z0lVaYm0xjLUXHuY+kb69gJ83mHfiSl+2Whq2YCrbY9j6E40wbhNOs7MXLvdcBSi0z qqsNVfpAdr3vUkudiQaRLYVK3kV1PRh6K/CuzF2OEEs23J/XUUq3CJEv6ZwR0e/e5RUf JMmRQEJwbXKoZG4a/8wb+19Mr7eJZXVwUU8ScSykaID6LAEDdftAA9VCTJ9/sVGWiIzK pShRHJkva8bnjvnEa2aHgHUFBPIY+FPwFk5aAxYql/box6xeLs3j3dkas6iqE1ryNIqd 1sqg+aHrVzl4PLRpYx+NPl4+NPRPCmNSpuQterad8rTsxPwPwNZP5rlnBEbcnLGzN8Wq Bybg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694632678; x=1695237478; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=bx2exNJucV3lvU1J+WGOSw53yJledT2LKaXdlFpKl9s=; b=Z4XghTdmjgs4FAoyL6idlmvISpnDgsMrYs0mnwY7tK/Nu5bSrI/c0ZOb7S9LYhANTZ 1qcW1zBxpKZDL7gA8XFj3CPgd3vCcUexpnVSFDKUbc6VKhqiYxi07CfLtIaI6gtfZDgs 6acAhS79HLtoEBXZ+Bj7245kgQBdH/nS6FRCE/EA4O/h9fzJ2NS5nJaCZ0RidNMwI2MR uXr4Hm5ZuqiO7tBienJanFvhFPEMwR97M3fGY4Asl5HQppoSnHiimbTcSk2t9ek7zL7p uwmeUiQCzMYmBXryaUjZWuCChQqg90moGKdeSnL4WdDPG/7YEkLDQpjaVLjzszsIzL3i LtPQ== X-Gm-Message-State: AOJu0YzxAZC7/R2zwoKq2ISjdQApBzt2B3ZFeASBtGjnCsPO7S5kt7hv jlud0vWO8bYar+P2I8pyWmwW7+w33c0ffXoCHD9s/w== X-Google-Smtp-Source: AGHT+IHSeXOATheLTIeau2pUy+UVDVVPxY/DMnDogL2kngOKdr8QmfOuKPaYbC2Qz8SyGqpqxCBRXA== X-Received: by 2002:a81:9c44:0:b0:571:11ea:b2dd with SMTP id n4-20020a819c44000000b0057111eab2ddmr3473734ywa.32.1694632678570; Wed, 13 Sep 2023 12:17:58 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id d15-20020a81d34f000000b005772abf6234sm3259020ywl.11.2023.09.13.12.17.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 12:17:58 -0700 (PDT) Date: Wed, 13 Sep 2023 15:17:57 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Patrick Steinhardt Subject: [PATCH v2 6/8] builtin/repack.c: store existing cruft packs separately Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org 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 --- builtin/repack.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index 9ebc2e774b..8103a9d308 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)); + } } } @@ -835,6 +840,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); From patchwork Wed Sep 13 19:18:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13383704 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F16C3EE0207 for ; Wed, 13 Sep 2023 19:18:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232278AbjIMTSJ (ORCPT ); Wed, 13 Sep 2023 15:18:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36768 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232254AbjIMTSG (ORCPT ); Wed, 13 Sep 2023 15:18:06 -0400 Received: from mail-yw1-x112d.google.com (mail-yw1-x112d.google.com [IPv6:2607:f8b0:4864:20::112d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 60C1F1999 for ; Wed, 13 Sep 2023 12:18:02 -0700 (PDT) Received: by mail-yw1-x112d.google.com with SMTP id 00721157ae682-59234aaca15so1744967b3.3 for ; Wed, 13 Sep 2023 12:18:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20230601.gappssmtp.com; s=20230601; t=1694632681; x=1695237481; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=aa83gloW/RSLi/CWbT6UII5KKGTKMtXEFWIeXs8Yq7A=; b=O+Hikdc9l0qkOij8V4iOGxgG/KhgnrL1oWx6mPMwSg5h7oIrLoA4t7G1+DB49DNVGC wPU7x5KGyOnUg4sir7Ola7FyqewiZv7cwnyatKNrkzhq48lMHA1ei83p/uT6rXxrCCur wSl7GBCzAPnIpNNuGQANxXkA+l4Oq3KbUh9M+BQRyadFMOqYRySFSFploGe36hhIK3Si cdirqFbFo3C2CqKNofJt/r5PTY9KsFfQuHAd6JhRvIZcT82QPNDFiZx5AMsEHY7STc5f bST3kXdeWb2Q6pVJwwWQeLZg85VA4gTHr0ulCdXWNdaYsvDi5nueGbsnCGExLTWCmgz+ yCPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694632681; x=1695237481; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=aa83gloW/RSLi/CWbT6UII5KKGTKMtXEFWIeXs8Yq7A=; b=boIidforqwuroSutedH6PNZPC8UdKFQHpwL7vDy6dH9GOmVKJBtNKH7yaJEatK5nUI 4rruS+OLqP2EFgIEdz1yXA2Np5zBVqgTAYYlMTFVZcwsxYfvTwx1DExwAlUwgp9W1BBf S5xm18GHFFyZDmHSqMCRYkv+aHnv9uA8oFljHCM9NdH0RLDHcCc6EhScEtFUT6tJwZMH f55Crh49ONI0yqwSNzAov/rKEnfzg1fT2z/XfXir1ZpBm5BZCcJF1stYqgwq49SqYk+N jdBRtxSfnpsnUz0u4snT1kOFSNERribAnG7r3eAddWBtMRhIDqxr78M5Ssq9dBHZp7RS xMSA== X-Gm-Message-State: AOJu0YygXlvfxVoMBmkEeCPOJiuZsac6iQHAYC/A5mgPPGJffzRtSepc GwvcX82w+iCPUfJR59wr7E+tPe2bjPVS4Y3WExjcIQ== X-Google-Smtp-Source: AGHT+IHD6Fsg9ovE8eod0Qk9bbFh48OkKD9Q3N4NkZ0SJgaMxC2xkG1ELS5wJ8y3BQf8GjYljCzCFQ== X-Received: by 2002:a0d:cb0c:0:b0:57a:50ba:b3a0 with SMTP id n12-20020a0dcb0c000000b0057a50bab3a0mr3300647ywd.12.1694632681418; Wed, 13 Sep 2023 12:18:01 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id i6-20020a0ddf06000000b005832ca42ba6sm3270787ywe.3.2023.09.13.12.18.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 12:18:01 -0700 (PDT) Date: Wed, 13 Sep 2023 15:18:00 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Patrick Steinhardt Subject: [PATCH v2 7/8] builtin/repack.c: avoid directly inspecting "util" Message-ID: <481a29599b1871af4f58a68bdf227b1fe477d365.1694632644.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org The `->util` field corresponding to each string_list_item is used to track the existence of some pack at the beginning of a repack operation was originally intended to be used as a bitfield. This bitfield tracked: - (1 << 0): whether or not the pack should be deleted - (1 << 1): whether or not the pack is cruft The previous commit removed the use of the second bit, but a future patch (from a different series than this one) will introduce a new use of it. So we could stop treating the util pointer as a bitfield and instead start treating it as if it were a boolean. But this would require some backtracking when that later patch is applied. Instead, let's avoid touching the ->util field directly, and instead introduce convenience functions like: - pack_mark_for_deletion() - pack_is_marked_for_deletion() Helped-by: Junio C Hamano Helped-by: Jeff King Helped-by: Patrick Steinhardt Signed-off-by: Taylor Blau --- builtin/repack.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index 8103a9d308..e9a091fbbc 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -111,6 +111,16 @@ static int has_existing_non_kept_packs(const struct existing_packs *existing) return existing->non_kept_packs.nr || existing->cruft_packs.nr; } +static void pack_mark_for_deletion(struct string_list_item *item) +{ + item->util = (void*)((uintptr_t)item->util | DELETE_PACK); +} + +static int pack_is_marked_for_deletion(struct string_list_item *item) +{ + return (uintptr_t)item->util & DELETE_PACK; +} + static void mark_packs_for_deletion_1(struct string_list *names, struct string_list *list) { @@ -130,7 +140,7 @@ static void mark_packs_for_deletion_1(struct string_list *names, * (if `-d` was given). */ if (!string_list_has_string(names, sha1)) - item->util = (void*)(uintptr_t)((size_t)item->util | DELETE_PACK); + pack_mark_for_deletion(item); } } @@ -158,7 +168,7 @@ static void remove_redundant_packs_1(struct string_list *packs) { struct string_list_item *item; for_each_string_list_item(item, packs) { - if (!((uintptr_t)item->util & DELETE_PACK)) + if (!pack_is_marked_for_deletion(item)) continue; remove_redundant_pack(packdir, item->string); } @@ -702,13 +712,13 @@ static void midx_included_packs(struct string_list *include, } } else { for_each_string_list_item(item, &existing->non_kept_packs) { - if ((uintptr_t)item->util & DELETE_PACK) + if (pack_is_marked_for_deletion(item)) 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) + if (pack_is_marked_for_deletion(item)) continue; string_list_insert(include, xstrfmt("%s.idx", item->string)); } From patchwork Wed Sep 13 19:18:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13383705 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3CB32EE0207 for ; Wed, 13 Sep 2023 19:18:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232356AbjIMTSR (ORCPT ); Wed, 13 Sep 2023 15:18:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57346 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232193AbjIMTSJ (ORCPT ); Wed, 13 Sep 2023 15:18:09 -0400 Received: from mail-yw1-x112d.google.com (mail-yw1-x112d.google.com [IPv6:2607:f8b0:4864:20::112d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 250BE19AD for ; Wed, 13 Sep 2023 12:18:05 -0700 (PDT) Received: by mail-yw1-x112d.google.com with SMTP id 00721157ae682-59bbdb435bfso1845607b3.3 for ; Wed, 13 Sep 2023 12:18:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20230601.gappssmtp.com; s=20230601; t=1694632684; x=1695237484; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=oU5X1YuO1DCAzcnKVtmesO+313Y28xIZL91JU5d57yU=; b=zSXzSRCY8y+pGTiLkee3axfrDyXFKbnpS8SzQ/+Jz/bJNhmddsgYBLnH/lJ5HBvJre BTBh6ROH6mTD/eCaCCxoBOEZBuUPgdrzKFzmDK4fl6DrrbkIaR560Kv2cr7XygrTmW8y B19+sxEiWvFyu6vGfiSZNLJ936+xIePAkkSdeNk4+OGjlO71wkX/cAIohXV1DyWelXyW 47KwkCh8fHJ3WqDUmJbm7PIYfAc92vOohHcel5oSjqhrPJP95aMIOYi1iOaw1D1vB3TZ lDRe2n10iZCwJL5QA4tqJxhCmQBTmKxx7qUNc585Pz8aD2juQFDnvsNu3L3I/uWiJ9qq oOyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694632684; x=1695237484; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=oU5X1YuO1DCAzcnKVtmesO+313Y28xIZL91JU5d57yU=; b=DnBshTARngjbjL0crm+CF56uYzRt2xGG3jk6g7To6qTHc2v8tvHv7he3byj/gRwvEd HZhhaW6UbtCy1v3q1xSYibiDhNat0bM2NdI34TzhUDevrGMsEssNtA9IqwfiRwXjCnlP 2vZOVVrvqfJ+jRrqmJjp7jTh/fk7zQ/HTaASO405cqfUGankOr477rzPq1Vyb5Ss+vfT wF2Mr0CBQk2kUzyPA25d0Dk61PmTjE7kYuFvcd7HtXOl4r70tX2JdyUhUclbYLwbhWI8 ePuFYW9kWFAxniYMfbzSmaM/ZBcwTkwGTIRse6PqeP0XRg0UCTtYpQJ0IYFPaGsAPAxQ 0cyQ== X-Gm-Message-State: AOJu0YyrOrPaDLB8EWCyCzVGweyCLgDb2KWtSWb47GC5p6FmbDQNT2q+ J/TmQ3LZaPx8Rjw9Ay6oP3zDXNQ+3oiPPFx+AscXAw== X-Google-Smtp-Source: AGHT+IG5ZQNVIZkkzMRi0XdDs6+VVaos60zYeFfz/UZAXfHt+HfjJkdo34oOE6LxLUKk34gntlz7Mw== X-Received: by 2002:a81:a20e:0:b0:581:2887:22be with SMTP id w14-20020a81a20e000000b00581288722bemr3733813ywg.37.1694632684123; Wed, 13 Sep 2023 12:18:04 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id en11-20020a05690c2b8b00b0059bc980b1eesm493698ywb.6.2023.09.13.12.18.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 12:18:03 -0700 (PDT) Date: Wed, 13 Sep 2023 15:18:03 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Patrick Steinhardt Subject: [PATCH v2 8/8] builtin/repack.c: extract common cruft pack loop Message-ID: <68748eb9c865ba8252852713533e2f47e2cac489.1694632644.git.me@ttaylorr.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org When generating the list of packs to store in a MIDX (when given the `--write-midx` option), we include any cruft packs both during --geometric and non-geometric repacks. But the rules for when we do and don't have to check whether any of those cruft packs were queued for deletion differ slightly between the two cases. But the two can be unified, provided there is a little bit of extra detail added in the comment to clarify when it is safe to avoid checking for any pending deletions (and why it is OK to do so even when not required). Signed-off-by: Taylor Blau --- builtin/repack.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/builtin/repack.c b/builtin/repack.c index e9a091fbbc..529e13120d 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -702,26 +702,31 @@ static void midx_included_packs(struct string_list *include, string_list_insert(include, strbuf_detach(&buf, NULL)); } - - 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 { for_each_string_list_item(item, &existing->non_kept_packs) { if (pack_is_marked_for_deletion(item)) continue; string_list_insert(include, xstrfmt("%s.idx", item->string)); } + } - for_each_string_list_item(item, &existing->cruft_packs) { - if (pack_is_marked_for_deletion(item)) - continue; - string_list_insert(include, xstrfmt("%s.idx", item->string)); - } + for_each_string_list_item(item, &existing->cruft_packs) { + /* + * When doing a --geometric repack, there is no need to check + * for deleted packs, since we're by definition not doing an + * ALL_INTO_ONE repack (hence no packs will be deleted). + * Otherwise we must check for and exclude any packs which are + * enqueued for deletion. + * + * So we could omit the conditional below in the --geometric + * case, but doing so is unnecessary since no packs are marked + * as pending deletion (since we only call + * `mark_packs_for_deletion()` when doing an all-into-one + * repack). + */ + if (pack_is_marked_for_deletion(item)) + continue; + string_list_insert(include, xstrfmt("%s.idx", item->string)); } }