Re: [PATCH v2] setup: remove unnecessary variable

[Junio C Hamano <gitster@pobox.com>]

Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:

> The TODO comment suggested to heed core.bare from template config file
> if no command line override given. And the prev_bare_repository
> variable seems to have been placed for this sole purpose as it is not
> used anywhere else.

OK.

> However, it was clarified by Junio [1] that such values (including
> core.bare) are ignored intentionally and does not make sense to
> propagate them from template config to repository config. Also, the
> directories for the worktree and repository are already created, and
> therefore the bare/non-bare decision has already been made, by the
> point we reach the codepath where the TODO comment is placed.

Correct.  Who said it is much less interesting than what was said,
so I would have written the first part of the paragraph more like

	Values including core.bare from the template file are
	ignored on purpose because they may not make sense for the
	repository being created [1].  Also, the directories for ...

but I'll let it pass.

> diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
> index b1eb5c01b8..29cf8a9661 100755
> --- a/t/t1301-shared-repo.sh
> +++ b/t/t1301-shared-repo.sh
> @@ -52,7 +52,7 @@ test_expect_success 'shared=all' '
>  	test 2 = $(git config core.sharedrepository)
>  '
>  
> -test_expect_failure 'template can set core.bare' '
> +test_expect_success 'template cannot set core.bare' '
>  	test_when_finished "rm -rf subdir" &&
>  	test_when_finished "rm -rf templates" &&
>  	test_config core.bare true &&
> @@ -60,18 +60,7 @@ test_expect_failure 'template can set core.bare' '
>  	mkdir -p templates/ &&
>  	cp .git/config templates/config &&
>  	git init --template=templates subdir &&
> -	test_path_exists subdir/HEAD
> +	test_path_is_missing subdir/HEAD
>  '

So we used to say "subdir should be created as a bare repository but
we fail to do so", but now "subdir should become a non-bare repository
because 'git init' is run without the --bare option".  OK.

> -
> -test_expect_success 'template can set core.bare but overridden by command line' '
> -	test_when_finished "rm -rf subdir" &&
> -	test_when_finished "rm -rf templates" &&
> -	test_config core.bare true &&
> -	umask 0022 &&
> -	mkdir -p templates/ &&
> -	cp .git/config templates/config &&
> -	git init --no-bare --template=templates subdir &&
> -	test_path_exists subdir/.git/HEAD
> -'

This removal is a bit unexpected.  Is it because we established with
the previous test that core.bare in the template should not affect
the outcome, so this is not worth testing?

> diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh
> index a400bcca62..e93e0d0cc3 100755
> --- a/t/t5606-clone-options.sh
> +++ b/t/t5606-clone-options.sh
> @@ -120,14 +120,14 @@ test_expect_success 'prefers -c config over --template config' '
>  
>  '
>  
> -test_expect_failure 'prefers --template config even for core.bare' '
> +test_expect_success 'ignore --template config for core.bare' '
>  
>  	template="$TRASH_DIRECTORY/template-with-bare-config" &&
>  	mkdir "$template" &&
>  	git config --file "$template/config" core.bare true &&
>  	git clone "--template=$template" parent clone-bare-config &&
> -	test "$(git -C clone-bare-config config --local core.bare)" = "true" &&
> -	test_path_is_file clone-bare-config/HEAD
> +	test "$(git -C clone-bare-config config --local core.bare)" = "false" &&
> +	test_path_is_missing clone-bare-config/HEAD
>  '

This is in the same spirit as the first change in t1301, which seems
OK.

Thanks.