> @@ -683,22 +755,41 @@ 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;
> + string_list_sort(&names);
> + for_each_string_list_item(item, &existing_packs) {
> + char *sha1;
> + size_t len = strlen(item->string);
> + if (len < hexsz)
> + continue;
> + sha1 = item->string + len - hexsz;
> + item->util = (void*)(intptr_t)!string_list_has_string(&names, sha1);
OK, here is the tricky part. They are marked for deletion here...
> + }
> + }
> +
> + if (write_midx) {
> + struct string_list include = STRING_LIST_INIT_NODUP;
> + midx_included_packs(&include, &existing_packs,
> + &existing_kept_packs, &names, geometry);
...the mark for deletion is taken into account during the execution of
midx_included_packs() (as can be seen by looking at that function)...
> +
> + ret = write_midx_included_packs(&include,
> + show_progress, write_bitmaps > 0);
> +
> + string_list_clear(&include, 0);
> +
> + if (ret)
> + return ret;
> + }
> +
> reprepare_packed_git(the_repository);
>
> if (delete_redundant) {
> int opts = 0;
> - if (pack_everything & ALL_INTO_ONE) {
> - const int hexsz = the_hash_algo->hexsz;
> - string_list_sort(&names);
> - for_each_string_list_item(item, &existing_packs) {
> - char *sha1;
> - size_t len = strlen(item->string);
> - if (len < hexsz)
> - continue;
> - sha1 = item->string + len - hexsz;
> - if (!string_list_has_string(&names, sha1))
> - remove_redundant_pack(packdir, item->string);
> - }
> + for_each_string_list_item(item, &existing_packs) {
> + if (!item->util)
> + continue;
...and the marks are also used here. I was at first confused about why
the functionality of midx_included_packs() depended on whether redundant
packs were marked for deletion - if they are redundant, shouldn't they
never be taken into account (regardless of whether we actually delete
them)? But I guess it makes sense as an overall design point that we
pass all packs that are to remain (so if they will be deleted, exclude
them, and if they will not be, include them).
I think a comment "mark this pack for deletion" at the point we write
the mark (so, where the cast to intptr_t is) is worthwhile. Other than
that, this patch looks good to me.
I (and the others in our review club) only managed to reach this patch.
I hope to get to the other 2 by the end of the week.
