{"thread":{"id":"28556","subject":"[PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD","startedAt":"2011-10-03T06:41:20Z","lastAt":"2011-10-24T23:07:52Z","messageCount":7,"participants":["Jonathan Nieder","Junio C Hamano","Sverre Rabbelier","Greg Troxel"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"176708","messageId":"20111003064120.GA24396@elie","threadId":"28556","inReplyTo":null,"subject":"[PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-03T06:41:20Z","receivedAt":"2011-10-03T06:41:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The g+s bit on directories to make group ownership inherited is a\nSysVism --- BSD and most of its descendants do not need it since they\ndo the sane thing by default without g+s. In fact, on some\nfilesystems (but not all --- tmpfs works this way but UFS does not),\nthe kernel of FreeBSD does not even allow non-root users to set setgid\nbit on directories and produces errors when one tries:\n\n\t$ git init --shared dir\n\tfatal: Could not make /tmp/dir/.git/refs writable by group\n\nSince the setgid bit would only mean \"do what you were going to do\nalready\", it's better to avoid setting it. Accordingly, ever since\nv1.5.5-rc0~59^2 (Do not use GUID on dir in git init --share=all on\nFreeBSD, 2008-03-05), git on true FreeBSD has done exactly that. Set\nDIR_HAS_BSD_GROUP_SEMANTICS in the makefile for GNU/kFreeBSD, too, so\nmachines that use glibc with the kernel of FreeBSD get the same fix.\n\nThis fixes t0001-init.sh and t1301-shared-repo.sh on GNU/kFreeBSD\nwhen running tests with --root pointing to a directory that uses\ntmpfs.\n\nSigned-off-by: Jonathan Nieder \n---\nSorry to have taken so long to send this one out. Anyway, it seems\nto me like the right thing to do. Petr, what do you think?\n\n Makefile | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 8d6d4515..924749ed 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -820,6 +820,7 @@ ifeq ($(uname_S),GNU/kFreeBSD)\n \tNO_STRLCPY = YesPlease\n \tNO_MKSTEMPS = YesPlease\n \tHAVE_PATHS_H = YesPlease\n+\tDIR_HAS_BSD_GROUP_SEMANTICS = YesPlease\n endif\n ifeq ($(uname_S),UnixWare)\n \tCC = cc\n-- \n1.7.7.rc1\n"},{"id":"176711","messageId":"20111003071949.GC17289@elie","threadId":"28556","inReplyTo":"20111003064120.GA24396@elie","subject":"Re: [PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-03T07:19:49Z","receivedAt":"2011-10-03T07:19:49Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> Since the setgid bit would only mean \"do what you were going to do\n> already\", it's better to avoid setting it. Accordingly, ever since\n> v1.5.5-rc0~59^2 (Do not use GUID on dir in git init --share=all on\n> FreeBSD, 2008-03-05), git on true FreeBSD has done exactly that. Set\n> DIR_HAS_BSD_GROUP_SEMANTICS in the makefile for GNU/kFreeBSD, too, so\n> machines that use glibc with the kernel of FreeBSD get the same fix.\n[...]\n> Sorry to have taken so long to send this one out. Anyway, it seems\n> to me like the right thing to do. Petr, what do you think?\n\nfwiw:\n\nAcked-by: Petr Salinger \n\nThanks for looking it over.\n"},{"id":"176777","messageId":"7vlit1ga2l.fsf@alter.siamese.dyndns.org","threadId":"28556","inReplyTo":"20111003071949.GC17289@elie","subject":"Re: [PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-03T19:16:18Z","receivedAt":"2011-10-03T19:16:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Jonathan Nieder wrote:\n>\n>> Since the setgid bit would only mean \"do what you were going to do\n>> already\", it's better to avoid setting it. Accordingly, ever since\n>> v1.5.5-rc0~59^2 (Do not use GUID on dir in git init --share=all on\n>> FreeBSD, 2008-03-05), git on true FreeBSD has done exactly that. Set\n>> DIR_HAS_BSD_GROUP_SEMANTICS in the makefile for GNU/kFreeBSD, too, so\n>> machines that use glibc with the kernel of FreeBSD get the same fix.\n> [...]\n>> Sorry to have taken so long to send this one out. Anyway, it seems\n>> to me like the right thing to do. Petr, what do you think?\n>\n> fwiw:\n>\n> Acked-by: Petr Salinger \n>\n> Thanks for looking it over.\n\nSorry, this is very confusing. Are JN and PS one and the same person?\n"},{"id":"176778","messageId":"CAGdFq_hQ-qoktgHB6ACF8H2AmeMyfo-bW4VBxfL-CPw5kDMFQw@mail.gmail.com","threadId":"28556","inReplyTo":"7vlit1ga2l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-10-03T19:19:11Z","receivedAt":"2011-10-03T19:19:11Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Oct 3, 2011 at 21:16, Junio C Hamano wrote:\n> Sorry, this is very confusing. Are JN and PS one and the same person?\n\nI would assume PS mailed JN off list?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"176784","messageId":"20111003194214.GA18153@elie","threadId":"28556","inReplyTo":"CAGdFq_hQ-qoktgHB6ACF8H2AmeMyfo-bW4VBxfL-CPw5kDMFQw@mail.gmail.com","subject":"Re: [PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-03T19:42:40Z","receivedAt":"2011-10-03T19:42:40Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> I would assume PS mailed JN off list?\n\nYes, that's right. Sorry for the cryptic message.\n"},{"id":"178160","messageId":"20111022111107.GA12130@elie.domain.sunraytvi.com","threadId":"28556","inReplyTo":"20111003064120.GA24396@elie","subject":"Re: [PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-22T11:11:07Z","receivedAt":"2011-10-22T11:11:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(people cc-ed: your input would be welcome on [*] below. See commit\n81a24b52, \"Do not use GUID on dir in git init --shared=all on FreeBSD\"\nfor context)\n\nHi Junio,\n\n>From Documentation/RelNotes/1.7.7.1.txt:\n\n * On some BSD systems, adding +s bit on directories is detrimental\n (it is not necessary on BSD to begin with). The installation\n procedure has been updated to take this into account.\n\nI assume this is referring to 0b20dd8f (Makefile: do not set setgid\nbit on directories on GNU/kFreeBSD, 2011-10-03), which admittedly\ndoes have a subject line that suggests it would be about that (sorry\nabout that). The change was actually about \"git init -s\" which sets\nthe setgid bit on SysV-style systems to allow shared access to a\nrepository (and can provoke errors on BSD-style systems, depending on\nhow permissive the filesystem in use wants to be).\n\nMore to the point, the patch was just taking a fix that arrived for\nFreeBSD in v1.5.5 days and making it also apply to machines using an\n(obscure) GNU userland/FreeBSD kernel mixture.\n\nBy the way, maybe other BSD-style ports (NetBSD, OpenBSD) should be\nsetting DIR_HAS_BSD_GROUP_SEMANTICS to get this fix, too[*]? Then the\nrelease notes could look something like this:\n\nSigned-off-by: Jonathan Nieder \n---\n Documentation/RelNotes/1.7.7.1.txt | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git i/Documentation/RelNotes/1.7.7.1.txt w/Documentation/RelNotes/1.7.7.1.txt\nindex fecfac8a..e3c29ff0 100644\n--- i/Documentation/RelNotes/1.7.7.1.txt\n+++ w/Documentation/RelNotes/1.7.7.1.txt\n@@ -5,8 +5,9 @@ Fixes since v1.7.7\n ------------------\n \n * On some BSD systems, adding +s bit on directories is detrimental\n- (it is not necessary on BSD to begin with). The installation\n- procedure has been updated to take this into account.\n+ (it is not necessary on BSD to begin with). \"git init --shared\"\n+ has been updated to take this into account without extra makefile\n+ settings on platforms the Makefile knows about.\n \n * After incorrectly written third-party tools store a tag object in\n HEAD, git diagnosed it as a repository corruption and refused to\n-- \n"},{"id":"178252","messageId":"rmibot6aszb.fsf@fnord.ir.bbn.com","threadId":"28556","inReplyTo":"20111022111107.GA12130@elie.domain.sunraytvi.com","subject":"Re: [PATCH] Makefile: do not set setgid bit on directories on GNU/kFreeBSD","fromName":"Greg Troxel","fromEmail":"gdt@ir.bbn.com","sentAt":"2011-10-24T23:07:52Z","receivedAt":"2011-10-24T23:07:52Z","isPatch":true,"sender":{"key":"gdt@ir.bbn.com","avatar":null},"body":"\n * On some BSD systems, adding +s bit on directories is detrimental\n (it is not necessary on BSD to begin with). The installation\n procedure has been updated to take this into account.\n\nI looked at the NetBSD 5 sources, and as expected files are created\n(unconditionally) with the gid of the parent directory.\n\nSetting the setgid flag is only allowed if the inode's gid is in the\nprocess gid set. This is really about files that might be executed,\nbut the check is independent of regular file/directory.\n\n\"git init --shared\" creates a repository, mode 2775, and that normally\nseems fine. It seems good to have the sgid bit on, in case the\nrepository is transferred to another machine with different semantics,\nand it's a clue to humans about the intended behavior, even if it's\nnon-optional on BSD.\n\nI created a directory, mode 755, owned by me, and with group that I *do\nnot* belong to. Then, \"git init --shared\" produced:\n\n fatal: Could not make /home/gdt/FOO/.git/refs writable by group\n\nbut really the issue was setting the sgid bit:\n\n# all with git version 1.7.6.3\n13 $ l -d .git/refs/\ndrwxr-xr-x 2 gdt kmem 512 Oct 24 18:53 .git/refs/\n14 $ chmod g+w .git/refs/\n15 $ l -d .git/refs/\ndrwxrwxr-x 2 gdt kmem 512 Oct 24 18:53 .git/refs/\n16 $ chmod g+s .git/refs/\nchmod: .git/refs/: Operation not permitted\n\nHowever, this is a pathological situation, because I've created a shared\nrepository that I can write to because I own it, and group kmem people\ncan write to because they're in the group, but I couldn't write to other\ngroup kmem resources.\n\nIs this not-allowed-to-set-setgid issue the problem the patch is trying\nto avoid? Or something else?\n\nI did run the regression tests at one point and don't remember this\nfailing.\n\nSo all in all I am agnostic as to whether DIR_HAS_BSD_GROUP_SEMANTICS\nshould be defined on NetBSD; personally I prefer to see the setgid\nbits.\n\n"}]}