Re: [PATCH] t4129: fix setfacl-related permissions failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Adam

Apologies for the late reply.

On Wed, Dec 23, 2020 at 8:44 AM Adam Dinwoodie <adam@xxxxxxxxxxxxx> wrote:
>
> When running this test in Cygwin, it's necessary to remove the inherited
> access control lists from the Git working directory in order for later
> permissions tests to work as expected.

Nit: Although this sentence is correct and the bug was first found on
Cygwin, the test may fail in any other environment which has a default
ACL set. In this sense, I think we could perhaps rephrase the commit
message to something like this:

This test creates a couple files and expects their permissions to be
based on the umask. However, if the test's directory has a default ACL
set, it will be inherited by the created files, overriding the umask.
To work around that, the test attempts to remove the default ACL, but
it erroneously passes a nonexistent path to the setfacl command. Fix
that by passing the working directory.

> ---
>  t/t4129-apply-samemode.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t4129-apply-samemode.sh b/t/t4129-apply-samemode.sh
> index 41818d8315..576632f868 100755
> --- a/t/t4129-apply-samemode.sh
> +++ b/t/t4129-apply-samemode.sh
> @@ -78,7 +78,7 @@ test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree
>         test_config core.sharedRepository 0666 &&
>         (
>                 # Remove a default ACL if possible.
> -               (setfacl -k newdir 2>/dev/null || true) &&
> +               (setfacl -k . 2>/dev/null || true) &&

The change is obviously correct, thanks!

Reviewed-by: Matheus Tavares <matheus.bernardino@xxxxxx>



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux