[PATCH] multi-pack-index: fix --object-dir from outside repo

[PATCH] multi-pack-index: fix --object-dir from outside repo

[Johannes Berg]

If using --object-dir to point into a repo, 'write' 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.

Fixes: 38ff7cabb6b8 ("pack-revindex: write multi-pack reverse indexes")
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 midx.c                      | 10 +++++-----
 t/t5319-multi-pack-index.sh |  8 ++++++++
 2 files changed, 13 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..7f393e52409d 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -201,6 +201,14 @@ test_expect_success 'write midx with twelve packs' '
 
 compare_results_with_midx "twelve packs"
 
+test_expect_success 'multi-pack-index with --object-dir need not be in repo' '
+	p="$(pwd)" &&
+	rm -f $objdir/multi-pack-index &&
+	cd / &&
+	git multi-pack-index --object-dir="$p/$objdir" write &&
+	cd "$p"
+'
+
 test_expect_success 'warn on improper hash version' '
 	git init --object-format=sha1 sha1 &&
 	(
-- 
2.31.1

Re: [PATCH] multi-pack-index: fix --object-dir from outside repo

[Derrick Stolee]

On 8/20/2021 3:35 PM, Johannes Berg wrote:
> If using --object-dir to point into a repo, 'write' 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.

Thanks for finding this! It's difficult to cover all of
the cases, but I'm glad you found this and added a test.

> +test_expect_success 'multi-pack-index with --object-dir need not be in repo' '
> +	p="$(pwd)" &&
> +	rm -f $objdir/multi-pack-index &&
> +	cd / &&
> +	git multi-pack-index --object-dir="$p/$objdir" write &&
> +	cd "$p"

Why are you using "cd /" here? Even if you mean to use "cd",
please do so within a sub-shell.

Could you instead init a new repo within the current directory
and point the object-dir to that location?

It could look something like this, (warning: I did not test this)

	git init other &&
	test_commit -C other first &&
	git multi-pack-index --object-dir=other/.git/objects write

And is the only post-condition you are checking that we do not
crash? Or is there a specific result you are looking for? For
instance, we can double check that the MIDX was written:

	test_path_is_file other/.git/objects/pack/multi-pack-index

but also you seem to be touching areas that delete files. Could
we 'touch' some of those and then see them get deleted?

Thanks,
-Stolee

Re: [PATCH] multi-pack-index: fix --object-dir from outside repo

[Taylor Blau]

On Fri, Aug 20, 2021 at 09:35:04PM +0200, Johannes Berg wrote:
> If using --object-dir to point into a repo, 'write' 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.

Thanks for CC'ing me; I have definitely been wondering about the
intended behavior of `--object-dir` on the list recently [1].

I think your patch message could use some clarifying, though.  Invoking

    cd $REPO/..
    git multi-pack-index write --object-dir=$REPO/.git/objects

has... different behavior depending on which side of the "write"
argument you put `--object-dir". On the left-hand side (i.e.,
"--object-dir=... write", you get something like:

    cd $REPO/..
    git multi-pack-index --object-dir=$REPO/.git/objects write

    zsh: segmentation fault  git.compile multi-pack-index ...

because the_repository->objects->odb isn't initialized (so reading
`path` in `clear_midx_files_ext` crashes). But in the opposite order
(i.e., "write --object-dir=...") you get:

    BUG: environment.c:280: git environment hasn't been setup
    zsh: abort      git.compile multi-pack-index write

because we catch that case much earlier in get_object_directory(). Why?
Because cmd_multi_pack_index() fills in the value of object_dir with
get_object_directory() if it isn't filled in already, but seeing "write"
causes us to stop parsing and dispatch to the sub-command
cmd_multi_pack_index_write().

I discussed this a little in [1] also (see the part about using
RUN_SETUP instead). There are definitely different ways to handle that;
you could equally imagine only dying if we were both outside of a Git
repository and didn't point at one via `--object-dir`.

But that's separate from another issue which is fixed by your patch
which is that we don't respect the value of `--object-dir` when cleaning
up MIDX .rev files via clear_midx_files_ext().

Your fix there (to use the path of an object_dir instead of a repository
struct) makes sense (since we don't ever fill in a repository struct
corresponding to the `--object-dir` parameter from the MIDX code).

But I think that's a separate issue than the RUN_SETUP thing I mentioned
earlier, so I would probably consider breaking this into two patches,
the first which addresses the RUN_SETUP thing, and the second which is
this fix.

>  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);

We can rely on this value always being non-NULL, so this is good.

> -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);

And here's the most important part of the change, which is obviously
correct. But note to other reviewers that this has nothing to do with
the RUN_SETUP issue I mentioned earlier, since
for_each_file_in_pack_dir() doesn't care about that.

> +test_expect_success 'multi-pack-index with --object-dir need not be in repo' '
> +	p="$(pwd)" &&
> +	rm -f $objdir/multi-pack-index &&
> +	cd / &&
> +	git multi-pack-index --object-dir="$p/$objdir" write &&
> +	cd "$p"
> +'
> +

I agree with Stolee that there should be a new repo created within the
current working directory, that way you can "cd .." and be both outside
of the repo you just created, but not outside of the test environment.

But let's make sure that we're not deleting any files that we should be
leaving alone. So it might be good to do something like:

    git init repo &&
    test_when_finished "rm -fr repo" &&
    (
      cd repo &&

      test_commit base &&
      git repack -d &&
    ) &&

    rev="$objdir/pack/multi-pack-index-$(midx_checksum $objdir).rev" &&
    touch $rev &&

    git multi-pack-index write --object-dir=repo/.git/objects &&

    test_path_is_file repo/.git/objects/pack/multi-pack-index &&
    test_path_is_file repo/.git/objects/multi-pack-index &&
    test_path_is_file $objdir/pack/multi-pack-index &&
    test_path_is_file $rev

That isn't testing the "invoked from a non-repo, but --object-dir" is
given case, but I think that's fine since they really are separate
things.

Note also that midx_checksum doesn't exist, but it is merely a wrapper
over a test-tool that prints out (for a multi_pack_index "m") `m->data +
m->data_len - the_hash_algo->rawsz`.

So between splitting the patch, clarifying the patch message, and
implementing support for this new test helper, this may be more of a
project than you were bargaining for ;). Let me know if you want any
help. I also don't mind taking care of it myself, since I promised in
[1] that I'd fix this issue anyway.

Thanks,
Taylor

[1]: https://lore.kernel.org/git/YQMFIljXl7sAAA%2FL@nand.local/

Re: [PATCH] multi-pack-index: fix --object-dir from outside repo

[Johannes Berg]

Hi Derrick,

> > +test_expect_success 'multi-pack-index with --object-dir need not be in repo' '
> > +	p="$(pwd)" &&
> > +	rm -f $objdir/multi-pack-index &&
> > +	cd / &&
> > +	git multi-pack-index --object-dir="$p/$objdir" write &&
> > +	cd "$p"
> 
> Why are you using "cd /" here? 
> 

I just needed to go outside the current test git directory, the tests
are running in a way that the current working directory is already the
git tree I'm operating in.

> Even if you mean to use "cd",
> please do so within a sub-shell.

I thought about it, but clearly all the tests are run in a sub-shell, so
it didn't seem necessary? But happy to change, I don't really care
either way.

Could you instead init a new repo within the current directory
and point the object-dir to that location?

I guess I could, but all the other stuff in here is already making a new
repo in the current working dir, and already initializing it with
objects, etc.

Doing it all over again seemed like a waste of time?

It could look something like this, (warning: I did not test this)

	git init other &&
	test_commit -C other first &&
	git multi-pack-index --object-dir=other/.git/objects write

Sure.

Actually, this won't work to test for the crash, I'd have to do
something like

git init other
test_commit -C other first &&
(
mkdir non-git &&
cd non-git &&
git multi-pack-index --object-dir=../other/.git/objects write
)

or so.

And is the only post-condition you are checking that we do not
crash?

Yes, I was assuming that it'd actually work at that point - maybe not
the best assumption, it could (erroneously) exit with a 0 exit status
but have done nothing.

> Or is there a specific result you are looking for? For
instance, we can double check that the MIDX was written:

	test_path_is_file other/.git/objects/pack/multi-pack-index

So I guess that would be a good idea.

but also you seem to be touching areas that delete files. Could
we 'touch' some of those and then see them get deleted?

Ah, well, that's the underlying issue but I'm not sure we even ever get
to that code? Then again, yes, the *.rev files should get removed, I'll
see - not even sure I know how to get them to be generated in the first
place, is that even supported already?

johannes

Re: [PATCH] multi-pack-index: fix --object-dir from outside repo

[Johannes Berg]

Hi Taylor,

> Thanks for CC'ing me; I have definitely been wondering about the
> intended behavior of `--object-dir` on the list recently [1].

Oh, hah. Not really being a contributor I'm not following the list,
thanks for the pointer.

> I think your patch message could use some clarifying, though.  Invoking
> 
>     cd $REPO/..
>     git multi-pack-index write --object-dir=$REPO/.git/objects
> 
> has... different behavior depending on which side of the "write"
> argument you put `--object-dir".

Wait what?! To be honest, I didn't expect it to even be valid on the
right-hand side of "write".

>  On the left-hand side (i.e.,
> "--object-dir=... write", you get something like:
> 
>     cd $REPO/..
>     git multi-pack-index --object-dir=$REPO/.git/objects write
> 
>     zsh: segmentation fault  git.compile multi-pack-index ...
> 
> because the_repository->objects->odb isn't initialized (so reading
> `path` in `clear_midx_files_ext` crashes).

Right, this is what I was doing.

>  But in the opposite order
> (i.e., "write --object-dir=...") you get:
> 
>     BUG: environment.c:280: git environment hasn't been setup
>     zsh: abort      git.compile multi-pack-index write
> 
> because we catch that case much earlier in get_object_directory(). Why?
> Because cmd_multi_pack_index() fills in the value of object_dir with
> get_object_directory() if it isn't filled in already, but seeing "write"
> causes us to stop parsing and dispatch to the sub-command
> cmd_multi_pack_index_write().

Great ...

But why do we even support both? What would the semantic difference be?

I'd be happy with either one of them working I guess :)

> I discussed this a little in [1] also (see the part about using
> RUN_SETUP instead). There are definitely different ways to handle that;
> you could equally imagine only dying if we were both outside of a Git
> repository and didn't point at one via `--object-dir`.
> 
> But that's separate from another issue which is fixed by your patch
> which is that we don't respect the value of `--object-dir` when cleaning
> up MIDX .rev files via clear_midx_files_ext().

Well, I _mostly_ meant to fix the crash, but yes, this is really the
underlying issue, and indeed we should clean up the .rev files in this
case as well.

> Your fix there (to use the path of an object_dir instead of a repository
> struct) makes sense (since we don't ever fill in a repository struct
> corresponding to the `--object-dir` parameter from the MIDX code).
> 
> But I think that's a separate issue than the RUN_SETUP thing I mentioned
> earlier, so I would probably consider breaking this into two patches,
> the first which addresses the RUN_SETUP thing, and the second which is
> this fix.

I never wanted to fix the other issue though ;-)

And honestly, I think I don't understand the discussion at [1] well
enough to really submit a patch for it.

> > +test_expect_success 'multi-pack-index with --object-dir need not be in repo' '
> > +	p="$(pwd)" &&
> > +	rm -f $objdir/multi-pack-index &&
> > +	cd / &&
> > +	git multi-pack-index --object-dir="$p/$objdir" write &&
> > +	cd "$p"
> > +'
> > +
> 
> I agree with Stolee that there should be a new repo created within the
> current working directory, that way you can "cd .." and be both outside
> of the repo you just created, but not outside of the test environment.

OK, fair enough, I'll resubmit.

> But let's make sure that we're not deleting any files that we should be
> leaving alone. So it might be good to do something like:
> 
>     git init repo &&
>     test_when_finished "rm -fr repo" &&
>     (
>       cd repo &&
> 
>       test_commit base &&
>       git repack -d &&
>     ) &&
> 
>     rev="$objdir/pack/multi-pack-index-$(midx_checksum $objdir).rev" &&
>     touch $rev &&

Hah, so you just manually pretend it was there - and meanwhile I was
looking for a way to get git to generate one :)

> 
>     git multi-pack-index write --object-dir=repo/.git/objects &&

Now this has the order of arguments the other way around, why?

>     test_path_is_file repo/.git/objects/pack/multi-pack-index &&
>     test_path_is_file repo/.git/objects/multi-pack-index &&
>     test_path_is_file $objdir/pack/multi-pack-index &&
>     test_path_is_file $rev

Why would test_path_is_file? Seems like it should be !test_path_is_file?

> 
> That isn't testing the "invoked from a non-repo, but --object-dir" is
> given case, but I think that's fine since they really are separate
> things.

But that's the one thing I really want to work :)

> Note also that midx_checksum doesn't exist, but it is merely a wrapper
> over a test-tool that prints out (for a multi_pack_index "m") `m->data +
> m->data_len - the_hash_algo->rawsz`.
> 
> So between splitting the patch, clarifying the patch message, and
> implementing support for this new test helper, this may be more of a
> project than you were bargaining for ;).
> 

Sounds like ;)
But actually we don't really care about the midx_checksum here, afaict?
MIDX_WRITE_REV_INDEX isn't ever set, so the rev files are not created
today?

> Let me know if you want any
> help. I also don't mind taking care of it myself, since I promised in
> [1] that I'd fix this issue anyway.

Thanks :)
How about I resubmit this patch with some of the edits, especially with
test for the case I care about (--object-dir used from a non-git place
to point elsewhere) and then you can build on top of that?

Thanks,
johannes

> [1]: https://lore.kernel.org/git/YQMFIljXl7sAAA%2FL@nand.local/
> 

Re: [PATCH] multi-pack-index: fix --object-dir from outside repo

[Junio C Hamano]

Johannes Berg <johannes@sipsolutions.net> writes:

> I just needed to go outside the current test git directory, the tests
> are running in a way that the current working directory is already the
> git tree I'm operating in.
>
>> Even if you mean to use "cd",
>> please do so within a sub-shell.
>
> I thought about it, but clearly all the tests are run in a sub-shell, so
> it didn't seem necessary? But happy to change, I don't really care
> either way.

Please learn to care before you write your next test, then ;-)

These tests are not run in a sub-shell; they are eval'ed, so that
the assignment they make to variables can persist and affect the
next test piece.

Thanks.

Re: [PATCH] multi-pack-index: fix --object-dir from outside repo

[Johannes Berg]

On Mon, 2021-08-23 at 01:05 -0700, Junio C Hamano wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
> 
> > I just needed to go outside the current test git directory, the tests
> > are running in a way that the current working directory is already the
> > git tree I'm operating in.
> > 
> > > Even if you mean to use "cd",
> > > please do so within a sub-shell.
> > 
> > I thought about it, but clearly all the tests are run in a sub-shell, so
> > it didn't seem necessary? But happy to change, I don't really care
> > either way.
> 
> Please learn to care before you write your next test, then ;-)

Hey now, I'm fixing your segfaults ;-)

> These tests are not run in a sub-shell; they are eval'ed, so that
> the assignment they make to variables can persist and affect the
> next test piece.

Makes sense. FWIW, the test *did* restore the CWD so things worked, and
subshells are actually ugly (need to import test-lib-functions.sh again
if you want to use those), but I'll make it work somehow.


More importantly, how do you feel about the "cd /"?

The tests are always run in a place where there's a parent git folder
(even if it's git itself), so you cannot reproduce the segfault in a
test without the "cd /", though I guess "cd /tmp" would also work or
something, but "cd /" felt pretty safe, hopefully not many people have
"/.git" on their system.

johannes

Re: [PATCH] multi-pack-index: fix --object-dir from outside repo

[Derrick Stolee]

On 8/23/2021 4:10 AM, Johannes Berg wrote:
> On Mon, 2021-08-23 at 01:05 -0700, Junio C Hamano wrote:
>> Johannes Berg <johannes@sipsolutions.net> writes:
>>
>>> I just needed to go outside the current test git directory, the tests
>>> are running in a way that the current working directory is already the
>>> git tree I'm operating in.
>>>
>>>> Even if you mean to use "cd",
>>>> please do so within a sub-shell.
>>>
>>> I thought about it, but clearly all the tests are run in a sub-shell, so
>>> it didn't seem necessary? But happy to change, I don't really care
>>> either way.
>>
>> Please learn to care before you write your next test, then ;-)
> 
> Hey now, I'm fixing your segfaults ;-)
> 
>> These tests are not run in a sub-shell; they are eval'ed, so that
>> the assignment they make to variables can persist and affect the
>> next test piece.
> 
> Makes sense. FWIW, the test *did* restore the CWD so things worked,

This assumes that your test completes to run the second "cd".

> and
> subshells are actually ugly (need to import test-lib-functions.sh again
> if you want to use those), but I'll make it work somehow.

We just add subshells this way:

test_expect_success 'test name' '
	prep_step &&
	(
		# now in a subshell
		cd wherever &&
		do things
		# don't need to cd again
	) &&
	continue test
'

> More importantly, how do you feel about the "cd /"?
>
> The tests are always run in a place where there's a parent git folder
> (even if it's git itself), so you cannot reproduce the segfault in a
> test without the "cd /", though I guess "cd /tmp" would also work or
> something, but "cd /" felt pretty safe, hopefully not many people have
> "/.git" on their system.

Don't leave the directory your test is set up to run in.

Git has a very large test suite full of examples to use for inspiration.
If you do not see a pattern used within the test suite, then there is
probably good reason to avoid that pattern.

Thanks,
-Stolee

Re: [PATCH] multi-pack-index: fix --object-dir from outside repo

[Johannes Berg]

On Mon, 2021-08-23 at 09:19 -0400, Derrick Stolee wrote:
> 
> We just add subshells this way:
> 
> test_expect_success 'test name' '
> 	prep_step &&
> 	(
> 		# now in a subshell
> 		cd wherever &&
> 		do things
> 		# don't need to cd again
> 	) &&
> 	continue test
> '

Sure. I know how to do subshells :)

My point was that inside the subshell you cannot do test_path_is_file
and similar, because the subshell didn't import the libs.

> > More importantly, how do you feel about the "cd /"?
> > 
> > The tests are always run in a place where there's a parent git folder
> > (even if it's git itself), so you cannot reproduce the segfault in a
> > test without the "cd /", though I guess "cd /tmp" would also work or
> > something, but "cd /" felt pretty safe, hopefully not many people have
> > "/.git" on their system.
> 
> Don't leave the directory your test is set up to run in.

I was specifically asking Junio ;-)

But realistically, if this is the requirement you want to impose, then
you _cannot_ test for the segfault within git's test suite. Your loss.

johannes

Re: [PATCH] multi-pack-index: fix --object-dir from outside repo

[Junio C Hamano]

Johannes Berg <johannes@sipsolutions.net> writes:

> Makes sense. FWIW, the test *did* restore the CWD so things worked, and
> subshells are actually ugly (need to import test-lib-functions.sh again
> if you want to use those), but I'll make it work somehow.

You do not need to dot-include test-lib-functions or anything ugly
or special.  The variables (not only the exported ones but regular
shell variables) and shell functions that is visible immediately
before you enter the opening "(" are all visible in the subshell.

The only notable difference you need to keep in mind when using
subshell is that you cannot affect variables and environment in
general of the calling shell.  In this case, you are taking
advantage of it---no matter where you chdir to, the main test
procedure that spawned the subshell will not be affected even if
your tests fail inside a subshell.  But it also disallows you from
doing certain things that rely on the ability to modify shell
variables, like setting up test_when_finished clean-up routine.

> More importantly, how do you feel about the "cd /"?

Please don't.  If somebody had a repository in /.git and the program
you are testing is buggy, you'd risk destroying it.  In general, it
is not a good idea to step outside the test directory you are given,
especially if you are *not* limiting yourself to read-only operation.

> The tests are always run in a place where there's a parent git folder
> (even if it's git itself), so you cannot reproduce the segfault in a
> test without the "cd /"

There is a "nongit" test helper in the test suite.  Would that work
for your case?

Thanks.

Re: [PATCH] multi-pack-index: fix --object-dir from outside repo

[Junio C Hamano]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Mon, 2021-08-23 at 09:19 -0400, Derrick Stolee wrote:
>> 
>> We just add subshells this way:
>> 
>> test_expect_success 'test name' '
>> 	prep_step &&
>> 	(
>> 		# now in a subshell
>> 		cd wherever &&
>> 		do things
>> 		# don't need to cd again
>> 	) &&
>> 	continue test
>> '
>
> Sure. I know how to do subshells :)
>
> My point was that inside the subshell you cannot do test_path_is_file
> and similar, because the subshell didn't import the libs.

Everything the call to prep_step, and anything that came before that
call, did to the environment, like setting shell functions and
variables, is visible inside the ( ... subshell ... ).

> I was specifically asking Junio ;-)
>
> But realistically, if this is the requirement you want to impose, then
> you _cannot_ test for the segfault within git's test suite. Your loss.

With that "cannot", I think you are assuming too much.  

Because Git is a fairly long-lived project, we've had our share of
cases where we needed to test for bugs that happen only when outside
a repository.

And we have facility for just that, it's called "nongit" test helper
that comes from test-lib-functions.sh, which is dot-included already
so your subshells get it for free.

t5300-pack-object.sh, for example, wants to make sure that the "git
index-pack --stdin" command invoked in a directory that is not a
repository, but "git index-pack <packfile>" works outside a
repository, and has two tests that uses the nongit helper.

Thanks.