Re: [PATCH 3/3] safe.directory: document and check that it's ignored in the environment

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 27, 2022, 20:42 UTC
Message-ID: <xmqqlevql0lj.fsf@gitster.g>
In-Reply-To: <20220427170649.4949-4-szeder.dev@gmail.com>

SZEDER Gábor <szeder.dev@gmail.com> writes:

Show 5 quoted lines

> The description of 'safe.directory' mentions that it's respected in
> the system and global configs, and ignored in the repository config
> and on the command line, but it doesn't mention whether it's respected
> or ignored when specified via environment variables (nor does the
> commit message adding 'safe.directory' [1]).

If we had GIT_SAFE_DIRECTORIES that lists the safe directories (like $PATH does), that would have been absolutely necessary to document how it works, but GIT_CONFIG_* is merely an implementation detail of how "git -c var=val" works and I am not sure if it is even a good idea to hardcode how they happen to work like these tests. The only thing the users should know is that GIT_CONFIG_{KEY,VALUE}_* are used internally by the implementation and they should not muck with it, no?

I am moderately negative about this change.

Show 26 quoted lines

> diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
> index 82dac0eb93..238b25f91a 100755
> --- a/t/t0033-safe-directory.sh
> +++ b/t/t0033-safe-directory.sh
> @@ -21,6 +21,21 @@ test_expect_success 'ignoring safe.directory on the command line' '
>  	grep "unsafe repository" err
>  '
>  
> +test_expect_success 'ignoring safe.directory in the environment' '
> +	test_must_fail env GIT_CONFIG_COUNT=1 \
> +		GIT_CONFIG_KEY_0="safe.directory" \
> +		GIT_CONFIG_VALUE_0="$(pwd)" \
> +		git status 2>err &&
> +	grep "unsafe repository" err
> +'
> +
> +test_expect_success 'ignoring safe.directory in GIT_CONFIG_PARAMETERS' '
> +	test_must_fail env \
> +		GIT_CONFIG_PARAMETERS="${SQ}safe.directory${SQ}=${SQ}$(pwd)${SQ}" \
> +		git status 2>err &&
> +	grep "unsafe repository" err
> +'
> +
>  test_expect_success 'ignoring safe.directory in repo config' '
>  	(
>  		unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&