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>

diff --git a/midx.c b/midx.c
index 321c6fdd2f184ad8842be703a072413699a74a9d..902e1a7a7d9df1ff77ad9d9eb7fc8e54a7ddb1c2 100644 (file)
--- 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 9034e94c0a78e180b45f2fa1bb76c517a6e241db..e953cdd6d1ce284a1c690ff77a02b11dbfeb28fe 100755 (executable)
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -201,6 +201,34 @@ 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 repo &&
+       git clone -s repo alternate &&
+
+       test_when_finished "rm -rf repo alternate" &&
+
+       (
+               cd repo &&
+               test_commit base &&
+               git repack -d
+       ) &&
+
+       ours="alternate/.git/objects/pack/multi-pack-index-123.rev" &&
+       theirs="repo/.git/objects/pack/multi-pack-index-abc.rev" &&
+       touch "$ours" "$theirs" &&
+
+       (
+               cd alternate &&
+               git multi-pack-index --object-dir ../repo/.git/objects write
+       ) &&
+
+       # writing a midx in "repo" should not remove the .rev file in the
+       # alternate
+       test_path_is_file repo/.git/objects/pack/multi-pack-index &&
+       test_path_is_file $ours &&
+       test_path_is_missing $theirs
+'
+
 test_expect_success 'warn on improper hash version' '
        git init --object-format=sha1 sha1 &&
        (