| Message ID | 2728513.vuYhMxLoTh@mintaka.ncbr.muni.cz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC,resend] builtin/repack: Honor --keep-pack and .keep when repacking promisor objects | expand |
On 2025-01-29 at 10:02:06, Tomáš Trnka wrote: > diff --git a/builtin/repack.c b/builtin/repack.c > index d6bb37e84a..fe62fe03eb 100644 > --- a/builtin/repack.c > +++ b/builtin/repack.c > @@ -388,15 +388,23 @@ static int has_pack_ext(const struct generated_pack_data > *data, > } > > static void repack_promisor_objects(const struct pack_objects_args *args, > - struct string_list *names) > + struct string_list *names, > + struct string_list > *keep_pack_list) I don't have a strong opinion about the technical aspects of this patch (nor sufficient knowledge to review it)[0], but I noticed that there's a couple of places, this line among them, which are unexpectedly wrapped, so I don't believe this patch will actually apply. I noticed that the email didn't specify an MUA header (or I missed it), so I can't make a suggestion on how to fix your MUA, but you may want to use `git send-email` to avoid this problem in the future. [0] In other words, no need to CC me on a resend.
On Thursday, 30 January 2025 3:26:19, CET brian m. carlson wrote: > I don't have a strong opinion about the technical aspects of this patch > (nor sufficient knowledge to review it)[0], but I noticed that there's a > couple of places, this line among them, which are unexpectedly wrapped, > so I don't believe this patch will actually apply. Oops, my sincere apologies to everyone. I'll resend once again. (I messed up by not realizing that KMail will re-enable line wrapping when re- sending my original message, which was formatted correctly.) 2T
diff --git a/builtin/repack.c b/builtin/repack.c index d6bb37e84a..fe62fe03eb 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -388,15 +388,23 @@ static int has_pack_ext(const struct generated_pack_data *data, } static void repack_promisor_objects(const struct pack_objects_args *args, - struct string_list *names) + struct string_list *names, + struct string_list *keep_pack_list) { struct child_process cmd = CHILD_PROCESS_INIT; FILE *out; struct strbuf line = STRBUF_INIT; + int i; prepare_pack_objects(&cmd, args, packtmp); cmd.in = -1; + 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); + /*
git-repack currently does not pass --keep-pack or --honor-pack-keep to the git-pack-objects handling promisor packs. This means that settings like gc.bigPackThreshold are completely ignored for promisor packs. The simple fix is to just copy the keep-pack logic into repack_promisor_objects(), although this could possibly be improved by making prepare_pack_objects() handle it instead. Signed-off-by: Tomáš Trnka <trnka@scm.com> --- RFC: This probably needs a test, but where and how should it be implemented? Perhaps in t7700-repack.sh, copying one of the tests using prepare_for_keep_packs and just touching .promisor files? Or instead in t/t0410-partial-clone.sh using a copy/variant of one of the basic repack tests there? builtin/repack.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) * NEEDSWORK: Giving pack-objects only the OIDs without any ordering * hints may result in suboptimal deltas in the resulting pack. See if @@ -1350,7 +1358,7 @@ int cmd_repack(int argc, strvec_push(&cmd.args, "--delta-islands"); if (pack_everything & ALL_INTO_ONE) { - repack_promisor_objects(&po_args, &names); + repack_promisor_objects(&po_args, &names, &keep_pack_list); if (has_existing_non_kept_packs(&existing) && delete_redundant && base-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f