If using --object-dir to point into a repo while the current working dir is outside, such as git init /repo git -C /repo ... # add some objects cd /non-repo git multi-pack-index --object-dir /repo/.git/objects/ write the binary will segfault trying to access the object-dir via the repo it found, but that's not fully initialized. Fix it to use the object_dir properly to clean up the *.rev files, this avoids the crash and cleans up the *.rev files for the now rewritten multi-pack-index properly. Fixes: 38ff7cabb6b8 ("pack-revindex: write multi-pack reverse indexes") Cc: Taylor Blau <me@xxxxxxxxxxxx> Signed-off-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx> --- Due to running inside git's tree, even with TEST_NO_CREATE_REPO=t I cannot reproduce the segfault in a test without the "cd /", so I've kept that. Yes, the test caught in that case that the *.rev file wasn't cleaned up (due to being initialized to the wrong git repo [git's] and cleaning up there!), but I wanted to test the segfault too. --- midx.c | 10 +++++----- t/t5319-multi-pack-index.sh | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/midx.c b/midx.c index 321c6fdd2f18..902e1a7a7d9d 100644 --- a/midx.c +++ b/midx.c @@ -882,7 +882,7 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash, strbuf_release(&buf); } -static void clear_midx_files_ext(struct repository *r, const char *ext, +static void clear_midx_files_ext(const char *object_dir, const char *ext, unsigned char *keep_hash); static int midx_checksum_valid(struct multi_pack_index *m) @@ -1086,7 +1086,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * if (flags & MIDX_WRITE_REV_INDEX) write_midx_reverse_index(midx_name, midx_hash, &ctx); - clear_midx_files_ext(the_repository, ".rev", midx_hash); + clear_midx_files_ext(object_dir, ".rev", midx_hash); commit_lock_file(&lk); @@ -1135,7 +1135,7 @@ static void clear_midx_file_ext(const char *full_path, size_t full_path_len, die_errno(_("failed to remove %s"), full_path); } -static void clear_midx_files_ext(struct repository *r, const char *ext, +static void clear_midx_files_ext(const char *object_dir, const char *ext, unsigned char *keep_hash) { struct clear_midx_data data; @@ -1146,7 +1146,7 @@ static void clear_midx_files_ext(struct repository *r, const char *ext, hash_to_hex(keep_hash), ext); data.ext = ext; - for_each_file_in_pack_dir(r->objects->odb->path, + for_each_file_in_pack_dir(object_dir, clear_midx_file_ext, &data); @@ -1165,7 +1165,7 @@ void clear_midx_file(struct repository *r) if (remove_path(midx)) die(_("failed to clear multi-pack-index at %s"), midx); - clear_midx_files_ext(r, ".rev", NULL); + clear_midx_files_ext(r->objects->odb->path, ".rev", NULL); free(midx); } diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 3d4d9f10c31b..3b6331f64113 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -201,6 +201,25 @@ test_expect_success 'write midx with twelve packs' ' compare_results_with_midx "twelve packs" +test_expect_success 'multi-pack-index *.rev cleanup with --object-dir' ' + git init objdir-test-repo && + test_when_finished "rm -rf objdir-test-repo" && + ( + cd objdir-test-repo && + test_commit base && + git repack -d + ) && + rev="objdir-test-repo/$objdir/pack/multi-pack-index-abcdef123456.rev" && + touch $rev && + ( + base="$(pwd)" && + cd / && # run outside any git repo, including git itself + git multi-pack-index --object-dir="$base/objdir-test-repo/$objdir" write + ) && + test_path_is_file objdir-test-repo/$objdir/pack/multi-pack-index && + test_path_is_missing $rev +' + test_expect_success 'warn on improper hash version' ' git init --object-format=sha1 sha1 && ( -- 2.31.1