undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

builtin/repack.c: add cruft packs to MIDX during geometric repack

Browse Source 
When using cruft packs, the following race can occur when a geometric
repack that writes a MIDX bitmap takes place afterwords:

  - First, create an unreachable object and do an all-into-one cruft
    repack which stores that object in the repository's cruft pack.
  - Then make that object reachable.
  - Finally, do a geometric repack and write a MIDX bitmap.

Assuming that we are sufficiently unlucky as to select a commit from the
MIDX which reaches that object for bitmapping, then the `git
multi-pack-index` process will complain that that object is missing.

The reason is because we don't include cruft packs in the MIDX when
doing a geometric repack. Since the "make that object reachable" doesn't
necessarily mean that we'll create a new copy of that object in one of
the packs that will get rolled up as part of a geometric repack, it's
possible that the MIDX won't see any copies of that now-reachable
object.

Of course, it's desirable to avoid including cruft packs in the MIDX
because it causes the MIDX to store a bunch of objects which are likely
to get thrown away. But excluding that pack does open us up to the above
race.

This patch demonstrates the bug, and resolves it by including cruft
packs in the MIDX even when doing a geometric repack.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Taylor Blau 2022-05-20 19:18:11 -04:00 committed by Junio C Hamano
parent 72263ffc32
commit ddee3703b3
2 changed files with 46 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
builtin/repack.c
View File

@ -23,6 +23,7 @@
#define PACK_CRUFT 4
#define DELETE_PACK 1
#define CRUFT_PACK 2
static int pack_everything;
static int delta_base_offset = 1;
@ -159,10 +160,15 @@ static void collect_pack_filenames(struct string_list *fname_nonkept_list,
fname = xmemdupz(e->d_name, len);
if ((extra_keep->nr > 0 && i < extra_keep->nr) ||
(file_exists(mkpath("%s/%s.keep", packdir, fname))))
(file_exists(mkpath("%s/%s.keep", packdir, fname)))) {
string_list_append_nodup(fname_kept_list, fname);
else
string_list_append_nodup(fname_nonkept_list, fname);
} else {
struct string_list_item *item;
item = string_list_append_nodup(fname_nonkept_list,
fname);
if (file_exists(mkpath("%s/%s.mtimes", packdir, fname)))
item->util = (void*)(uintptr_t)CRUFT_PACK;
}
}
closedir(dir);
}
@ -564,6 +570,17 @@ static void midx_included_packs(struct string_list *include,
string_list_insert(include, strbuf_detach(&buf, NULL));
}
for_each_string_list_item(item, existing_nonkept_packs) {
if (!((uintptr_t)item->util & CRUFT_PACK)) {
/*
* no need to check DELETE_PACK, since we're not
* doing an ALL_INTO_ONE repack
*/
continue;
}
string_list_insert(include, xstrfmt("%s.idx", item->string));
}
} else {
for_each_string_list_item(item, existing_nonkept_packs) {
if ((uintptr_t)item->util & DELETE_PACK)

26
t/t5329-pack-objects-cruft.sh
View File

@ -648,4 +648,30 @@ test_expect_success 'cruft --local drops unreachable objects' '
)
'
test_expect_success 'MIDX bitmaps tolerate reachable cruft objects' '
git init repo &&
test_when_finished "rm -fr repo" &&
(
cd repo &&
test_commit reachable &&
test_commit cruft &&
unreachable="$(git rev-parse cruft)" &&
git reset --hard $unreachable^ &&
git tag -d cruft &&
git reflog expire --all --expire=all &&
git repack --cruft -d &&
# resurrect the unreachable object via a new commit. the
# new commit will get selected for a bitmap, but be
# missing one of its parents from the selected packs.
git reset --hard $unreachable &&
test_commit resurrect &&
git repack --write-midx --write-bitmap-index --geometric=2 -d
)
'
test_done