[PATCH 3/3] builtin/repack.c: ensure that `names` is sorted
- From
Taylor Blau <me@ttaylorr.com>
- Date
- May 20, 2022, 19:01 UTC
- Message-ID
- <a3d25b9e5dda9a999597115933bf0ce70dce51b2.1653073280.git.me@ttaylorr.com>
- In-Reply-To
- <cover.1653073280.git.me@ttaylorr.com>
The previous patch demonstrates a scenario where the list of packs written by `pack-objects` (and stored in the `names` string_list) is out-of-order, and can thus cause us to delete packs we shouldn't.
This patch resolves that bug by ensuring that `names` is sorted in all cases, not just when
delete_redundant && pack_everything & ALL_INTO_ONE
is true.
Because we did sort `names` in that case (which, prior to `--geometric` repacks, was the only time we would actually delete packs, this is only a bug for `--geometric` repacks.
It would be sufficient to only sort `names` when `delete_redundant` is set to a non-zero value. But sorting a small list of strings is cheap, and it is defensive against future calls to `string_list_has_string()` on this list.
Co-discovered-by: Victoria Dye <vdye@github.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> --- builtin/repack.c | 3 ++- t/t7703-repack-geometric.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c index ea56e92ad5..0e4aae80c0 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -856,6 +856,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (!names.nr && !po_args.quiet) printf_ln(_("Nothing new to pack.")); + string_list_sort(&names); + for_each_string_list_item(item, &names) { item->util = (void *)(uintptr_t)populate_pack_exts(item->string); } @@ -896,7 +898,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix) 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_nonkept_packs) { char *sha1; size_t len = strlen(item->string); diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh index 2cd1de7295..da87f8b2d8 100755 --- a/t/t7703-repack-geometric.sh +++ b/t/t7703-repack-geometric.sh @@ -231,7 +231,7 @@ test_expect_success '--geometric chooses largest MIDX preferred pack' ' ) ' -test_expect_failure '--geometric with pack.packSizeLimit' ' +test_expect_success '--geometric with pack.packSizeLimit' ' git init pack-rewrite && test_when_finished "rm -fr pack-rewrite" && (
-- 2.36.1.94.gb0d54bedca