repack: populate extension bits incrementally

After generating the main pack and then any additional cruft packs, we iterate over the "names" list (which contains hashes of packs generated by pack-objects), and call populate_pack_exts() for each. There's one small problem with this. In repack_promisor_objects(), we may add entries to "names" and call populate_pack_exts() for them. Calling it again is mostly just wasteful, as we'll stat() the filename with each possible extension, get the same result, and just overwrite our bits. So we could drop the call there, and leave the final loop to populate all of the bits. But instead, this patch does the reverse: drops the final loop, and teaches the other two sites to populate the bits as they add entries. This makes the code easier to reason about, as you never have to worry about when the util field is valid; it is always valid for each entry. It also serves my ulterior purpose: recording the generated filenames as soon as possible will make it easier for a future patch to use them for cleaning up from a failed operation. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 21, 2022 at 20:21 UTC b639606fd0e20584edd2515236fcc69ada24e430
1 file changed +9 -6
builtin/repack.c
+9 -6
@@ -714,10 +714,14 @@ static int write_cruft_pack(const struct pack_objects_args *args,
714
715 out = xfdopen(cmd.out, "r");
716 while (strbuf_getline_lf(&line, out) != EOF) {
717 + struct string_list_item *item;
718 +
719 if (line.len != the_hash_algo->hexsz)
720 die(_("repack: Expecting full hex object ID lines only "
721 "from pack-objects."));
720 - string_list_append(names, line.buf);
722 +
723 + item = string_list_append(names, line.buf);
724 + item->util = populate_pack_exts(line.buf);
725 }
726 fclose(out);
727
@@ -956,9 +960,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
960
961 out = xfdopen(cmd.out, "r");
962 while (strbuf_getline_lf(&line, out) != EOF) {
963 + struct string_list_item *item;
964 +
965 if (line.len != the_hash_algo->hexsz)
966 die(_("repack: Expecting full hex object ID lines only from pack-objects."));
961 - string_list_append(&names, line.buf);
967 + item = string_list_append(&names, line.buf);
968 + item->util = populate_pack_exts(item->string);
969 }
970 fclose(out);
971 ret = finish_command(&cmd);
@@ -997,10 +1004,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1004
1005 string_list_sort(&names);
1006
1000 - for_each_string_list_item(item, &names) {
1001 - item->util = populate_pack_exts(item->string);
1002 - }
1003 -
1007 close_object_store(the_repository->objects);
1008
1009 /*