[PATCH v3 1/3] repack: pack everything into packfile
- From
Han Young <hanyang.tony@bytedance.com>
- Date
- Oct 11, 2024, 08:24 UTC
- Message-ID
- <20241011082404.88939-2-hanyang.tony@bytedance.com>
- In-Reply-To
- <20241011082404.88939-1-hanyang.tony@bytedance.com>
In a partial repository, creating a local commit and then fetching causes the following state to occur:
commit tree blob C3 ---- T3 -- B3 (fetched from remote, in promisor pack) | C2 ---- T2 -- B2 (created locally, in non-promisor pack) | C1 ---- T1 -- B1 (fetched from remote, in promisor pack)
During garbage collection, parents of promisor objects are marked as UNINTERESTING and are subsequently garbage collected. In this case, C2 would be deleted and attempts to access that commit would result in "bad object" errors (originally reported here[1]).
For partial repos, repacking is done in two steps. We first repack all the objects in promisor packfile, then repack all the non-promisor objects. It turns out C2, T2 and B2 are not repacked in either steps, ended up deleted. We can avoid this by packing everything into a promisor packfile, if the repo is partial cloned.
[1] https://lore.kernel.org/git/20240802073143.56731-1-hanyang.tony@bytedance.com/
Helped-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Han Young <hanyang.tony@bytedance.com> --- builtin/repack.c | 46 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 7 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c index 79f407ca04..50e14ccfc4 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -343,6 +343,23 @@ static int write_oid(const struct object_id *oid, return 0; } +static int write_loose_oid(const struct object_id *oid, + const char *path UNUSED, + void *data) +{ + struct child_process *cmd = data; + + if (cmd->in == -1) { + if (start_command(cmd)) + die(_("could not start pack-objects to repack promisor objects")); + } + + if (write_in_full(cmd->in, oid_to_hex(oid), the_hash_algo->hexsz) < 0 || + write_in_full(cmd->in, "\n", 1) < 0) + die(_("failed to feed promisor objects to pack-objects")); + return 0; +} + static struct { const char *name; unsigned optional:1; @@ -392,12 +409,15 @@ static int has_pack_ext(const struct generated_pack_data *data, BUG("unknown pack extension: '%s'", ext); } -static void repack_promisor_objects(const struct pack_objects_args *args, - struct string_list *names) +static int repack_promisor_objects(const struct pack_objects_args *args, + struct string_list *names, + struct string_list *list, + int pack_all) { struct child_process cmd = CHILD_PROCESS_INIT; FILE *out; struct strbuf line = STRBUF_INIT; + struct string_list_item *item; prepare_pack_objects(&cmd, args, packtmp); cmd.in = -1; @@ -409,13 +429,19 @@ static void repack_promisor_objects(const struct pack_objects_args *args, * {type -> existing pack order} ordering when computing deltas instead * of a {type -> size} ordering, which may produce better deltas. */ - for_each_packed_object(write_oid, &cmd, - FOR_EACH_OBJECT_PROMISOR_ONLY); + if (pack_all) + for_each_packed_object(write_oid, &cmd, 0); + else + for_each_string_list_item(item, list) { + pack_mark_retained(item); + } + + for_each_loose_object(write_loose_oid, &cmd, 0); if (cmd.in == -1) { /* No packed objects; cmd was never started */ child_process_clear(&cmd); - return; + return 0; } close(cmd.in); @@ -453,6 +479,7 @@ static void repack_promisor_objects(const struct pack_objects_args *args, if (finish_command(&cmd)) die(_("could not finish pack-objects to repack promisor objects")); strbuf_release(&line); + return 0; } struct pack_geometry { @@ -1356,9 +1383,13 @@ int cmd_repack(int argc, if (use_delta_islands) strvec_push(&cmd.args, "--delta-islands"); - if (pack_everything & ALL_INTO_ONE) { - repack_promisor_objects(&po_args, &names); + if (repo_has_promisor_remote(the_repository)) { + ret = repack_promisor_objects(&po_args, &names, + &existing.non_kept_packs, pack_everything & ALL_INTO_ONE); + goto pack_objects_end; + } + if (pack_everything & ALL_INTO_ONE) { if (has_existing_non_kept_packs(&existing) && delete_redundant && !(pack_everything & PACK_CRUFT)) { @@ -1478,6 +1509,7 @@ int cmd_repack(int argc, } } +pack_objects_end: if (po_args.filter_options.choice) { if (!filter_to) filter_to = packtmp;
-- 2.47.0.266.g0b04b6b485.dirty