{"thread":{"id":"13119","subject":"[PATCH] Make core.sharedRepository more generic","startedAt":"2008-04-15T09:13:26Z","lastAt":"2008-04-16T05:22:10Z","messageCount":2,"participants":["Heikki Orsila","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"74424","messageId":"20080415091326.GA18100@zakalwe.fi","threadId":"13119","inReplyTo":null,"subject":"[PATCH] Make core.sharedRepository more generic","fromName":"Heikki Orsila","fromEmail":"heikki.orsila@iki.fi","sentAt":"2008-04-15T09:13:26Z","receivedAt":"2008-04-15T09:13:26Z","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'. Users 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. Values compatible with old Git versions are written\nas they were before, for compatibility reasons. That is, \"1\" for\n\"group\" and \"2\" for \"everybody\".\n\n\"git config core.sharedRepository 0xxx\" is also handled.\n\nSigned-off-by: Heikki Orsila <heikki.orsila@iki.fi>\n---\nThis is version 5 of the generic shared mode patch.\n\nVersion 2 handles the directory x flags better than version 1.\n\nVersion 3 removes a warning for the o+w case, fixes a compatibility\nproblem with older Git's, and corrects some style issues.\n\nVersion 4 really fixes the o+w case.\n\nVersion 5 fixes backwards compatibility for old versions of Git.\nPERM_GROUP and PERM_EVERYBODY are written in the old numeric format\nwhen doing \"git init --shared\".\n\nBtw, the current behavior (not introduced by this patch) of\n\"config core.sharedRepository group\" writes string \"group\" to the \nrepository, so it creates incompatible repositories. For completeness\nsake, we would need some kind of hooks to git_config_set_multivar().\nActually, the current architecture bothers me, as there should be some \ncommon \"convention\"/\"system\" for handling compatibility values for all \ncommands.\n\n Documentation/config.txt   |    7 ++++-\n Documentation/git-init.txt |    8 +++++-\n builtin-init-db.c          |   11 ++++++-\n cache.h                    |   16 +++++++++--\n path.c                     |   32 +++++++++++------------\n setup.c                    |   60 +++++++++++++++++++++++++++++++++----------\n 6 files changed, 96 insertions(+), 38 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..a76f5d3 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -400,9 +400,16 @@ 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 for new share modes,\n+\t\t * and compatibility values for PERM_GROUP and\n+\t\t * PERM_EVERYBODY.\n \t\t */\n-\t\tsprintf(buf, \"%d\", shared_repository);\n+\t\tif (shared_repository == PERM_GROUP)\n+\t\t\tsprintf(buf, \"%d\", OLD_PERM_GROUP);\n+\t\telse if (shared_repository == PERM_EVERYBODY)\n+\t\t\tsprintf(buf, \"%d\", OLD_PERM_EVERYBODY);\n+\t\telse\n+\t\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..cfa0529 100644\n--- a/path.c\n+++ b/path.c\n@@ -266,24 +266,22 @@ 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/* Copy read bits to execute bits */\n+\t\tmode |= (shared_repository & 0444) >> 2;\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..1b4fa6a 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -428,21 +428,53 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \n int git_config_perm(const char *var, const char *value)\n {\n-\tif (value) {\n-\t\tint i;\n-\t\tif (!strcmp(value, \"umask\"))\n-\t\t\treturn PERM_UMASK;\n-\t\tif (!strcmp(value, \"group\"))\n-\t\t\treturn PERM_GROUP;\n-\t\tif (!strcmp(value, \"all\") ||\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+\tint i;\n+\tchar *endptr;\n+\n+\tif (value == NULL)\n+\t\treturn PERM_GROUP;\n+\n+\tif (!strcmp(value, \"umask\"))\n+\t\treturn PERM_UMASK;\n+\tif (!strcmp(value, \"group\"))\n+\t\treturn PERM_GROUP;\n+\tif (!strcmp(value, \"all\") ||\n+\t    !strcmp(value, \"world\") ||\n+\t    !strcmp(value, \"everybody\"))\n+\t\treturn PERM_EVERYBODY;\n+\n+\t/* Parse octal numbers */\n+\ti = strtol(value, &endptr, 8);\n+\n+\t/* If not an octal number, maybe true/false? */\n+\tif (*endptr != 0)\n+\t\treturn git_config_bool(var, value) ? PERM_GROUP : PERM_UMASK;\n+\n+\t/*\n+\t * Treat values 0, 1 and 2 as compatibility cases, otherwise it is\n+\t * a chmod value.\n+\t */\n+\tswitch (i) {\n+\tcase PERM_UMASK:               /* 0 */\n+\t\treturn PERM_UMASK;\n+\tcase OLD_PERM_GROUP:           /* 1 */\n+\t\treturn PERM_GROUP;\n+\tcase OLD_PERM_EVERYBODY:       /* 2 */\n+\t\treturn PERM_EVERYBODY;\n \t}\n-\treturn git_config_bool(var, value);\n+\n+\t/* A filemode value was given: 0xxx */\n+\n+\tif ((i & 0600) != 0600)\n+\t\tdie(\"Problem with core.sharedRepository filemode value \"\n+\t\t    \"(0%.3o).\\nThe owner of files must always have \"\n+\t\t    \"read and write permissions.\", i);\n+\n+\t/*\n+\t * Mask filemode value. Others can not get write permission.\n+\t * x flags for directories are handled separately.\n+\t */\n+\treturn i & 0666;\n }\n \n int check_repository_format_version(const char *var, const char *value)\n-- \n1.5.4.4\n"},{"id":"74523","messageId":"7vk5iytlvh.fsf@gitster.siamese.dyndns.org","threadId":"13119","inReplyTo":"20080415091326.GA18100@zakalwe.fi","subject":"Re: [PATCH] Make core.sharedRepository more generic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-16T05:22:10Z","receivedAt":"2008-04-16T05:22:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heikki Orsila <heikki.orsila@iki.fi> writes:\n\n> git init --shared=0xxx, where '0xxx' is an octal number, will create\n> a repository with file modes set to '0xxx'. Users 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. Values compatible with old Git versions are written\n> as they were before, for compatibility reasons. That is, \"1\" for\n> \"group\" and \"2\" for \"everybody\".\n>\n> \"git config core.sharedRepository 0xxx\" is also handled.\n>\n> Signed-off-by: Heikki Orsila <heikki.orsila@iki.fi>\n> ---\n> This is version 5 of the generic shared mode patch.\n\nLooking better.\n\n> Btw, the current behavior (not introduced by this patch) of\n> \"config core.sharedRepository group\" writes string \"group\" to the \n> repository, so it creates incompatible repositories.\n\nYes, but that is what the user explicitly does, so I do not see it as a\ngrave problem compared to the one you fixed during this iteration.\n\nBut the patch breaks t1301.  I would suggest this fix on top of your\nversion.\n\n---\n\n path.c                 |    5 ++++-\n t/t1301-shared-repo.sh |   40 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 44 insertions(+), 1 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex cfa0529..2ae7cd9 100644\n--- a/path.c\n+++ b/path.c\n@@ -268,7 +268,10 @@ int adjust_shared_perm(const char *path)\n \tmode = st.st_mode;\n \n \tif (shared_repository) {\n-\t\tmode = (mode & ~0777) | shared_repository;\n+\t\tint tweak = shared_repository;\n+\t\tif (!(mode & S_IWUSR))\n+\t\t\ttweak &= ~0222;\n+\t\tmode = (mode & ~0777) | tweak;\n \t} else {\n \t\t/* Preserve old PERM_UMASK behaviour */\n \t\tif (mode & S_IWUSR)\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex 6bfe19a..586dab1 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -33,4 +33,44 @@ test_expect_success 'update-server-info honors core.sharedRepository' '\n \tesac\n '\n \n+for u in\t0660:rw-rw---- \\\n+\t\t0640:rw-r----- \\\n+\t\t0600:rw------- \\\n+\t\t0666:rw-rw-rw- \\\n+\t\t0664:rw-rw-r--\n+do\n+\tx=$(expr \"$u\" : \".*:\\([rw-]*\\)\") &&\n+\ty=$(echo \"$x\" | sed -e \"s/w/-/g\") &&\n+\tu=$(expr \"$u\" : \"\\([0-7]*\\)\") &&\n+\tgit config core.sharedrepository \"$u\" &&\n+\tumask 0277 &&\n+\n+\ttest_expect_success \"shared = $u ($y) ro\" '\n+\n+\t\trm -f .git/info/refs &&\n+\t\tgit update-server-info &&\n+\t\tactual=\"$(ls -l .git/info/refs)\" &&\n+\t\tactual=${actual%% *} &&\n+\t\ttest \"x$actual\" = \"x-$y\" || {\n+\t\t\tls -lt .git/info\n+\t\t\tfalse\n+\t\t}\n+\t'\n+\n+\tumask 077 &&\n+\ttest_expect_success \"shared = $u ($x) rw\" '\n+\n+\t\trm -f .git/info/refs &&\n+\t\tgit update-server-info &&\n+\t\tactual=\"$(ls -l .git/info/refs)\" &&\n+\t\tactual=${actual%% *} &&\n+\t\ttest \"x$actual\" = \"x-$x\" || {\n+\t\t\tls -lt .git/info\n+\t\t\tfalse\n+\t\t}\n+\n+\t'\n+\n+done\n+\n test_done\n"}]}