{"thread":{"id":"21715","subject":"'error: unable to set permission to './objects/...'","startedAt":"2009-11-22T20:02:12Z","lastAt":"2009-11-24T00:59:34Z","messageCount":7,"participants":["Rafal Rusin","Miklos Vajna","Junio C Hamano","Martin Langhoff"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"128124","messageId":"9bbf67fa0911221202r605de38coc2496b1f09123725@mail.gmail.com","threadId":"21715","inReplyTo":null,"subject":"'error: unable to set permission to './objects/...'","fromName":"Rafal Rusin","fromEmail":"rafal.rusin@gmail.com","sentAt":"2009-11-22T20:02:12Z","receivedAt":"2009-11-22T20:02:12Z","isPatch":false,"sender":{"key":"rafal.rusin@gmail.com","avatar":"https://gravatar.com/avatar/d7c51b52e4d1cdb83aa33d7afa0732c4ef83c0b5076b20945b9123f70e8bf6e9?d=mp&s=160"},"body":"Hello,\n\nI'm hosting git repository on filesystem with 'chmod '\ncausing permission denied error (it's smbfs mounted directory),\nWhen I was doing push to such repo using file:// protocol, I got\nfollowing error:\nerror: unable to set permission to './objects/...'\n\nI did a small fix to sha1_file.c (patch in attachment) and git now\nwarns when unable to chmod, and continues push. This resolved problem.\nWhat do you think about applying it?\n\nRegards,\n-- \nRafał Rusin\nhttp://rrusin.blogspot.com\nhttp://www.touk.pl\nhttp://top.touk.pl\n\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 4ea0b18..305e751 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2272,8 +2272,9 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n \t}\n \n out:\n-\tif (set_shared_perm(filename, (S_IFREG|0444)))\n-\t\treturn error(\"unable to set permission to '%s'\", filename);\n+\tif (set_shared_perm(filename, (S_IFREG|0444))) {\n+\t\twarning(\"unable to set permission to '%s'\", filename);\n+ }\n \treturn 0;\n }\n \n"},{"id":"128126","messageId":"20091122220900.GO23718@genesis.frugalware.org","threadId":"21715","inReplyTo":"9bbf67fa0911221202r605de38coc2496b1f09123725@mail.gmail.com","subject":"Re: 'error: unable to set permission to './objects/...'","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2009-11-22T22:09:00Z","receivedAt":"2009-11-22T22:09:00Z","isPatch":false,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sun, Nov 22, 2009 at 09:02:12PM +0100, Rafal Rusin wrote:\n> I did a small fix to sha1_file.c (patch in attachment) and git now\n> warns when unable to chmod, and continues push. This resolved problem.\n> What do you think about applying it?\n\nPlease read Documentation/SubmittingPatches, patches should be sent\ninline and with a Signed-off-by line.\n"},{"id":"128132","messageId":"7vd43acf7y.fsf@alter.siamese.dyndns.org","threadId":"21715","inReplyTo":"9bbf67fa0911221202r605de38coc2496b1f09123725@mail.gmail.com","subject":"Re: 'error: unable to set permission to './objects/...'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-22T23:27:45Z","receivedAt":"2009-11-22T23:27:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rafal Rusin writes:\n\n> I'm hosting git repository on filesystem with 'chmod '\n> causing permission denied error (it's smbfs mounted directory),\n> When I was doing push to such repo using file:// protocol, I got\n> following error:\n> error: unable to set permission to './objects/...'\n>\n> I did a small fix to sha1_file.c (patch in attachment) and git now\n> warns when unable to chmod, and continues push. This resolved problem.\n> What do you think about applying it?\n\nSuppose the user wanted to use this as a shared public repository and\nconfigured core.sharedrepository. If we try to set shared-perm and notice\na failure but keep going, what happens to the resulting repository? \n\nFor example, the umask of the user who is pushing objects, causing this\ncodepath to run, might be too tight to be usable for the purpose of making\nthe file readable for other members of the group. And the chmod() fails\nin this codepath. Then what? Wouldn't it make the resulting repository\nunusable?\n\nI think a _fix_ needs to first know why chmod is failing for you and\neither\n\n (1) make it not to fail; or\n\n (2) Perhaps your filesystem is lying and the result of chmod happens to\n be Ok (iow, the resulting file may be readable/writable by people who\n are supposed to be able to, accoring to the core.sharedrepository\n settings), in which case make the code notice the situation and keep\n going _only when_ it is safe to do so.\n\nI do not think your change to _unconditionally_ keep going is a fix.\n"},{"id":"128160","messageId":"9bbf67fa0911230135j7cfe5bcem991e750b6754f344@mail.gmail.com","threadId":"21715","inReplyTo":"7vd43acf7y.fsf@alter.siamese.dyndns.org","subject":"Re: 'error: unable to set permission to './objects/...'","fromName":"Rafal Rusin","fromEmail":"rafal.rusin@gmail.com","sentAt":"2009-11-23T09:35:39Z","receivedAt":"2009-11-23T09:35:39Z","isPatch":false,"sender":{"key":"rafal.rusin@gmail.com","avatar":"https://gravatar.com/avatar/d7c51b52e4d1cdb83aa33d7afa0732c4ef83c0b5076b20945b9123f70e8bf6e9?d=mp&s=160"},"body":"2009/11/23 Junio C Hamano :\n> Rafal Rusin writes:\n>\n>> I'm hosting git repository on filesystem with 'chmod '\n>> causing permission denied error (it's smbfs mounted directory),\n>> When I was doing push to such repo using file:// protocol, I got\n>> following error:\n>> error: unable to set permission to './objects/...'\n>>\n>> I did a small fix to sha1_file.c (patch in attachment) and git now\n>> warns when unable to chmod, and continues push. This resolved problem.\n>> What do you think about applying it?\n>\n> Suppose the user wanted to use this as a shared public repository and\n> configured core.sharedrepository.  If we try to set shared-perm and notice\n> a failure but keep going, what happens to the resulting repository?\n>\n> For example, the umask of the user who is pushing objects, causing this\n> codepath to run, might be too tight to be usable for the purpose of making\n> the file readable for other members of the group.  And the chmod() fails\n> in this codepath.  Then what?  Wouldn't it make the resulting repository\n> unusable?\n>\n> I think a _fix_ needs to first know why chmod is failing for you and\n> either\n>\n>  (1) make it not to fail; or\n>\n>  (2) Perhaps your filesystem is lying and the result of chmod happens to\n>     be Ok (iow, the resulting file may be readable/writable by people who\n>     are supposed to be able to, accoring to the core.sharedrepository\n>     settings), in which case make the code notice the situation and keep\n>     going _only when_ it is safe to do so.\n>\n> I do not think your change to _unconditionally_ keep going is a fix.\n\nThanks for reply. You are right about sharedrepository argument.\nAs for detecting this particular case, I think it's not possible.\nI think the best solution is to add repository config switch like\n'usefilepermissions' true by default. If set to false, git would skip\nchmod during push.\nDoes that make sense to you?\n\nRegards,\n-- \nRafał Rusin\nhttp://rrusin.blogspot.com\nhttp://www.touk.pl\nhttp://top.touk.pl\n"},{"id":"128164","messageId":"46a038f90911230152j34f42e85ibf4ed57660bd2cf2@mail.gmail.com","threadId":"21715","inReplyTo":"7vd43acf7y.fsf@alter.siamese.dyndns.org","subject":"Re: 'error: unable to set permission to './objects/...'","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2009-11-23T09:52:49Z","receivedAt":"2009-11-23T09:52:49Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Mon, Nov 23, 2009 at 12:27 AM, Junio C Hamano wrote:\n> And the chmod() fails\n> in this codepath.  Then what?  Wouldn't it make the resulting repository\n> unusable?\n\nSlightly off-topic: I keep encountering hosts where\ncore.sharedrepository doesn't always work with git+ssh. It may be\nrelated to old git versions (I see 1.5.8 in a host where I've seen it\nhappen recently), but I never fails to baffle me.\n\nBeen sysadmin'ing and developing on linux since '98 and there is\nnothing to prevent git from chmod'ing things right. And I know a few\nthings about git's plumbing :-) -- don't think it should be\nmysteriously failing.\n\n(And in several git repo servers I've spotted a cronjob doing chmod -R\ng+rwx, applied by sysadmins to make the devs STFU about it ;-) ).\n\nIs there a shortlist somewhere of reasons for it to be ignored?\nPerhaps it is a known bug in old versions? -- the repo servers are\noften behind...\n\nPuzzled minds want to know.\n\n\n\nm\n-- \n martin.langhoff@gmail.com\n martin@laptop.org -- School Server Architect\n - ask interesting questions\n - don't get distracted with shiny stuff - working code first\n - http://wiki.laptop.org/go/User:Martinlanghoff\n"},{"id":"128166","messageId":"7vhbsl5x5e.fsf@alter.siamese.dyndns.org","threadId":"21715","inReplyTo":"9bbf67fa0911230135j7cfe5bcem991e750b6754f344@mail.gmail.com","subject":"Re: 'error: unable to set permission to './objects/...'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-23T10:54:37Z","receivedAt":"2009-11-23T10:54:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rafal Rusin writes:\n\n> As for detecting this particular case, I think it's not possible.\n\nWhy not?\n\n> I think the best solution is to add repository config switch like\n> 'usefilepermissions' true by default. If set to false, git would skip\n> chmod during push.\n> Does that make sense to you?\n\nNot at all. That is like creating an \"allow broken behaviour\" option and\nhoping for the best.\n\nYou shouldn't have to invent such a configuration variable to begin with,\nas \"let umask set whatever permission and leave it be\" should already be\nthe case for !core.sharedrepository. There is something wrong in your\nset up and you need to get to the bottom of it, instead of coming up with\nan even worse breakage as a unworkable workaround.\n\nThere are some things I noticed while looking at the codepaths that are\ninvolved.\n\nmove_temp_to_file() is designed to move a temporary file that was created\nby odb_mkstemp(). As the latter eventually uses mkstemp(), some\nimplementations of which ignore umask and create a file that is readable\nonly by the user, move_temp_to_file() must loosen the permission, even in\na private repository that honors umask (i.e. not in a shared repository)..\n\n Side note. The creation of the temporary files in http.c that are\n given to move_temp_to_file() is not quite correct; they are created by\n fopen() without proper locking with mkstemp(). But that is a separate\n issue.\n\nBut the codepath tries to loosen it a bit too much. Even if user's umask\nis 077, files created in this codepath end up with world-readable because\nwe pretend that lstat() on the file returned 0444 (that is what a non-zero\nvalue given as the second parameter to set_shared_perm() means). We\nshould tighten it perhaps like the attached patch does.\n\nIn case it is not obvious, this patch is _not_ meant to help you work\naround the chmod() failure you are seeing on your filesystem. You need to\nfirst see _why_ it fails for you, in order to come closer to the real\nsolution.\n\n-- >8 --\nSubject: [PATCH] move_temp_to_file(): don't loosen permission too much\n\nWe always feed 0444 as the second parameter to set_shared_perm() when\nfinalizing a temporary file we created using mkstemp(), as some versions\nof glibc creates a temporary file with too tight a permission, ignoring\nthe user's umask. As the second parameter tells set_shared_perm() to\npretend that it is the permission bits the file currently has (i.e. what\nshould have been set by the user's umask), we should make it just as\nrestrictive as user's umask suggests.\n\n---\n sha1_file.c | 19 ++++++++++++++++---\n 1 files changed, 16 insertions(+), 3 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 63981fb..f0b8969 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2233,6 +2233,21 @@ static void write_sha1_file_prepare(const void *buf, unsigned long len,\n \tgit_SHA1_Final(sha1, &c);\n }\n \n+static int fix_tempfile_mode(const char *filename)\n+{\n+\tstatic mode_t user_mode;\n+\n+\tif (!user_mode) {\n+\t\tuser_mode = umask(0);\n+\t\tumask(user_mode);\n+\t\tuser_mode = S_IFREG | ((user_mode ^ 0777) & 0666);\n+\t}\n+\n+\tif (!set_shared_perm(filename, user_mode))\n+\t\treturn 0;\n+\treturn error(\"unable to set permission to '%s'\", filename);\n+}\n+\n /*\n * Move the just written object into its final resting place.\n * NEEDSWORK: this should be renamed to finalize_temp_file() as\n@@ -2274,9 +2289,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n \t}\n \n out:\n-\tif (set_shared_perm(filename, (S_IFREG|0444)))\n-\t\treturn error(\"unable to set permission to '%s'\", filename);\n-\treturn 0;\n+\treturn fix_tempfile_mode(filename);\n }\n \n static int write_buffer(int fd, const void *buf, size_t len)\n"},{"id":"128219","messageId":"9bbf67fa0911231659n761bafdcm91f16d3259a9721b@mail.gmail.com","threadId":"21715","inReplyTo":"7vhbsl5x5e.fsf@alter.siamese.dyndns.org","subject":"Re: 'error: unable to set permission to './objects/...'","fromName":"Rafal Rusin","fromEmail":"rafal.rusin@gmail.com","sentAt":"2009-11-24T00:59:34Z","receivedAt":"2009-11-24T00:59:34Z","isPatch":false,"sender":{"key":"rafal.rusin@gmail.com","avatar":"https://gravatar.com/avatar/d7c51b52e4d1cdb83aa33d7afa0732c4ef83c0b5076b20945b9123f70e8bf6e9?d=mp&s=160"},"body":"OK, I solved problem by upgrading samba on server from 3.0.23c to\n3.2.8 (it's old, because it's arm server). This newer version doesn't\nhave problem with chmod.\nJunio, your idea to investigate why it fails was good :-)\nThanks for your time.\n\n2009/11/23 Junio C Hamano :\n> Rafal Rusin writes:\n>\n>> As for detecting this particular case, I think it's not possible.\n>\n> Why not?\n>\n>> I think the best solution is to add repository config switch like\n>> 'usefilepermissions' true by default. If set to false, git would skip\n>> chmod during push.\n>> Does that make sense to you?\n>\n> Not at all.  That is like creating an \"allow broken behaviour\" option and\n> hoping for the best.\n>\n> You shouldn't have to invent such a configuration variable to begin with,\n> as \"let umask set whatever permission and leave it be\" should already be\n> the case for !core.sharedrepository.  There is something wrong in your\n> set up and you need to get to the bottom of it, instead of coming up with\n> an even worse breakage as a unworkable workaround.\n>\n> There are some things I noticed while looking at the codepaths that are\n> involved.\n>\n> move_temp_to_file() is designed to move a temporary file that was created\n> by odb_mkstemp().  As the latter eventually uses mkstemp(), some\n> implementations of which ignore umask and create a file that is readable\n> only by the user, move_temp_to_file() must loosen the permission, even in\n> a private repository that honors umask (i.e. not in a shared repository)..\n>\n>    Side note.  The creation of the temporary files in http.c that are\n>    given to move_temp_to_file() is not quite correct; they are created by\n>    fopen() without proper locking with mkstemp().  But that is a separate\n>    issue.\n>\n> But the codepath tries to loosen it a bit too much.  Even if user's umask\n> is 077, files created in this codepath end up with world-readable because\n> we pretend that lstat() on the file returned 0444 (that is what a non-zero\n> value given as the second parameter to set_shared_perm() means).  We\n> should tighten it perhaps like the attached patch does.\n>\n> In case it is not obvious, this patch is _not_ meant to help you work\n> around the chmod() failure you are seeing on your filesystem.  You need to\n> first see _why_ it fails for you, in order to come closer to the real\n> solution.\n>\n> -- >8 --\n> Subject: [PATCH] move_temp_to_file(): don't loosen permission too much\n>\n> We always feed 0444 as the second parameter to set_shared_perm() when\n> finalizing a temporary file we created using mkstemp(), as some versions\n> of glibc creates a temporary file with too tight a permission, ignoring\n> the user's umask.  As the second parameter tells set_shared_perm() to\n> pretend that it is the permission bits the file currently has (i.e. what\n> should have been set by the user's umask), we should make it just as\n> restrictive as user's umask suggests.\n>\n> ---\n>  sha1_file.c |   19 ++++++++++++++++---\n>  1 files changed, 16 insertions(+), 3 deletions(-)\n>\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 63981fb..f0b8969 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -2233,6 +2233,21 @@ static void write_sha1_file_prepare(const void *buf, unsigned long len,\n>        git_SHA1_Final(sha1, &c);\n>  }\n>\n> +static int fix_tempfile_mode(const char *filename)\n> +{\n> +       static mode_t user_mode;\n> +\n> +       if (!user_mode) {\n> +               user_mode = umask(0);\n> +               umask(user_mode);\n> +               user_mode = S_IFREG | ((user_mode ^ 0777) & 0666);\n> +       }\n> +\n> +       if (!set_shared_perm(filename, user_mode))\n> +               return 0;\n> +       return error(\"unable to set permission to '%s'\", filename);\n> +}\n> +\n>  /*\n>  * Move the just written object into its final resting place.\n>  * NEEDSWORK: this should be renamed to finalize_temp_file() as\n> @@ -2274,9 +2289,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)\n>        }\n>\n>  out:\n> -       if (set_shared_perm(filename, (S_IFREG|0444)))\n> -               return error(\"unable to set permission to '%s'\", filename);\n> -       return 0;\n> +       return fix_tempfile_mode(filename);\n>  }\n>\n>  static int write_buffer(int fd, const void *buf, size_t len)\n>\n>\n\n\nRegards,\n-- \nRafał Rusin\nhttp://rrusin.blogspot.com\nhttp://www.touk.pl\nhttp://top.touk.pl\n"}]}