| Message ID | 20201223114431.4595-1-adam@dinwoodie.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a1e03535db70eea3ffdff0ca0ec286e489cda648 |
| Headers | show |
| Series | t4129: fix setfacl-related permissions failure | expand |
Hi, Adam Apologies for the late reply. On Wed, Dec 23, 2020 at 8:44 AM Adam Dinwoodie <adam@dinwoodie.org> 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@usp.br>
Matheus Tavares Bernardino <matheus.bernardino@usp.br> writes: > Hi, Adam > > Apologies for the late reply. > > On Wed, Dec 23, 2020 at 8:44 AM Adam Dinwoodie <adam@dinwoodie.org> 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@usp.br> Thanks, both. Will queue.
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) && umask 0077 && # Test both files (f1) and leading dirs (d)
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. As such, fix an error in the test script so that the ACLs are set for the working directory, not a nonexistent subdirectory. Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org> --- t/t4129-apply-samemode.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)