{"thread":{"id":"23027","subject":"[PATCH 0/5] tweak t1304 ACL tests so they work on Solaris","startedAt":"2010-03-15T17:14:32Z","lastAt":"2010-03-15T18:53:59Z","messageCount":12,"participants":["Brandon Casey","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"136850","messageId":"YowxgPZqaOkg2MZzY1CxL1fbmUZ7CMUf5DgqI3S0X83DgqLNI9AGfI83JRNT8LWYHqXBB-PGS1Q@cipher.nrlssc.navy.mil","threadId":"23027","inReplyTo":null,"subject":"[PATCH 0/5] tweak t1304 ACL tests so they work on Solaris","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T17:14:32Z","receivedAt":"2010-03-15T17:14:32Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"The operaton of setfacl and the interaction of mode bits with ACL seem\nto differ on Solaris and Linux.  The current t1304 tests fail on Solaris\n7 and 10.\n\nThe following patch series seems to fix the tests on Solaris 10.  Solaris\n7 still fails t1304.3 for some unknown reason.  I used the script below to\ndo some testing.\n\nOn Linux it prints out:\n\n  Initialized empty Git repository in /var/tmp/test_acl2/.git/\n  [master (root-commit) 22176cb] test commit\n   1 files changed, 1 insertions(+), 0 deletions(-)\n   create mode 100644 foo.txt\n  -r--r-----+ 1 casey NRL7240 29 Mar 15 11:47 .git/objects/90/bfcb510602aa11ae53a42dcec18ea39fbd8cec\n  # file: .git/objects/90/bfcb510602aa11ae53a42dcec18ea39fbd8cec\n  # owner: casey\n  # group: NRL7240\n  user::r--\n  user:display:rwx\t#effective:r--\n  group::---\n  mask::r--\n  other::---\n\n  -r--r-----+ 1 casey NRL7240 225 Mar 15 11:47 .git/objects/pack/pack-0611d0bcfb48f3db9f0d51334a035289272dbb2c.pack\n  # file: .git/objects/pack/pack-0611d0bcfb48f3db9f0d51334a035289272dbb2c.pack\n  # owner: casey\n  # group: NRL7240\n  user::r--\n  user:display:rwx\t#effective:r--\n  group::---\n  mask::r--\n  other::---\n\n\nOn Solaris 10 it prints out:\n\n  Initialized empty Git repository in /var/tmp/test_acl2/.git/\n  [master (root-commit) 5e4c0c6] test commit\n   1 files changed, 1 insertions(+), 0 deletions(-)\n   create mode 100644 foo.txt\n  -r--------+  1 casey    NRL7240       29 Mar 15 11:49 .git/objects/90/bfcb510602aa11ae53a42dcec18ea39fbd8cec\n\n  # file: .git/objects/90/bfcb510602aa11ae53a42dcec18ea39fbd8cec\n  # owner: casey\n  # group: NRL7240\n  user::r--\n  user:display:rwx\t\t#effective:r--\n  group::---\t\t#effective:---\n  mask:r--\n  other:---\n  -r--------+  1 casey    NRL7240      225 Mar 15 11:49 .git/objects/pack/pack-fedf57238f710f6b713019fe02fddab7a63702d0.pack\n\n  # file: .git/objects/pack/pack-fedf57238f710f6b713019fe02fddab7a63702d0.pack\n  # owner: casey\n  # group: NRL7240\n  user::r--\n  user:display:rwx\t\t#effective:r--\n  group::---\t\t#effective:---\n  mask:r--\n  other:---\n\n\nOn Solaris 7 it prints out:\n\n  Initialized empty Git repository in /var/tmp/test_acl2/.git/\n  [master (root-commit) ca62c6f] test commit\n   1 files changed, 1 insertions(+), 0 deletions(-)\n   create mode 100644 foo.txt\n  -r--------+  1 casey    NRL7240       29 Mar 15 10:51 .git/objects/90/bfcb510602aa11ae53a42dcec18ea39fbd8cec\n\n  # file: .git/objects/90/bfcb510602aa11ae53a42dcec18ea39fbd8cec\n  # owner: casey\n  # group: NRL7240\n  user::r--\n  user:display:rwx\t\t#effective:r--\n  group::---\t\t#effective:---\n  mask:r--\n  other:---\n  -r--------+  1 casey    NRL7240      225 Mar 15 10:51 .git/objects/pack/pack-dc814256703ad9c01e6dd1080c7151f664c6382a.pack\n\n  # file: .git/objects/pack/pack-dc814256703ad9c01e6dd1080c7151f664c6382a.pack\n  # owner: casey\n  # group: NRL7240\n  user::r--\n  user:display:rwx\t\t#effective:---\n  group::---\t\t#effective:---\n  mask:---\n  other:---\n\n\nSo, on Solaris 7, the pack file incorrectly has effective permissions of\n'---' for the 'display' user.  For some reason the mask ACL was not\ninherited from the parent directory.\n\n\nWhen the patch series is applied on top of 7aba6185, which introduced these\ntests, Linux still correctly fails t1304.3, but t1304.2 now passes.  This is\nok since t1304.2 was only failing because git gave object files more\npermissive file permissions than what were requested, not because the ACL's\nwere clobbered.  Of course, when applied on top of master, all tests pass on\nLinux, and now on Solaris 10.\n\n\n--->8--- test_acl.sh --->8---\n#!/bin/sh\n\nalt_user=display\n\ntestcase=${1:-'1'}\n\ncase \"$testcase\" in\n   1) test_dir=/var/tmp/test_acl1 ;;\n   2) test_dir=/var/tmp/test_acl2\n      GIT_EXEC_PATH=`pwd`\n      PATH=`pwd`:$PATH\n      GITPERLLIB=`pwd`/perl/blib/lib\n      export GIT_EXEC_PATH PATH GITPERLLIB\n      ;;\n   *)\n      echo 1>&2 \"Usage: $0 [1|2]\"\n      exit 1\n      ;;\nesac\n\n\numask 077 &&\n  rm -rf \"$test_dir\" && mkdir \"$test_dir\" && cd \"$test_dir\" &&\n  setfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx . &&\n  setfacl -m d:u:$alt_user:rwx . &&\n  setfacl -m u:$alt_user:rwx,m:rwx . &&\n  git init &&\n  echo 'this is a test' > foo.txt &&\n  git add foo.txt &&\n  git commit -m 'test commit' &&\n  ls -l .git/objects/90/bfcb510602aa11ae53a42dcec18ea39fbd8cec &&\n  getfacl .git/objects/90/bfcb510602aa11ae53a42dcec18ea39fbd8cec &&\n  git gc &&\n  ls -l .git/objects/pack/*.pack &&\n  getfacl .git/objects/pack/*.pack\n--->8---\n\n\nBrandon Casey (5):\n  t/t1304: avoid -d option to setfacl\n  t/t1304: set the Default ACL base entries\n  t/t1304: use 'test -r' to test readability rather than looking at\n    mode bits\n  t/t1304: set the mask ACL that is checked in check_perms_and_acl\n  t/t1304: make a second colon optional in the mask ACL check\n\n t/t1304-default-acl.sh |   23 ++++++-----------------\n 1 files changed, 6 insertions(+), 17 deletions(-)\n"},{"id":"136851","messageId":"YowxgPZqaOkg2MZzY1CxL7osRwRHVqDoA4pDFlm_036BvsYP4hI0frMTwyMvLVDr6FREt89Ntxk@cipher.nrlssc.navy.mil","threadId":"23027","inReplyTo":"YowxgPZqaOkg2MZzY1CxL1fbmUZ7CMUf5DgqI3S0X83DgqLNI9AGfI83JRNT8LWYHqXBB-PGS1Q@cipher.nrlssc.navy.mil","subject":"[PATCH 1/5] t/t1304: avoid -d option to setfacl","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T17:14:33Z","receivedAt":"2010-03-15T17:14:33Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nSome platforms (Solaris) have a setfacl whose -d switch works differently\nthan the one on Linux.  On Linux, it causes all operations to be applied\nto the Default ACL.  There is a notation for operating on the Default ACL:\n\n   [d[efault]:] [u[ser]:]uid [:perms]\n\nso use it instead of the -d switch.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n t/t1304-default-acl.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex cc30be4..415a2dd 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -46,8 +46,8 @@ dirs_to_set=\"./ .git/ .git/objects/ .git/objects/pack/\"\n \n test_expect_success 'Setup test repo' '\n \tsetfacl -m u:root:rwx          $dirs_to_set &&\n-\tsetfacl -d -m u:\"$LOGNAME\":rwx $dirs_to_set &&\n-\tsetfacl -d -m u:root:rwx       $dirs_to_set &&\n+\tsetfacl -m d:u:\"$LOGNAME\":rwx  $dirs_to_set &&\n+\tsetfacl -m d:u:root:rwx        $dirs_to_set &&\n \n \ttouch file.txt &&\n \tgit add file.txt &&\n-- \n1.6.6.2\n"},{"id":"136852","messageId":"YowxgPZqaOkg2MZzY1CxL62AYRaulAaBTuVJyBy21GtGshrh3yFvpwnH4N2NnyXFt4HiCe6vR8E@cipher.nrlssc.navy.mil","threadId":"23027","inReplyTo":"YowxgPZqaOkg2MZzY1CxL1fbmUZ7CMUf5DgqI3S0X83DgqLNI9AGfI83JRNT8LWYHqXBB-PGS1Q@cipher.nrlssc.navy.mil","subject":"[PATCH 2/5] t/t1304: set the Default ACL base entries","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T17:14:34Z","receivedAt":"2010-03-15T17:14:34Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nAccording to the Linux setfacl man page, in order for an ACL to be valid,\nthe following rules must be satisfied:\n\n   * Whenever an ACL contains any Default ACL entries, the three Default\n     ACL base entries (default owner, default group, and default others)\n     must also exist.\n\n   * Whenever a Default ACL contains named user entries or named group\n     objects, it must also contain a default effective rights mask.\n\nSome implementations of setfacl (Linux) do this automatically when\nnecessary, some (Solaris) do not.  Solaris's setfacl croaks when trying to\ncreate a default user ACL if the above rules are not satisfied.  So, create\nthem before modifying the default user ACL's.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n t/t1304-default-acl.sh |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex 415a2dd..3a1532b 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -45,6 +45,7 @@ check_perms_and_acl () {\n dirs_to_set=\"./ .git/ .git/objects/ .git/objects/pack/\"\n \n test_expect_success 'Setup test repo' '\n+\tsetfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&\n \tsetfacl -m u:root:rwx          $dirs_to_set &&\n \tsetfacl -m d:u:\"$LOGNAME\":rwx  $dirs_to_set &&\n \tsetfacl -m d:u:root:rwx        $dirs_to_set &&\n-- \n1.6.6.2\n"},{"id":"136854","messageId":"YowxgPZqaOkg2MZzY1CxL61O63C5ULimj64t69Bc-wsW3IlWf4Zc9WmA3ihonAb8UtR5thcko00@cipher.nrlssc.navy.mil","threadId":"23027","inReplyTo":"YowxgPZqaOkg2MZzY1CxL1fbmUZ7CMUf5DgqI3S0X83DgqLNI9AGfI83JRNT8LWYHqXBB-PGS1Q@cipher.nrlssc.navy.mil","subject":"[PATCH 3/5] t/t1304: use 'test -r' to test readability rather than looking at mode bits","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T17:14:35Z","receivedAt":"2010-03-15T17:14:35Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nThis test was using the group read permission bit as an indicator of the\ndefault ACL mask.  This behavior is valid on Linux but not on other\nplatforms like Solaris.  So, rather than looking at mode bits, just test\nreadability for the user.  This, along with the checks for the existence\nof the ACL's that were set on the parent directories, should be enough.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n t/t1304-default-acl.sh |   15 +--------------\n 1 files changed, 1 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex 3a1532b..52246d7 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -20,21 +20,8 @@ if ! setfacl -m u:root:rwx .; then\n     test_done\n fi\n \n-modebits () {\n-\tls -l \"$1\" | sed -e 's|^\\(..........\\).*|\\1|'\n-}\n-\n check_perms_and_acl () {\n-\tactual=$(modebits \"$1\") &&\n-\tcase \"$actual\" in\n-\t-r--r-----*)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo \"Got permission '$actual', expected '-r--r-----'\"\n-\t\tfalse\n-\t\t;;\n-\tesac &&\n+\ttest -r \"$1\" &&\n \tgetfacl \"$1\" > actual &&\n \tgrep -q \"user:root:rwx\" actual &&\n \tgrep -q \"user:${LOGNAME}:rwx\" actual &&\n-- \n1.6.6.2\n"},{"id":"136853","messageId":"YowxgPZqaOkg2MZzY1CxLzeL4DIwxt_o3RKS9xen-cewKbECZA7OIb7mUaY2TbsVR_rHE5GTeU0@cipher.nrlssc.navy.mil","threadId":"23027","inReplyTo":"YowxgPZqaOkg2MZzY1CxL1fbmUZ7CMUf5DgqI3S0X83DgqLNI9AGfI83JRNT8LWYHqXBB-PGS1Q@cipher.nrlssc.navy.mil","subject":"[PATCH 4/5] t/t1304: set the mask ACL that is checked in check_perms_and_acl","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T17:14:36Z","receivedAt":"2010-03-15T17:14:36Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n t/t1304-default-acl.sh |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex 52246d7..85351ae 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -33,6 +33,7 @@ dirs_to_set=\"./ .git/ .git/objects/ .git/objects/pack/\"\n \n test_expect_success 'Setup test repo' '\n \tsetfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&\n+\tsetfacl -m m:rwx               $dirs_to_set &&\n \tsetfacl -m u:root:rwx          $dirs_to_set &&\n \tsetfacl -m d:u:\"$LOGNAME\":rwx  $dirs_to_set &&\n \tsetfacl -m d:u:root:rwx        $dirs_to_set &&\n-- \n1.6.6.2\n"},{"id":"136855","messageId":"YowxgPZqaOkg2MZzY1CxL2y9KYLK5N2WFlWwYu3VB6Y3RrTnbp2pATZ27RaMhDiRScZADoVUqz4@cipher.nrlssc.navy.mil","threadId":"23027","inReplyTo":"YowxgPZqaOkg2MZzY1CxL1fbmUZ7CMUf5DgqI3S0X83DgqLNI9AGfI83JRNT8LWYHqXBB-PGS1Q@cipher.nrlssc.navy.mil","subject":"[PATCH 5/5] t/t1304: make a second colon optional in the mask ACL check","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T17:14:37Z","receivedAt":"2010-03-15T17:14:37Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nSolaris only uses one colon in the listing of the ACL mask, Linux uses two,\nso substitute egrep for grep and make the second colon optional.\n\nThe -q option for Solaris 7's /usr/xpg4/bin/egrep does not appear to be\nimplemented, so redirect output to /dev/null.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n t/t1304-default-acl.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex 85351ae..055ad00 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -25,7 +25,7 @@ check_perms_and_acl () {\n \tgetfacl \"$1\" > actual &&\n \tgrep -q \"user:root:rwx\" actual &&\n \tgrep -q \"user:${LOGNAME}:rwx\" actual &&\n-\tgrep -q \"mask::r--\" actual &&\n+\tegrep \"mask::?r--\" actual > /dev/null 2>&1 &&\n \tgrep -q \"group::---\" actual || false\n }\n \n-- \n1.6.6.2\n"},{"id":"136856","messageId":"vpq3a017am6.fsf@bauges.imag.fr","threadId":"23027","inReplyTo":"YowxgPZqaOkg2MZzY1CxL61O63C5ULimj64t69Bc-wsW3IlWf4Zc9WmA3ihonAb8UtR5thcko00@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 3/5] t/t1304: use 'test -r' to test readability rather than looking at mode bits","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-03-15T17:33:53Z","receivedAt":"2010-03-15T17:33:53Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n> -\t-r--r-----*)\n> -\t\t: happy\n> -\t\t;;\n\n> +\ttest -r \"$1\" &&\n\nMaybe it makes sense to ask for -r--?-----* instead, to make sure we\ndon't give the read bit to anyone. But that's not really important\nsince we're protected by the directory's permission anyway.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"136857","messageId":"vpqy6ht5vvs.fsf@bauges.imag.fr","threadId":"23027","inReplyTo":"YowxgPZqaOkg2MZzY1CxLzeL4DIwxt_o3RKS9xen-cewKbECZA7OIb7mUaY2TbsVR_rHE5GTeU0@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 4/5] t/t1304: set the mask ACL that is checked in check_perms_and_acl","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-03-15T17:37:27Z","receivedAt":"2010-03-15T17:37:27Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n>  test_expect_success 'Setup test repo' '\n>  \tsetfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&\n> +\tsetfacl -m m:rwx               $dirs_to_set &&\n\nThe patch sounds right, but I don't understand the commit message. You\nset m:rwx, and check_perms_and_acl expects mask::r--, so it's not\nexactly what check_perms_and_acl checks.\n\nMy understanding is that you set the mask here to enforce the validity\nof the ACL, but then you may want to just squash this into [PATCH 2/5].\n\nBut again, that's bikeshedding, the patch serie looks right to me.\nSorry for not making the test right myself ;-).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"136858","messageId":"8fqqfdxYYa4RPD7FGUikyKCKHtfa5A5dhaSpyecsKHixseAVhiHOyw@cipher.nrlssc.navy.mil","threadId":"23027","inReplyTo":"vpq3a017am6.fsf@bauges.imag.fr","subject":"Re: [PATCH 3/5] t/t1304: use 'test -r' to test readability rather than looking at mode bits","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T17:51:24Z","receivedAt":"2010-03-15T17:51:24Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 03/15/2010 12:33 PM, Matthieu Moy wrote:\n> Brandon Casey <casey@nrlssc.navy.mil> writes:\n> \n>> -\t-r--r-----*)\n>> -\t\t: happy\n>> -\t\t;;\n> \n>> +\ttest -r \"$1\" &&\n> \n> Maybe it makes sense to ask for -r--?-----* instead, to make sure we\n> don't give the read bit to anyone. But that's not really important\n> since we're protected by the directory's permission anyway.\n\nI'm a little hesitant to add a test on the output from 'ls', since\nSolaris and Linux produce different output in the presence of ACL's.\n\n-brandon\n"},{"id":"136860","messageId":"D90FzSTc_sIuYUj84CpQ_qxlaiYTWQsa6KDiDVYCRLQ@cipher.nrlssc.navy.mil","threadId":"23027","inReplyTo":"vpqy6ht5vvs.fsf@bauges.imag.fr","subject":"Re: [PATCH 4/5] t/t1304: set the mask ACL that is checked in check_perms_and_acl","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T18:17:42Z","receivedAt":"2010-03-15T18:17:42Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 03/15/2010 12:37 PM, Matthieu Moy wrote:\n> Brandon Casey <casey@nrlssc.navy.mil> writes:\n> \n>>  test_expect_success 'Setup test repo' '\n>>  \tsetfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&\n>> +\tsetfacl -m m:rwx               $dirs_to_set &&\n> \n> The patch sounds right, but I don't understand the commit message. You\n> set m:rwx, and check_perms_and_acl expects mask::r--, so it's not\n> exactly what check_perms_and_acl checks.\n\nAh, yeah, it does sound like I'm saying that check_perms_and_acl is\nchecking for the particular mask that I'm setting.  I really meant\nit to read more like: since check_perms_and_acl is checking the\n'mask ACL', it should be set appropriately.\n\n> My understanding is that you set the mask here to enforce the validity\n> of the ACL, but then you may want to just squash this into [PATCH 2/5].\n\nI think the ACL is valid according to the rules stated in the Linux man\npage, but depending on the previously existing mask ACL on the directories,\nthe other ACL's that were set may or may not have any effect.  I think on\nLinux, the setfacl command updates the effective rights mask when other\nACL entries are modified.  I don't think this happens on Solaris.\n\nIf I do this:\n\n  $ cd /var/tmp &&\n    mkdir test &&\n    setfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx test &&\n    setfacl -m d:u:guest:rwx test &&\n    setfacl -m u:guest:rwx test &&\n    getfacl test\n\nOn Solaris I get:\n\n  # file: test\n  # owner: XXX\n  # group: XXX\n  user::rwx\n  user:guest:rwx                #effective:---\n  group::---              #effective:---\n  mask:---\n  other:---\n  default:user::rwx\n  default:user:guest:rwx\n  default:group::---\n  default:mask:rwx\n  default:other:---\n\nand on Linux I get:\n\n# file: test\n# owner: XXX\n# group: XXX\nuser::rwx\nuser:guest:rwx\ngroup::---\nmask::rwx\nother::---\ndefault:user::rwx\ndefault:user:guest:rwx\ndefault:group::---\ndefault:mask::rwx\ndefault:other::---\n\n\nNotice how the mask entry is different.  On Solaris you get --- and user\n'guest' effectively has no permissions, while on Linux it has full rwx.\nSo for the test we should set the mask explicitly.\n\n-brandon\n"},{"id":"136864","messageId":"1OHmOAV6PwNgTpkrT1e6TCcWFBFU-8KuDhDpWjm2BlkAUdaqw5vTu9fV5pO66uT2OLcDdYFhZAU@cipher.nrlssc.navy.mil","threadId":"23027","inReplyTo":"YowxgPZqaOkg2MZzY1CxLzeL4DIwxt_o3RKS9xen-cewKbECZA7OIb7mUaY2TbsVR_rHE5GTeU0@cipher.nrlssc.navy.mil","subject":"[PATCH 4/5 v2] t/t1304: set the ACL effective rights mask","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-03-15T18:35:03Z","receivedAt":"2010-03-15T18:35:03Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nSome implementations of setfacl do not recalculate the effective rights\nmask when the ACL is modified.  So, set the effective rights mask\nexplicitly to ensure that the ACL's that are set on the directories will\nhave effect.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nHere's a new patch with an improved commit message (hopefully).\n\n-brandon\n\n\n t/t1304-default-acl.sh |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh\nindex 52246d7..85351ae 100755\n--- a/t/t1304-default-acl.sh\n+++ b/t/t1304-default-acl.sh\n@@ -33,6 +33,7 @@ dirs_to_set=\"./ .git/ .git/objects/ .git/objects/pack/\"\n \n test_expect_success 'Setup test repo' '\n \tsetfacl -m d:u::rwx,d:g::---,d:o:---,d:m:rwx $dirs_to_set &&\n+\tsetfacl -m m:rwx               $dirs_to_set &&\n \tsetfacl -m u:root:rwx          $dirs_to_set &&\n \tsetfacl -m d:u:\"$LOGNAME\":rwx  $dirs_to_set &&\n \tsetfacl -m d:u:root:rwx        $dirs_to_set &&\n-- \n1.6.6.2\n"},{"id":"136866","messageId":"vpqr5nl2z7c.fsf@bauges.imag.fr","threadId":"23027","inReplyTo":"1OHmOAV6PwNgTpkrT1e6TCcWFBFU-8KuDhDpWjm2BlkAUdaqw5vTu9fV5pO66uT2OLcDdYFhZAU@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 4/5 v2] t/t1304: set the ACL effective rights mask","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-03-15T18:53:59Z","receivedAt":"2010-03-15T18:53:59Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n> Here's a new patch with an improved commit message (hopefully).\n\nYes, I do like it better.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}