From: Taylor Blau <me@ttaylorr.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org, Jeff King <peff@peff.net>,
Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH v2 0/8] repack: refactor pack snapshot-ing logic
Date: Thu, 14 Sep 2023 14:01:54 -0400 [thread overview]
Message-ID: <ZQNKkn0YYLUyN5Ih@nand.local> (raw)
In-Reply-To: <CAP8UFD1rDb-iYf4LYb7n=K4KpQ-JR-JK4TkQpGJ-TCfTNFFbnA@mail.gmail.com>
On Thu, Sep 14, 2023 at 01:10:38PM +0200, Christian Couder wrote:
> Ok, I will try to review and merge this with
> cc/repack-sift-filtered-objects-to-separate-pack soon.
I took a look at how much/little effort was going to be required, and
luckily the changes are isolated only to a single patch. It's just your
f1ffa71e8f (repack: add `--filter=<filter-spec>` option, 2023-09-11),
and in particular the `write_filtered_pack()` function.
I started messing around with it myself and generated the following
fixup! which can be applied on top of your version of f1ffa71e8f. It's
mostly straightforward, but there is a gotcha that the loop over
non-kept packs has to change to:
for_each_string_list_item(item, &existing->non_kept_packs)
/* ... */
for_each_string_list_item(item, &existing->cruft_packs)
/* ... */
, instead of just the first loop over non_kept_packs, since cruft packs
are stored in a separate list.
In any event, here's the fixup! I generated on top of that patch:
--- 8< ---
Subject: [PATCH] fixup! repack: add `--filter=<filter-spec>` option
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/repack.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 120f4241c0..0d23323d05 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -834,15 +834,13 @@ static int finish_pack_objects_cmd(struct child_process *cmd,
static int write_filtered_pack(const struct pack_objects_args *args,
const char *destination,
const char *pack_prefix,
- struct string_list *keep_pack_list,
- struct string_list *names,
- struct string_list *existing_packs,
- struct string_list *existing_kept_packs)
+ struct existing_packs *existing,
+ struct string_list *names)
{
struct child_process cmd = CHILD_PROCESS_INIT;
struct string_list_item *item;
FILE *in;
- int ret, i;
+ int ret;
const char *caret;
const char *scratch;
int local = skip_prefix(destination, packdir, &scratch);
@@ -853,9 +851,8 @@ static int write_filtered_pack(const struct pack_objects_args *args,
if (!pack_kept_objects)
strvec_push(&cmd.args, "--honor-pack-keep");
- for (i = 0; i < keep_pack_list->nr; i++)
- strvec_pushf(&cmd.args, "--keep-pack=%s",
- keep_pack_list->items[i].string);
+ for_each_string_list_item(item, &existing->kept_packs)
+ strvec_pushf(&cmd.args, "--keep-pack=%s", item->string);
cmd.in = -1;
@@ -872,10 +869,12 @@ static int write_filtered_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->cruft_packs)
fprintf(in, "%s.pack\n", item->string);
caret = pack_kept_objects ? "" : "^";
- for_each_string_list_item(item, existing_kept_packs)
+ for_each_string_list_item(item, &existing->kept_packs)
fprintf(in, "%s%s.pack\n", caret, item->string);
fclose(in);
@@ -1261,10 +1260,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
ret = write_filtered_pack(&po_args,
packtmp,
find_pack_prefix(packdir, packtmp),
- &keep_pack_list,
- &names,
- &existing_nonkept_packs,
- &existing_kept_packs);
+ &existing,
+ &names);
if (ret)
goto cleanup;
}
--
2.42.0.137.g6fe1dff026
--- >8 ---
Thanks,
Taylor
next prev parent reply other threads:[~2023-09-14 18:01 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 ` [PATCH v2 2/8] builtin/repack.c: extract marking packs for deletion Taylor Blau
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 [this message]
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=ZQNKkn0YYLUyN5Ih@nand.local \
--to=me@ttaylorr.com \
--cc=christian.couder@gmail.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).