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

From: Derrick Stolee <stolee@gmail.com>
Date: Aug 22, 2021, 23:51 UTC
Message-ID: <04ed58aa-94fa-010e-f4db-f41cd51876a5@gmail.com>
In-Reply-To: <20210820193504.37044-1-johannes@sipsolutions.net>

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.

Show 6 quoted lines

> +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