On Fri, Apr 29, 2022 at 10:57:58AM -0700, Junio C Hamano wrote:
> Derrick Stolee <derrickstolee@xxxxxxxxxx> writes:
>
> > On 4/27/2022 4:37 PM, Junio C Hamano wrote:
> >> SZEDER Gábor <szeder.dev@xxxxxxxxx> writes:
> >>
> >>> According to the documentation 'safe.directory' "is only respected
> >>> when specified in a system or global config, not when it is specified
> >>> in a repository config or via the command line option -c
> >>> safe.directory=<path>".
> >>>
> >>> Add tests to check that 'safe.directory' in the repository config or
> >>> on the command line is indeed ignored.
> >>>
> >>> Signed-off-by: SZEDER Gábor <szeder.dev@xxxxxxxxx>
> >>> ---
> >>> t/t0033-safe-directory.sh | 13 +++++++++++++
> >>> 1 file changed, 13 insertions(+)
> >>>
> >>> diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
> >>> index 6f9680e8b0..82dac0eb93 100755
> >>> --- a/t/t0033-safe-directory.sh
> >>> +++ b/t/t0033-safe-directory.sh
> >>> @@ -16,6 +16,19 @@ test_expect_success 'safe.directory is not set' '
> >>> expect_rejected_dir
> >>> '
> >>>
> >>> +test_expect_success 'ignoring safe.directory on the command line' '
> >>> + test_must_fail git -c safe.directory="$(pwd)" status 2>err &&
> >>> + grep "unsafe repository" err
> >>> +'
> >>> +
> >>> +test_expect_success 'ignoring safe.directory in repo config' '
> >>> + (
> >>> + unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
> >>> + git config safe.directory "$(pwd)"
> >>> + ) &&
> >>> + expect_rejected_dir
> >>> +'
> >>
> >> I am debating myself if we want to remove the in-repository
> >> safe.directory configuration setting after this test piece is done,
> >> with test_when_finished. We just made sure, with this test, that
> >> having the variable does not affect anything, so the subsequent
> >> tests should not care hence it is probably OK. On the other hand,
> >> to make sure those settings they make (e.g. setting it globally is
> >> what the next test we can see below does) is what affects their
> >> outcome, it removes one more thing we need to worry about if we
> >> clean after ourselves. I dunno.
> >
> > test_config would do the same, right? I think it automatically
> > does the test_when_finished for us.
>
> I thought it (specifically, anything depends on test_when_finished)
> has trouble working well from inside a subprocess?
Yeah, test_config doesn't work in a subshell, because modifying
the variable holding the cleanup commands won't be visible after
leaving the subshell, and the protection added in 0968f12a99
(test-lib-functions: detect test_when_finished in subshell,
2015-09-05) will kick in. And we do need a subshell to set the
config, because without unsetting GIT_TEST_ASSUME_DIFFERENT_OWNER 'git
config' would refuse to touch the config file.
I think something like
test_when_finished "(
unset GIT_TEST_ASSUME_DIFFERENT_USER &&
git config --unset safe.directory
)"
would work, though.
I considered cleaning up the config file, but all subsequent tests
leave behind config settings for later tests as well (in the global
config file, though).
