Re: [PATCH v5 3/4] receive-pack: Protect current branch for bare repository worktree

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 10, 2021, 04:00 UTC
Message-ID: <211110.86tugkirnh.gmgdl@evledraar.gmail.com>
In-Reply-To: <20211109230941.2518143-3-andersk@mit.edu>

On Tue, Nov 09 2021, Anders Kaseorg wrote:

Show 16 quoted lines

> +test_expect_success 'denyCurrentBranch and bare repository worktrees' '
> +	test_when_finished "rm -fr bare.git" &&
> +	git clone --bare . bare.git &&
> +	git -C bare.git worktree add wt &&
> +	test_commit grape &&
> +	test_config -C bare.git receive.denyCurrentBranch refuse &&
> +	test_must_fail git push bare.git HEAD:wt &&
> +	test_config -C bare.git receive.denyCurrentBranch updateInstead &&
> +	git push bare.git HEAD:wt &&
> +	test_path_exists bare.git/wt/grape.t &&
> +	test_must_fail git push --delete bare.git wt
> +'
> +
>  test_expect_success 'refuse fetch to current branch of worktree' '
>  	test_when_finished "git worktree remove --force wt && git branch -D wt" &&
>  	git worktree add wt &&

Nit: Pick either a "git init sub-repo" or "rm -rf when-done.git" pattern
as you're doing here, or test_config. It doesn't make sense to combine
the two.

We don't need to run around in test_when_finished and unset config for something we're about to "rm -rf" anyway.

I think it's good practice to avoid test_config whenever possible, i.e. it's made redundant by using a sturdier test pattern of not needlessly sharing state.

But when that's needed, i.e. you need one persistent repo you're modifying, is when it should be used.