Re: [PATCH v4 1/3] t: regression git needs safe.directory when using sudo

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

Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

> Note that because of the way sudo interacts with the system, a much
> more complete integration with the test framework will require a lot
> more work and that was therefore intentionally punted for now.
>
> The current implementation requires ...
> ...
> If it fails to run, then it means your local setup wouldn't work for the
> test because of the configuration sudo has or other system settings, and
> things that might help are to comment out sudo's secure_path config, and
> make sure that the account you are using has no restrictions on the
> commands it can run through sudo, just like is provided for the user in
> the CI.
>
> For example (assuming a username of marta for you) something probably
> similar to the following entry in your /etc/sudoers (or equivalent) file:
>
>   marta	ALL=(ALL:ALL) NOPASSWD: ALL
>
> Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
> Helped-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>

Very well written.

> +test_lazy_prereq SUDO '
> +	sudo -n id -u >u &&
> +	id -u root >r &&
> +	test_cmp u r &&
> +	command -v git >u &&
> +	sudo command -v git >r &&
> +	test_cmp u r
> +'

I vaguely recall mentions of older dash that lack "command -v" made
earlier, but implementations of dash I have handy seem to know it.
I am personally fine with this as this script has a very narrow and
limited audience in mind.

> +test_expect_success SUDO 'setup' '
> +	sudo rm -rf root &&
> +	mkdir -p root/r &&
> +	sudo chown root root &&
> +	(
> +		cd root/r &&
> +		git init
> +	)
> +'

So, "root/" is owned by root, "root/r" is owned by the tester.

> +test_expect_failure SUDO 'sudo git status as original owner' '
> +	(
> +		cd root/r &&
> +		git status &&

The tester runs "git status" in "root/r" owned by the tester and it
should succeed.

> +		sudo git status

We want the tester to be able to do the same while temporarily
becoming 'root' with "sudo", but we know it fails right now.

> +	)
> +'

Mental note.  We do not need root to be owned by 'root' with the
tests we see here.  Perhaps we would add some that requires it in
later patches.  We'll see.

> +# this MUST be always the last test
> +test_expect_success SUDO 'cleanup' '
> +	sudo rm -rf root
> +'
> +
> +test_done

So far, looking good.

Thanks.