Re: [PATCH v2 1/3] config: use gitdir to get worktree config

[Glen Choo <chooglen@google.com>]

"Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Finally, add tests to 't3007-ls-files-recurse-submodules.sh' to verify the
> corrected config is loaded. Use 'ls-files' to test this because, unlike some
> other '--recurse-submodules' commands, 'ls-files' parses the config of the
> submodule in the same process as the superproject (via 'show_submodule()' ->
> 'repo_read_index()' -> 'prepare_repo_settings()'). As a result,
> 'the_repository' points to the config of the superproject but the
> commondir/gitdir in the config sequence will be that of the submodule,
> providing the exact scenario needed to verify this patch.
>
> The first test ('--recurse-submodules parses submodule repo config') checks
> that the submodule's *repo* config is read when running 'ls-files' on the
> superproject; this confirms already-working behavior, serving as a reference
> for how worktree config parsing should behave. The second test
> ('--recurse-submodules parses submodule worktree config') tests the same
> scenario as the previous but instead using the *worktree* config,
> demonstrating the corrected behavior. The 'test_config' helper is extended
> for this case so that it properly applies the '--worktree' option to the
> configure/unconfigure operations it performs.

Thanks! This version is really clear.

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 6e19ebc922a..b3864e22e9a 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -542,8 +542,17 @@ test_config () {
>  		config_dir=$1
>  		shift
>  	fi
> -	test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} '$1'" &&
> -	git ${config_dir:+-C "$config_dir"} config "$@"
> +
> +	# If --worktree is provided, use it to configure/unconfigure
> +	is_worktree=
> +	if test "$1" = --worktree
> +	then
> +		is_worktree=1
> +		shift
> +	fi
> +
> +	test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} ${is_worktree:+--worktree} '$1'" &&
> +	git ${config_dir:+-C "$config_dir"} config ${is_worktree:+--worktree} "$@"
>  }
>  
>  test_config_global () {

The test_config() change looks good to me. In the future, I think we
should extend this so that we can get rid of test_config_global() (which
is basically the same thing) and have only have one way of doing this.