Re: [PATCH v2] multi-pack-index: fix *.rev cleanups with --object-dir

On Mon, Aug 23, 2021 at 11:40:49AM +0200, Johannes Berg wrote:

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.

I'm not entirely convinced that writing a midx when not "inside" a repo
is something that we want to support. But if we do, then...

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.

...there's a helper in the test suite for doing this kind of "not in a
repo" test:

diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 3b6331f641..3981bf96d0 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh

@@ -211,11 +211,8 @@ test_expect_success 'multi-pack-index *.rev cleanup with --object-dir' '
 	) &&
 	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
-	) &&
+	nongit git multi-pack-index \
+		--object-dir="$PWD/objdir-test-repo/$objdir" write &&
 	test_path_is_file objdir-test-repo/$objdir/pack/multi-pack-index &&
 	test_path_is_missing $rev
 '

-Peff