midx: fix `*.rev` cleanups with `--object-dir`

If using --object-dir to point into an object directory which belongs to a different repository than the one in the current working directory, such as: git init repo git -C repo ... # add some objects cd alternate 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. Worse, if we later call clear_midx_files_ext(), we will use `the_repository` and remove files out of the wrong object directory. Fix this by using the given object_dir (or the object directory of `the_repository` if `--object-dir` wasn't given) to properly to clean up the *.rev files, avoiding the crash. Original-patch-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Aug 31, 2021 at 16:51 UTC 426c00e4541367bcd96e542f971d8995e3bb1d54
2 files changed +33 -5
midx.c
+5 -5
@@ -882,7 +882,7 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash,
882 strbuf_release(&buf);
883 }
884
885 -static void clear_midx_files_ext(struct repository *r, const char *ext,
885 +static void clear_midx_files_ext(const char *object_dir, const char *ext,
886 unsigned char *keep_hash);
887
888 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 *
1086
1087 if (flags & MIDX_WRITE_REV_INDEX)
1088 write_midx_reverse_index(midx_name, midx_hash, &ctx);
1089 - clear_midx_files_ext(the_repository, ".rev", midx_hash);
1089 + clear_midx_files_ext(object_dir, ".rev", midx_hash);
1090
1091 commit_lock_file(&lk);
1092
@@ -1135,7 +1135,7 @@ static void clear_midx_file_ext(const char *full_path, size_t full_path_len,
1135 die_errno(_("failed to remove %s"), full_path);
1136 }
1137
1138 -static void clear_midx_files_ext(struct repository *r, const char *ext,
1138 +static void clear_midx_files_ext(const char *object_dir, const char *ext,
1139 unsigned char *keep_hash)
1140 {
1141 struct clear_midx_data data;
@@ -1146,7 +1146,7 @@ static void clear_midx_files_ext(struct repository *r, const char *ext,
1146 hash_to_hex(keep_hash), ext);
1147 data.ext = ext;
1148
1149 - for_each_file_in_pack_dir(r->objects->odb->path,
1149 + for_each_file_in_pack_dir(object_dir,
1150 clear_midx_file_ext,
1151 &data);
1152
@@ -1165,7 +1165,7 @@ void clear_midx_file(struct repository *r)
1165 if (remove_path(midx))
1166 die(_("failed to clear multi-pack-index at %s"), midx);
1167
1168 - clear_midx_files_ext(r, ".rev", NULL);
1168 + clear_midx_files_ext(r->objects->odb->path, ".rev", NULL);
1169
1170 free(midx);
1171 }
t/t5319-multi-pack-index.sh
+28
@@ -201,6 +201,34 @@ test_expect_success 'write midx with twelve packs' '
201
202 compare_results_with_midx "twelve packs"
203
204 +test_expect_success 'multi-pack-index *.rev cleanup with --object-dir' '
205 + git init repo &&
206 + git clone -s repo alternate &&
207 +
208 + test_when_finished "rm -rf repo alternate" &&
209 +
210 + (
211 + cd repo &&
212 + test_commit base &&
213 + git repack -d
214 + ) &&
215 +
216 + ours="alternate/.git/objects/pack/multi-pack-index-123.rev" &&
217 + theirs="repo/.git/objects/pack/multi-pack-index-abc.rev" &&
218 + touch "$ours" "$theirs" &&
219 +
220 + (
221 + cd alternate &&
222 + git multi-pack-index --object-dir ../repo/.git/objects write
223 + ) &&
224 +
225 + # writing a midx in "repo" should not remove the .rev file in the
226 + # alternate
227 + test_path_is_file repo/.git/objects/pack/multi-pack-index &&
228 + test_path_is_file $ours &&
229 + test_path_is_missing $theirs
230 +'
231 +
232 test_expect_success 'warn on improper hash version' '
233 git init --object-format=sha1 sha1 &&
234 (