{"thread":{"id":"18658","subject":"bsd group semantics","startedAt":"2009-03-31T11:26:38Z","lastAt":"2009-03-31T22:25:42Z","messageCount":9,"participants":["Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"110000","messageId":"20090331112637.GA1910@coredump.intra.peff.net","threadId":"18658","inReplyTo":null,"subject":"bsd group semantics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-31T11:26:38Z","receivedAt":"2009-03-31T11:26:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The recently added test t1301.17 is failing for me on FreeBSD; the\ncreated directories don't have the GID bit set.\n\nI traced this back to the fact that FORCE_DIR_SET_GID is a no-op on\nFreeBSD due to the DIR_HAS_BSD_GROUP_SEMANTICS flag being set. I am not\nsure I understand the original reasoning for this flag at all; GID seems\nto work just fine on my FreeBSD 6.1 test system.\n\nThe original thread is here:\n\n http://article.gmane.org/gmane.comp.version-control.git/75999\n\nIt looks like Alex was having the problem on FreeBSD 4. I'm not sure if\nthis flag can go away with the new set_shared_perm, or if I need to just\nenable on FreeBSD <= 4.\n\n-Peff\n"},{"id":"110021","messageId":"7vvdpp6623.fsf@gitster.siamese.dyndns.org","threadId":"18658","inReplyTo":"20090331112637.GA1910@coredump.intra.peff.net","subject":"Re: bsd group semantics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-31T15:55:00Z","receivedAt":"2009-03-31T15:55:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> The recently added test t1301.17 is failing for me on FreeBSD; the\n> created directories don't have the GID bit set.\n>\n> I traced this back to the fact that FORCE_DIR_SET_GID is a no-op on\n> FreeBSD due to the DIR_HAS_BSD_GROUP_SEMANTICS flag being set. I am not\n> sure I understand the original reasoning for this flag at all; GID seems\n> to work just fine on my FreeBSD 6.1 test system.\n\nIIRC, DIR_HAS_BSD_GROUP_SEMANTICS means you do not have to ask explicitly\nwith g+s to the filesystem to use the \"subdirectory is owned by the same\ngroup as its parent\" semantics. On SysV you have to ask; on BSD you do\nnot have to (and do not need FORCE_DIR_SET_GID).\n\n> The original thread is here:\n>\n> http://article.gmane.org/gmane.comp.version-control.git/75999\n\nThat one was not about \"you do not have to ask\", but \"you are not allowed\nto ask because the request will fail\". Perhaps between FBSD4 and FBSD6\nthings changed, and you can now make g+s request (which I presume is still\na no-op other than setting the bit on)?\n\nIdeally the test should be checking if the subdirectory is owned by the\nsame group as the toplevel, but that is rather hard to correctly arrange,\nas it depends on the set of groups the user who runs the test belongs to,\nhow the git work tree is set up (if it is owned by his primary group or a\nsecondary), etc.\n\nFor now, I'd suggest teaching the test not to care about g+s bit.\n"},{"id":"110024","messageId":"20090331160842.GA9019@coredump.intra.peff.net","threadId":"18658","inReplyTo":"7vvdpp6623.fsf@gitster.siamese.dyndns.org","subject":"Re: bsd group semantics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-31T16:08:42Z","receivedAt":"2009-03-31T16:08:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 31, 2009 at 08:55:00AM -0700, Junio C Hamano wrote:\n\n> IIRC, DIR_HAS_BSD_GROUP_SEMANTICS means you do not have to ask explicitly\n> with g+s to the filesystem to use the \"subdirectory is owned by the same\n> group as its parent\" semantics. On SysV you have to ask; on BSD you do\n> not have to (and do not need FORCE_DIR_SET_GID).\n\nOK, I see. Thanks for the explanation. So the test is wrong, since it\nexplicitly checks for g+s, and we only need it sometimes.\n\n> That one was not about \"you do not have to ask\", but \"you are not allowed\n> to ask because the request will fail\". Perhaps between FBSD4 and FBSD6\n> things changed, and you can now make g+s request (which I presume is still\n> a no-op other than setting the bit on)?\n\nYes, you can ask just fine now:\n\n $ uname -sr\n FreeBSD 6.1-RELEASE-p17-jc1\n $ mkdir foo\n $ ls -ld foo\n drwxr-xr-x 2 peff peff 512 Mar 31 09:04 foo/\n $ chmod g+s foo\n $ ls -ld foo\n drwxr-sr-x 2 peff peff 512 Mar 31 09:04 foo/\n\nBut it isn't necessary.\n\n> Ideally the test should be checking if the subdirectory is owned by the\n> same group as the toplevel, but that is rather hard to correctly arrange,\n> as it depends on the set of groups the user who runs the test belongs to,\n> how the git work tree is set up (if it is owned by his primary group or a\n> secondary), etc.\n\nShouldn't that just be:\n\n perl -e 'sub group { return (stat(shift))[5] }' \\\n -e 'exit group($ARGV[0]) == group($ARGV[1]) ? 0 : 1' \\\n a b\n\n?\n\n-Peff\n"},{"id":"110040","messageId":"7veiwd61k1.fsf@gitster.siamese.dyndns.org","threadId":"18658","inReplyTo":"20090331160842.GA9019@coredump.intra.peff.net","subject":"Re: bsd group semantics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-31T17:32:14Z","receivedAt":"2009-03-31T17:32:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Mar 31, 2009 at 08:55:00AM -0700, Junio C Hamano wrote:\n> ...\n>> Ideally the test should be checking if the subdirectory is owned by the\n>> same group as the toplevel, but that is rather hard to correctly arrange,\n>> as it depends on the set of groups the user who runs the test belongs to,\n>> how the git work tree is set up (if it is owned by his primary group or a\n>> secondary), etc.\n>\n> Shouldn't that just be:\n>\n> perl -e 'sub group { return (stat(shift))[5] }' \\\n> -e 'exit group($ARGV[0]) == group($ARGV[1]) ? 0 : 1' \\\n> a b\n>\n> ?\n\nMy point was if I belong only to my personal group and git.git work tree\nis owned by junio.junio to begin with, you would never be able to notice\nany breakage with such a test.\n"},{"id":"110047","messageId":"20090331184604.GA10701@coredump.intra.peff.net","threadId":"18658","inReplyTo":"7veiwd61k1.fsf@gitster.siamese.dyndns.org","subject":"Re: bsd group semantics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-31T18:46:04Z","receivedAt":"2009-03-31T18:46:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 31, 2009 at 10:32:14AM -0700, Junio C Hamano wrote:\n\n> My point was if I belong only to my personal group and git.git work tree\n> is owned by junio.junio to begin with, you would never be able to notice\n> any breakage with such a test.\n\nAh, I see. Yes, you would need to belong to at least two groups to make\nsuch a test feasible.\n\nSuch a requirement is generally outside the scope of running the\ntest suite; do you want to just ditch the g+s test, then, or do you want\nme to work up something conditional based on the Makefile flag?\n\n-Peff\n"},{"id":"110060","messageId":"7vy6ul4exy.fsf@gitster.siamese.dyndns.org","threadId":"18658","inReplyTo":"20090331184604.GA10701@coredump.intra.peff.net","subject":"Re: bsd group semantics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-31T20:26:01Z","receivedAt":"2009-03-31T20:26:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Mar 31, 2009 at 10:32:14AM -0700, Junio C Hamano wrote:\n>\n>> My point was if I belong only to my personal group and git.git work tree\n>> is owned by junio.junio to begin with, you would never be able to notice\n>> any breakage with such a test.\n>\n> Ah, I see. Yes, you would need to belong to at least two groups to make\n> such a test feasible.\n>\n> Such a requirement is generally outside the scope of running the\n> test suite; do you want to just ditch the g+s test, then, or do you want\n> me to work up something conditional based on the Makefile flag?\n\nFor now, I'd suggest teaching the test not to care about g+s bit.\nAfter all, they are about giving correct mode bits to files and\ndirectories. Correct mode bits for group do not mean anything if you make\nthem owned by a wrong group, but that is not something we have tested so\nfar, and can be a separate test anyway.\n"},{"id":"110063","messageId":"20090331203600.GA24340@coredump.intra.peff.net","threadId":"18658","inReplyTo":"7vy6ul4exy.fsf@gitster.siamese.dyndns.org","subject":"Re: bsd group semantics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-31T20:36:00Z","receivedAt":"2009-03-31T20:36:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 31, 2009 at 01:26:01PM -0700, Junio C Hamano wrote:\n\n> For now, I'd suggest teaching the test not to care about g+s bit.\n> After all, they are about giving correct mode bits to files and\n> directories. Correct mode bits for group do not mean anything if you make\n> them owned by a wrong group, but that is not something we have tested so\n> far, and can be a separate test anyway.\n\nMakes sense. How about this?\n\n-- >8 --\nSubject: [PATCH] t1301: loosen test for forced modes\n\nOne of the aspects of the test checked explicitly for the\ng+s bit to be set on created directories. However, this is\nonly the means to an end (the \"end\" being having the correct\ngroup set). And in fact, on systems where\nDIR_HAS_BSD_GROUP_SEMANTICS is set, we do not even need to\nuse this \"means\" at all, causing the test to fail.\n\nThis patch removes that part of the test. In an ideal world\nit would be replaced by a test to check that the group was\nproperly assigned, but that is difficult to automate because\nit requires the user running the test suite be a member of\nmultiple groups.\n\nSigned-off-by: Jeff King \n---\n t/t1301-shared-repo.sh | 3 ---\n 1 files changed, 0 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex 11ef302..9b18507 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -147,9 +147,6 @@ test_expect_success 'forced modes' '\n \t# Everything must be unaccessible to others\n \ttest -z \"$(sed -n -e \"/^.......---/d\" actual)\" &&\n \n-\t# All directories must have 2770\n-\ttest -z \"$(sed -n -e \"/^drwxrws---/d\" -e \"/^d/p\" actual)\" &&\n-\n \t# post-update hook must be 0770\n \ttest -z \"$(sed -n -e \"/post-update/{\n \t\t/^-rwxrwx---/d\n-- \n1.6.2.1.591.geb450\n"},{"id":"110069","messageId":"7vr60d4btl.fsf@gitster.siamese.dyndns.org","threadId":"18658","inReplyTo":"20090331203600.GA24340@coredump.intra.peff.net","subject":"Re: bsd group semantics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-31T21:33:26Z","receivedAt":"2009-03-31T21:33:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Mar 31, 2009 at 01:26:01PM -0700, Junio C Hamano wrote:\n>\n>> For now, I'd suggest teaching the test not to care about g+s bit.\n>> After all, they are about giving correct mode bits to files and\n>> directories. Correct mode bits for group do not mean anything if you make\n>> them owned by a wrong group, but that is not something we have tested so\n>> far, and can be a separate test anyway.\n>\n> Makes sense. How about this?\n>\n> -- >8 --\n> Subject: [PATCH] t1301: loosen test for forced modes\n>\n> One of the aspects of the test checked explicitly for the\n> g+s bit to be set on created directories. However, this is\n> only the means to an end (the \"end\" being having the correct\n> group set). And in fact, on systems where\n> DIR_HAS_BSD_GROUP_SEMANTICS is set, we do not even need to\n> use this \"means\" at all, causing the test to fail.\n>\n> This patch removes that part of the test. In an ideal world\n> it would be replaced by a test to check that the group was\n> properly assigned, but that is difficult to automate because\n> it requires the user running the test suite be a member of\n> multiple groups.\n>\n> Signed-off-by: Jeff King \n> ---\n> t/t1301-shared-repo.sh | 3 ---\n> 1 files changed, 0 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\n> index 11ef302..9b18507 100755\n> --- a/t/t1301-shared-repo.sh\n> +++ b/t/t1301-shared-repo.sh\n> @@ -147,9 +147,6 @@ test_expect_success 'forced modes' '\n> \t# Everything must be unaccessible to others\n> \ttest -z \"$(sed -n -e \"/^.......---/d\" actual)\" &&\n> \n> -\t# All directories must have 2770\n> -\ttest -z \"$(sed -n -e \"/^drwxrws---/d\" -e \"/^d/p\" actual)\" &&\n\nDid you mean \n\n-\t# All directories must have 2770\n-\ttest -z \"$(sed -n -e \"/^drwxrws---/d\" -e \"/^d/p\" actual)\" &&\n+\t# All directories must have either 2770 or 770\n+\ttest -z \"$(sed -n -e \"/^drwxrw[sx]---/d\" -e \"/^d/p\" actual)\" &&\n\n\n> -\n> \t# post-update hook must be 0770\n> \ttest -z \"$(sed -n -e \"/post-update/{\n> \t\t/^-rwxrwx---/d\n> -- \n> 1.6.2.1.591.geb450\n"},{"id":"110075","messageId":"20090331222540.GA25820@coredump.intra.peff.net","threadId":"18658","inReplyTo":"7vr60d4btl.fsf@gitster.siamese.dyndns.org","subject":"Re: bsd group semantics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-31T22:25:42Z","receivedAt":"2009-03-31T22:25:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 31, 2009 at 02:33:26PM -0700, Junio C Hamano wrote:\n\n> > -\t# All directories must have 2770\n> > -\ttest -z \"$(sed -n -e \"/^drwxrws---/d\" -e \"/^d/p\" actual)\" &&\n> \n> Did you mean \n> \n> -\t# All directories must have 2770\n> -\ttest -z \"$(sed -n -e \"/^drwxrws---/d\" -e \"/^d/p\" actual)\" &&\n> +\t# All directories must have either 2770 or 770\n> +\ttest -z \"$(sed -n -e \"/^drwxrw[sx]---/d\" -e \"/^d/p\" actual)\" &&\n\nNo, I meant what I said, but I was wrong. ;) I was thinking that bit\nonly tested for one thing, but it was actually testing for two. Sorry.\n\nCan you fix it up when you apply, then?\n\n-Peff\n"}]}