Re: [PATCH v2] multi-pack-index: fix *.rev cleanups with --object-dir
- From
Jeff King <peff@peff.net>
- Date
- Aug 23, 2021, 16:06 UTC
- Message-ID
- <YSPHdofrDOQk3xmy@coredump.intra.peff.net>
- In-Reply-To
- <20210823094049.44136-1-johannes@sipsolutions.net>
On Mon, Aug 23, 2021 at 11:40:49AM +0200, Johannes Berg wrote:
Show 13 quoted lines
> 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...
Show 6 quoted lines
> 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