{"thread":{"id":"13081","subject":"[PATCH] Make core.sharedRepository more generic","startedAt":"2008-04-12T18:51:05Z","lastAt":"2008-04-12T22:20:34Z","messageCount":6,"participants":["Heikki Orsila","Samuel Tardieu"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"74221","messageId":"20080412185105.GA14331@zakalwe.fi","threadId":"13081","inReplyTo":null,"subject":"[PATCH] Make core.sharedRepository more generic","fromName":"Heikki Orsila","fromEmail":"heikki.orsila@iki.fi","sentAt":"2008-04-12T18:51:05Z","receivedAt":"2008-04-12T18:51:05Z","isPatch":true,"sender":{"key":"heikki.orsila@iki.fi","avatar":null},"body":"git init --shared=0xxx, where '0xxx' is an octal number, will create\na repository with file modes set to '0xxx'. User's with a safe umask\nvalue (0077) can use this option to force file modes. For example,\n'0640' is a group-readable but not group-writable regardless of\nuser's umask value.\n\n\"git config core.sharedRepository 0xxx\" is also handled.\n---\n Documentation/config.txt | 7 +++++-\n Documentation/git-init.txt | 8 ++++++-\n builtin-init-db.c | 4 +-\n cache.h | 16 ++++++++++++--\n path.c | 38 ++++++++++++++++++++----------------\n setup.c | 45 ++++++++++++++++++++++++++++++++++++++++---\n 6 files changed, 90 insertions(+), 28 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex fe43b12..7a24f6e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -261,7 +261,12 @@ core.sharedRepository::\n \tgroup-writable). When 'all' (or 'world' or 'everybody'), the\n \trepository will be readable by all users, additionally to being\n \tgroup-shareable. When 'umask' (or 'false'), git will use permissions\n-\treported by umask(2). See linkgit:git-init[1]. False by default.\n+\treported by umask(2). When '0xxx', where '0xxx' is an octal number,\n+\tfiles in the repository will have this mode value. '0xxx' will override\n+\tuser's umask value, and thus, users with a safe umask (0077) can use\n+\tthis option. Examples: '0660' is equivalent to 'group'. '0640' is a\n+\trepository that is group-readable but not group-writable.\n+\tSee linkgit:git-init[1]. False by default.\n \n core.warnAmbiguousRefs::\n \tIf true, git will warn you if the ref name you passed it is ambiguous\ndiff --git a/Documentation/git-init.txt b/Documentation/git-init.txt\nindex 62914da..b17ae84 100644\n--- a/Documentation/git-init.txt\n+++ b/Documentation/git-init.txt\n@@ -31,7 +31,7 @@ structure, some suggested \"exclude patterns\", and copies of non-executing\n \"hook\" files. The suggested patterns and hook files are all modifiable and\n extensible.\n \n---shared[={false|true|umask|group|all|world|everybody}]::\n+--shared[={false|true|umask|group|all|world|everybody|0xxx}]::\n \n Specify that the git repository is to be shared amongst several users. This\n allows users belonging to the same group to push into that\n@@ -52,6 +52,12 @@ is given:\n - 'all' (or 'world' or 'everybody'): Same as 'group', but make the repository\n readable by all users.\n \n+ - '0xxx': '0xxx' is an octal number and each file will have mode '0xxx'\n+ Any option except 'umask' can be set using this option. '0xxx' will\n+ override users umask(2) value, and thus, users with a safe umask (0077)\n+ can use this option. '0640' will create a repository which is group-readable\n+ but not writable. '0660' is equivalent to 'group'.\n+\n By default, the configuration flag receive.denyNonFastForwards is enabled\n in shared repositories, so that you cannot force a non fast-forwarding push\n into it.\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex 2854868..8c63295 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -400,9 +400,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\tchar buf[10];\n \t\t/* We do not spell \"group\" and such, so that\n \t\t * the configuration can be read by older version\n-\t\t * of git.\n+\t\t * of git. Note, we use octal numbers.\n \t\t */\n-\t\tsprintf(buf, \"%d\", shared_repository);\n+\t\tsprintf(buf, \"0%o\", shared_repository);\n \t\tgit_config_set(\"core.sharedrepository\", buf);\n \t\tgit_config_set(\"receive.denyNonFastforwards\", \"true\");\n \t}\ndiff --git a/cache.h b/cache.h\nindex 2a1e7ec..f9c8d2b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -474,10 +474,20 @@ static inline void hashclr(unsigned char *hash)\n \n int git_mkstemp(char *path, size_t n, const char *template);\n \n+/*\n+ * NOTE NOTE NOTE!!\n+ *\n+ * PERM_UMASK, OLD_PERM_GROUP and OLD_PERM_EVERYBODY enumerations must\n+ * not be changed. Old repositories have core.sharedrepository written in\n+ * numeric format, and therefore these values are preserved for compatibility\n+ * reasons.\n+ */\n enum sharedrepo {\n-\tPERM_UMASK = 0,\n-\tPERM_GROUP,\n-\tPERM_EVERYBODY\n+\tPERM_UMASK = 0,\n+\tOLD_PERM_GROUP = 1,\n+\tOLD_PERM_EVERYBODY = 2,\n+\tPERM_GROUP = 0660,\n+\tPERM_EVERYBODY = 0664,\n };\n int git_config_perm(const char *var, const char *value);\n int adjust_shared_perm(const char *path);\ndiff --git a/path.c b/path.c\nindex f4ed979..79bfe82 100644\n--- a/path.c\n+++ b/path.c\n@@ -266,24 +266,28 @@ int adjust_shared_perm(const char *path)\n \tif (lstat(path, &st) < 0)\n \t\treturn -1;\n \tmode = st.st_mode;\n-\tif (mode & S_IRUSR)\n-\t\tmode |= (shared_repository == PERM_GROUP\n-\t\t\t ? S_IRGRP\n-\t\t\t : (shared_repository == PERM_EVERYBODY\n-\t\t\t ? (S_IRGRP|S_IROTH)\n-\t\t\t : 0));\n-\n-\tif (mode & S_IWUSR)\n-\t\tmode |= S_IWGRP;\n-\n-\tif (mode & S_IXUSR)\n-\t\tmode |= (shared_repository == PERM_GROUP\n-\t\t\t ? S_IXGRP\n-\t\t\t : (shared_repository == PERM_EVERYBODY\n-\t\t\t ? (S_IXGRP|S_IXOTH)\n-\t\t\t : 0));\n-\tif (S_ISDIR(mode))\n+\n+\tif (shared_repository) {\n+\t\tmode = (mode & ~0777) | shared_repository;\n+\t} else {\n+\t\t/* Preserve old PERM_UMASK behaviour */\n+\t\tif (mode & S_IWUSR)\n+\t\t\tmode |= S_IWGRP;\n+\t}\n+\n+\tif (S_ISDIR(mode)) {\n \t\tmode |= FORCE_DIR_SET_GID;\n+\n+\t\t/*\n+\t\t * The x flag for directories is determined from rw flags of\n+\t\t * user, group and others. Having a directory with +rw but\n+\t\t * -x does not make sense for Git repositories.\n+\t\t */\n+\t\tmode |= (shared_repository & 0600) ? S_IXUSR : 0;\n+\t\tmode |= (shared_repository & 0060) ? S_IXGRP : 0;\n+\t\tmode |= (shared_repository & 0006) ? S_IXOTH : 0;\n+\t}\n+\n \tif ((mode & st.st_mode) != mode && chmod(path, mode) < 0)\n \t\treturn -2;\n \treturn 0;\ndiff --git a/setup.c b/setup.c\nindex 3d2d958..0905509 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -430,6 +430,8 @@ int git_config_perm(const char *var, const char *value)\n {\n \tif (value) {\n \t\tint i;\n+\t\tchar *endptr;\n+\n \t\tif (!strcmp(value, \"umask\"))\n \t\t\treturn PERM_UMASK;\n \t\tif (!strcmp(value, \"group\"))\n@@ -438,11 +440,46 @@ int git_config_perm(const char *var, const char *value)\n \t\t !strcmp(value, \"world\") ||\n \t\t !strcmp(value, \"everybody\"))\n \t\t\treturn PERM_EVERYBODY;\n-\t\ti = atoi(value);\n-\t\tif (i > 1)\n-\t\t\treturn i;\n+\n+\t\t/* Parse octal numbers */\n+\t\ti = strtol(value, &endptr, 8);\n+\t\tif (*endptr != 0) {\n+\t\t\t/* Not an octal number. Maybe true/false? */\n+\t\t\tif (git_config_bool(var, value))\n+\t\t\t\treturn PERM_GROUP;\n+\t\t\telse\n+\t\t\t\treturn PERM_UMASK;\n+\t\t}\n+\n+\t\t/* Handle compatibility cases */\n+\t\tswitch (i) {\n+\t\tcase PERM_UMASK: /* 0 */\n+\t\t\treturn PERM_UMASK;\n+\t\tcase OLD_PERM_GROUP: /* 1 */\n+\t\t\treturn PERM_GROUP;\n+\t\tcase OLD_PERM_EVERYBODY: /* 2 */\n+\t\t\treturn PERM_EVERYBODY;\n+\t\t}\n+\n+\t\t/* A filemode value was given: 0xxx */\n+\n+\t\tif ((i & 0600) != 0600)\n+\t\t\tdie(\"Problem with core.sharedRepository filemode value\"\n+\t\t\t \" (0%.3o).\\nThe owner of files must always have \"\n+\t\t\t \"read and write permissions.\", i);\n+\n+\t\tif (i & 0002)\n+\t\t\twarning(\"core.sharedRepository filemode (0%.3o) is a \"\n+\t\t\t\t\"security threat.\\nMasking off write \"\n+\t\t\t\t\"permission for others\\n\", i);\n+\n+\t\t/*\n+ * Mask filemode value. Others can not get write permission.\n+\t\t * x flags for directories are handled separately.\n+ */\n+\t\treturn i & 0664;\n \t}\n-\treturn git_config_bool(var, value);\n+\treturn PERM_GROUP;\n }\n \n int check_repository_format_version(const char *var, const char *value)\n-- \n1.5.4.4\n"},{"id":"74223","messageId":"20080412185620.GF31039@zakalwe.fi","threadId":"13081","inReplyTo":"20080412185105.GA14331@zakalwe.fi","subject":"Re: [PATCH] Make core.sharedRepository more generic","fromName":"Heikki Orsila","fromEmail":"heikki.orsila@iki.fi","sentAt":"2008-04-12T18:56:20Z","receivedAt":"2008-04-12T18:56:20Z","isPatch":true,"sender":{"key":"heikki.orsila@iki.fi","avatar":null},"body":"On Sat, Apr 12, 2008 at 09:51:05PM +0300, Heikki Orsila wrote:\n> git init --shared=0xxx, where '0xxx' is an octal number, will create\n> a repository with file modes set to '0xxx'. User's with a safe umask\n> value (0077) can use this option to force file modes. For example,\n> '0640' is a group-readable but not group-writable regardless of\n> user's umask value.\n> \n> \"git config core.sharedRepository 0xxx\" is also handled.\n\nOops, forgot to sign this one.\n\nSigned-off-by: Heikki Orsila \n\nHeikki\n"},{"id":"74225","messageId":"2008-04-12-21-15-04+trackit+sam@rfc1149.net","threadId":"13081","inReplyTo":"20080412185105.GA14331@zakalwe.fi","subject":"Re: [PATCH] Make core.sharedRepository more generic","fromName":"Samuel Tardieu","fromEmail":"sam@rfc1149.net","sentAt":"2008-04-12T19:15:03Z","receivedAt":"2008-04-12T19:15:03Z","isPatch":true,"sender":{"key":"sam@rfc1149.net","avatar":"https://avatars.githubusercontent.com/u/44656?v=4"},"body":"The use of named constants vs. literals seem inconsistent in your\npatch, compare\n\n| + mode = (mode & ~0777) | shared_repository;\n\nto\n\n| + mode |= (shared_repository & 0600) ? S_IXUSR : 0;\n| + mode |= (shared_repository & 0060) ? S_IXGRP : 0;\n| + mode |= (shared_repository & 0006) ? S_IXOTH : 0;\n\nI first thought that you were using literals with \"shared_repository\"\nand named constants with mode but the first line I quoted shows that\nthis is not the case.\n\nBtw, aren't those last three lines better replaced by\n\n /* Copy read bits to execute bits */\n mode |= (shared_repository & 0444) >> 2;\n\nI don't see where you deal with executable files.\n\nAlso, wouldn't it be more consistent to use a negative value to\n--shared, that is a umask-compatible one, rather than a positive value\nwhich needs to be tweaked for directories and executable files? You\nwould only have to \"&\" 0666 or 0777 with \"~perms\" to get the right\npermissions.\n\n--shared=0007 would be equivalent to PERM_GROUP, --shared=0027 to\ngroup-readable-but-not-writable, and --shared=0002 to PERM_EVERYBODY.\n\n Sam\n-- \nSamuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/\n"},{"id":"74227","messageId":"20080412194634.GG31039@zakalwe.fi","threadId":"13081","inReplyTo":"2008-04-12-21-15-04+trackit+sam@rfc1149.net","subject":"Re: [PATCH] Make core.sharedRepository more generic","fromName":"Heikki Orsila","fromEmail":"shdl@zakalwe.fi","sentAt":"2008-04-12T19:46:34Z","receivedAt":"2008-04-12T19:46:34Z","isPatch":true,"sender":{"key":"shdl@zakalwe.fi","avatar":null},"body":"On Sat, Apr 12, 2008 at 09:15:03PM +0200, Samuel Tardieu wrote:\n> The use of named constants vs. literals seem inconsistent in your\n> patch, compare\n> \n> | + mode = (mode & ~0777) | shared_repository;\n> \n> to\n> \n> | + mode |= (shared_repository & 0600) ? S_IXUSR : 0;\n> | + mode |= (shared_repository & 0060) ? S_IXGRP : 0;\n> | + mode |= (shared_repository & 0006) ? S_IXOTH : 0;\n> \n> I first thought that you were using literals with \"shared_repository\"\n> and named constants with mode but the first line I quoted shows that\n> this is not the case.\n> \n> Btw, aren't those last three lines better replaced by\n> \n> /* Copy read bits to execute bits */\n> mode |= (shared_repository & 0444) >> 2;\n\nAgreed on both previous points. Will submit a new patch.\n\n> I don't see where you deal with executable files.\n\n> Also, wouldn't it be more consistent to use a negative value to\n> --shared, that is a umask-compatible one, rather than a positive value\n> which needs to be tweaked for directories and executable files? You\n> would only have to \"&\" 0666 or 0777 with \"~perms\" to get the right\n> permissions.\n> \n> --shared=0007 would be equivalent to PERM_GROUP, --shared=0027 to\n> group-readable-but-not-writable, and --shared=0002 to PERM_EVERYBODY.\n\nI see the point, but I like to think it as a chmod value rather than a \numask value.\n\n-- \nHeikki Orsila\nheikki.orsila@iki.fi\nhttp://www.iki.fi/shd\n"},{"id":"74228","messageId":"20080412195046.GH31039@zakalwe.fi","threadId":"13081","inReplyTo":"2008-04-12-21-15-04+trackit+sam@rfc1149.net","subject":"Re: [PATCH] Make core.sharedRepository more generic","fromName":"Heikki Orsila","fromEmail":"shdl@zakalwe.fi","sentAt":"2008-04-12T19:50:46Z","receivedAt":"2008-04-12T19:50:46Z","isPatch":true,"sender":{"key":"shdl@zakalwe.fi","avatar":null},"body":"On Sat, Apr 12, 2008 at 09:15:03PM +0200, Samuel Tardieu wrote:\n> I don't see where you deal with executable files.\n\nThis code sets permissions for files written to the .git/ directory. \nAre there script files in the .git/ directory that we want to set \nexecutable flag for?\n\n-- \nHeikki Orsila\nheikki.orsila@iki.fi\nhttp://www.iki.fi/shd\n"},{"id":"74233","messageId":"2008-04-13-00-20-34+trackit+sam@rfc1149.net","threadId":"13081","inReplyTo":"20080412195046.GH31039@zakalwe.fi","subject":"Re: [PATCH] Make core.sharedRepository more generic","fromName":"Samuel Tardieu","fromEmail":"sam@rfc1149.net","sentAt":"2008-04-12T22:20:34Z","receivedAt":"2008-04-12T22:20:34Z","isPatch":true,"sender":{"key":"sam@rfc1149.net","avatar":"https://avatars.githubusercontent.com/u/44656?v=4"},"body":">>>>> \"Heikki\" == Heikki Orsila writes:\n\nHeikki> Are there script files in the .git/ directory that we want to\nHeikki> set executable flag for?\n\nProbably not.\n\n Sam\n-- \nSamuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/\n"}]}