[PATCH] Make core.sharedRepository more generic

[PATCH] Make core.sharedRepository more generic

[Heikki Orsila]

git init --shared=0xxx, where '0xxx' is an octal number, will create
a repository with file modes set to '0xxx'. User's with a safe umask
value (0077) can use this option to force file modes. For example,
'0640' is a group-readable but not group-writable regardless of
user's umask value.

"git config core.sharedRepository 0xxx" is also handled.
---
 Documentation/config.txt   |    7 +++++-
 Documentation/git-init.txt |    8 ++++++-
 builtin-init-db.c          |    4 +-
 cache.h                    |   16 ++++++++++++--
 path.c                     |   38 ++++++++++++++++++++----------------
 setup.c                    |   45 ++++++++++++++++++++++++++++++++++++++++---
 6 files changed, 90 insertions(+), 28 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fe43b12..7a24f6e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -261,7 +261,12 @@ core.sharedRepository::
 	group-writable). When 'all' (or 'world' or 'everybody'), the
 	repository will be readable by all users, additionally to being
 	group-shareable. When 'umask' (or 'false'), git will use permissions
-	reported by umask(2). See linkgit:git-init[1]. False by default.
+	reported by umask(2). When '0xxx', where '0xxx' is an octal number,
+	files in the repository will have this mode value. '0xxx' will override
+	user's umask value, and thus, users with a safe umask (0077) can use
+	this option. Examples: '0660' is equivalent to 'group'. '0640' is a
+	repository that is group-readable but not group-writable.
+	See linkgit:git-init[1]. False by default.
 
 core.warnAmbiguousRefs::
 	If true, git will warn you if the ref name you passed it is ambiguous
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 62914da..b17ae84 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -31,7 +31,7 @@ structure, some suggested "exclude patterns", and copies of non-executing
 "hook" files.  The suggested patterns and hook files are all modifiable and
 extensible.
 
---shared[={false|true|umask|group|all|world|everybody}]::
+--shared[={false|true|umask|group|all|world|everybody|0xxx}]::
 
 Specify that the git repository is to be shared amongst several users.  This
 allows users belonging to the same group to push into that
@@ -52,6 +52,12 @@ is given:
  - 'all' (or 'world' or 'everybody'): Same as 'group', but make the repository
    readable by all users.
 
+ - '0xxx': '0xxx' is an octal number and each file will have mode '0xxx'
+   Any option except 'umask' can be set using this option. '0xxx' will
+   override users umask(2) value, and thus, users with a safe umask (0077)
+   can use this option. '0640' will create a repository which is group-readable
+   but not writable. '0660' is equivalent to 'group'.
+
 By default, the configuration flag receive.denyNonFastForwards is enabled
 in shared repositories, so that you cannot force a non fast-forwarding push
 into it.
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 2854868..8c63295 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -400,9 +400,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 		char buf[10];
 		/* We do not spell "group" and such, so that
 		 * the configuration can be read by older version
-		 * of git.
+		 * of git. Note, we use octal numbers.
 		 */
-		sprintf(buf, "%d", shared_repository);
+		sprintf(buf, "0%o", shared_repository);
 		git_config_set("core.sharedrepository", buf);
 		git_config_set("receive.denyNonFastforwards", "true");
 	}
diff --git a/cache.h b/cache.h
index 2a1e7ec..f9c8d2b 100644
--- a/cache.h
+++ b/cache.h
@@ -474,10 +474,20 @@ static inline void hashclr(unsigned char *hash)
 
 int git_mkstemp(char *path, size_t n, const char *template);
 
+/*
+ * NOTE NOTE NOTE!!
+ *
+ * PERM_UMASK, OLD_PERM_GROUP and OLD_PERM_EVERYBODY enumerations must
+ * not be changed. Old repositories have core.sharedrepository written in
+ * numeric format, and therefore these values are preserved for compatibility
+ * reasons.
+ */
 enum sharedrepo {
-	PERM_UMASK = 0,
-	PERM_GROUP,
-	PERM_EVERYBODY
+	PERM_UMASK          = 0,
+	OLD_PERM_GROUP      = 1,
+	OLD_PERM_EVERYBODY  = 2,
+	PERM_GROUP          = 0660,
+	PERM_EVERYBODY      = 0664,
 };
 int git_config_perm(const char *var, const char *value);
 int adjust_shared_perm(const char *path);
diff --git a/path.c b/path.c
index f4ed979..79bfe82 100644
--- a/path.c
+++ b/path.c
@@ -266,24 +266,28 @@ int adjust_shared_perm(const char *path)
 	if (lstat(path, &st) < 0)
 		return -1;
 	mode = st.st_mode;
-	if (mode & S_IRUSR)
-		mode |= (shared_repository == PERM_GROUP
-			 ? S_IRGRP
-			 : (shared_repository == PERM_EVERYBODY
-			    ? (S_IRGRP|S_IROTH)
-			    : 0));
-
-	if (mode & S_IWUSR)
-		mode |= S_IWGRP;
-
-	if (mode & S_IXUSR)
-		mode |= (shared_repository == PERM_GROUP
-			 ? S_IXGRP
-			 : (shared_repository == PERM_EVERYBODY
-			    ? (S_IXGRP|S_IXOTH)
-			    : 0));
-	if (S_ISDIR(mode))
+
+	if (shared_repository) {
+		mode = (mode & ~0777) | shared_repository;
+	} else {
+		/* Preserve old PERM_UMASK behaviour */
+		if (mode & S_IWUSR)
+			mode |= S_IWGRP;
+	}
+
+	if (S_ISDIR(mode)) {
 		mode |= FORCE_DIR_SET_GID;
+
+		/*
+		 * The x flag for directories is determined from rw flags of
+		 * user, group and others. Having a directory with +rw but
+		 * -x does not make sense for Git repositories.
+		 */
+		mode |= (shared_repository & 0600) ? S_IXUSR : 0;
+		mode |= (shared_repository & 0060) ? S_IXGRP : 0;
+		mode |= (shared_repository & 0006) ? S_IXOTH : 0;
+	}
+
 	if ((mode & st.st_mode) != mode && chmod(path, mode) < 0)
 		return -2;
 	return 0;
diff --git a/setup.c b/setup.c
index 3d2d958..0905509 100644
--- a/setup.c
+++ b/setup.c
@@ -430,6 +430,8 @@ int git_config_perm(const char *var, const char *value)
 {
 	if (value) {
 		int i;
+		char *endptr;
+
 		if (!strcmp(value, "umask"))
 			return PERM_UMASK;
 		if (!strcmp(value, "group"))
@@ -438,11 +440,46 @@ int git_config_perm(const char *var, const char *value)
 		    !strcmp(value, "world") ||
 		    !strcmp(value, "everybody"))
 			return PERM_EVERYBODY;
-		i = atoi(value);
-		if (i > 1)
-			return i;
+
+		/* Parse octal numbers */
+		i = strtol(value, &endptr, 8);
+		if (*endptr != 0) {
+			/* Not an octal number. Maybe true/false? */
+			if (git_config_bool(var, value))
+				return PERM_GROUP;
+			else
+				return PERM_UMASK;
+		}
+
+		/* Handle compatibility cases */
+		switch (i) {
+		case PERM_UMASK:               /* 0 */
+			return PERM_UMASK;
+		case OLD_PERM_GROUP:           /* 1 */
+			return PERM_GROUP;
+		case OLD_PERM_EVERYBODY:       /* 2 */
+			return PERM_EVERYBODY;
+		}
+
+		/* A filemode value was given: 0xxx */
+
+		if ((i & 0600) != 0600)
+			die("Problem with core.sharedRepository filemode value"
+			    " (0%.3o).\nThe owner of files must always have "
+			    "read and write permissions.", i);
+
+		if (i & 0002)
+			warning("core.sharedRepository filemode (0%.3o) is a "
+				"security threat.\nMasking off write "
+				"permission for others\n", i);
+
+		/*
+                 * Mask filemode value. Others can not get write permission.
+		 * x flags for directories are handled separately.
+                 */
+		return i & 0664;
 	}
-	return git_config_bool(var, value);
+	return PERM_GROUP;
 }
 
 int check_repository_format_version(const char *var, const char *value)
-- 
1.5.4.4

Re: [PATCH] Make core.sharedRepository more generic

[Heikki Orsila]

On Sat, Apr 12, 2008 at 09:51:05PM +0300, Heikki Orsila wrote:
> git init --shared=0xxx, where '0xxx' is an octal number, will create
> a repository with file modes set to '0xxx'. User's with a safe umask
> value (0077) can use this option to force file modes. For example,
> '0640' is a group-readable but not group-writable regardless of
> user's umask value.
> 
> "git config core.sharedRepository 0xxx" is also handled.

Oops, forgot to sign this one.

Signed-off-by: Heikki Orsila <heikki.orsila@iki.fi>

Heikki

Re: [PATCH] Make core.sharedRepository more generic

[Samuel Tardieu]

The use of named constants vs. literals seem inconsistent in your
patch, compare

| +               mode = (mode & ~0777) | shared_repository;

to

| +               mode |= (shared_repository & 0600) ? S_IXUSR : 0;
| +               mode |= (shared_repository & 0060) ? S_IXGRP : 0;
| +               mode |= (shared_repository & 0006) ? S_IXOTH : 0;

I first thought that you were using literals with "shared_repository"
and named constants with mode but the first line I quoted shows that
this is not the case.

Btw, aren't those last three lines better replaced by

  /* Copy read bits to execute bits */
  mode |= (shared_repository & 0444) >> 2;

I don't see where you deal with executable files.

Also, wouldn't it be more consistent to use a negative value to
--shared, that is a umask-compatible one, rather than a positive value
which needs to be tweaked for directories and executable files? You
would only have to "&" 0666 or 0777 with "~perms" to get the right
permissions.

--shared=0007 would be equivalent to PERM_GROUP, --shared=0027 to
group-readable-but-not-writable, and --shared=0002 to PERM_EVERYBODY.

  Sam
-- 
Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/

Re: [PATCH] Make core.sharedRepository more generic

[Heikki Orsila]

On Sat, Apr 12, 2008 at 09:15:03PM +0200, Samuel Tardieu wrote:
> The use of named constants vs. literals seem inconsistent in your
> patch, compare
> 
> | +               mode = (mode & ~0777) | shared_repository;
> 
> to
> 
> | +               mode |= (shared_repository & 0600) ? S_IXUSR : 0;
> | +               mode |= (shared_repository & 0060) ? S_IXGRP : 0;
> | +               mode |= (shared_repository & 0006) ? S_IXOTH : 0;
> 
> I first thought that you were using literals with "shared_repository"
> and named constants with mode but the first line I quoted shows that
> this is not the case.
> 
> Btw, aren't those last three lines better replaced by
> 
>   /* Copy read bits to execute bits */
>   mode |= (shared_repository & 0444) >> 2;

Agreed on both previous points. Will submit a new patch.

> I don't see where you deal with executable files.

> Also, wouldn't it be more consistent to use a negative value to
> --shared, that is a umask-compatible one, rather than a positive value
> which needs to be tweaked for directories and executable files? You
> would only have to "&" 0666 or 0777 with "~perms" to get the right
> permissions.
> 
> --shared=0007 would be equivalent to PERM_GROUP, --shared=0027 to
> group-readable-but-not-writable, and --shared=0002 to PERM_EVERYBODY.

I see the point, but I like to think it as a chmod value rather than a 
umask value.

-- 
Heikki Orsila
heikki.orsila@iki.fi
http://www.iki.fi/shd

Re: [PATCH] Make core.sharedRepository more generic

[Heikki Orsila]

On Sat, Apr 12, 2008 at 09:15:03PM +0200, Samuel Tardieu wrote:
> I don't see where you deal with executable files.

This code sets permissions for files written to the .git/ directory. 
Are there script files in the .git/ directory that we want to set 
executable flag for?

-- 
Heikki Orsila
heikki.orsila@iki.fi
http://www.iki.fi/shd

Re: [PATCH] Make core.sharedRepository more generic

[Samuel Tardieu]

>>>>> "Heikki" == Heikki Orsila <shdl@zakalwe.fi> writes:

Heikki> Are there script files in the .git/ directory that we want to
Heikki> set executable flag for?

Probably not.

  Sam
-- 
Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/

[PATCH] Make core.sharedRepository more generic

[Heikki Orsila]

git init --shared=0xxx, where '0xxx' is an octal number, will create
a repository with file modes set to '0xxx'. Users with a safe umask
value (0077) can use this option to force file modes. For example,
'0640' is a group-readable but not group-writable regardless of
user's umask value. Values compatible with old Git versions are written
as they were before, for compatibility reasons. That is, "1" for
"group" and "2" for "everybody".

"git config core.sharedRepository 0xxx" is also handled.

Signed-off-by: Heikki Orsila <heikki.orsila@iki.fi>
---
This is version 5 of the generic shared mode patch.

Version 2 handles the directory x flags better than version 1.

Version 3 removes a warning for the o+w case, fixes a compatibility
problem with older Git's, and corrects some style issues.

Version 4 really fixes the o+w case.

Version 5 fixes backwards compatibility for old versions of Git.
PERM_GROUP and PERM_EVERYBODY are written in the old numeric format
when doing "git init --shared".

Btw, the current behavior (not introduced by this patch) of
"config core.sharedRepository group" writes string "group" to the 
repository, so it creates incompatible repositories. For completeness
sake, we would need some kind of hooks to git_config_set_multivar().
Actually, the current architecture bothers me, as there should be some 
common "convention"/"system" for handling compatibility values for all 
commands.

 Documentation/config.txt   |    7 ++++-
 Documentation/git-init.txt |    8 +++++-
 builtin-init-db.c          |   11 ++++++-
 cache.h                    |   16 +++++++++--
 path.c                     |   32 +++++++++++------------
 setup.c                    |   60 +++++++++++++++++++++++++++++++++----------
 6 files changed, 96 insertions(+), 38 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fe43b12..7a24f6e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -261,7 +261,12 @@ core.sharedRepository::
 	group-writable). When 'all' (or 'world' or 'everybody'), the
 	repository will be readable by all users, additionally to being
 	group-shareable. When 'umask' (or 'false'), git will use permissions
-	reported by umask(2). See linkgit:git-init[1]. False by default.
+	reported by umask(2). When '0xxx', where '0xxx' is an octal number,
+	files in the repository will have this mode value. '0xxx' will override
+	user's umask value, and thus, users with a safe umask (0077) can use
+	this option. Examples: '0660' is equivalent to 'group'. '0640' is a
+	repository that is group-readable but not group-writable.
+	See linkgit:git-init[1]. False by default.
 
 core.warnAmbiguousRefs::
 	If true, git will warn you if the ref name you passed it is ambiguous
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 62914da..b17ae84 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -31,7 +31,7 @@ structure, some suggested "exclude patterns", and copies of non-executing
 "hook" files.  The suggested patterns and hook files are all modifiable and
 extensible.
 
---shared[={false|true|umask|group|all|world|everybody}]::
+--shared[={false|true|umask|group|all|world|everybody|0xxx}]::
 
 Specify that the git repository is to be shared amongst several users.  This
 allows users belonging to the same group to push into that
@@ -52,6 +52,12 @@ is given:
  - 'all' (or 'world' or 'everybody'): Same as 'group', but make the repository
    readable by all users.
 
+ - '0xxx': '0xxx' is an octal number and each file will have mode '0xxx'
+   Any option except 'umask' can be set using this option. '0xxx' will
+   override users umask(2) value, and thus, users with a safe umask (0077)
+   can use this option. '0640' will create a repository which is group-readable
+   but not writable. '0660' is equivalent to 'group'.
+
 By default, the configuration flag receive.denyNonFastForwards is enabled
 in shared repositories, so that you cannot force a non fast-forwarding push
 into it.
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 2854868..a76f5d3 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -400,9 +400,16 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 		char buf[10];
 		/* We do not spell "group" and such, so that
 		 * the configuration can be read by older version
-		 * of git.
+		 * of git. Note, we use octal numbers for new share modes,
+		 * and compatibility values for PERM_GROUP and
+		 * PERM_EVERYBODY.
 		 */
-		sprintf(buf, "%d", shared_repository);
+		if (shared_repository == PERM_GROUP)
+			sprintf(buf, "%d", OLD_PERM_GROUP);
+		else if (shared_repository == PERM_EVERYBODY)
+			sprintf(buf, "%d", OLD_PERM_EVERYBODY);
+		else
+			sprintf(buf, "0%o", shared_repository);
 		git_config_set("core.sharedrepository", buf);
 		git_config_set("receive.denyNonFastforwards", "true");
 	}
diff --git a/cache.h b/cache.h
index 2a1e7ec..f9c8d2b 100644
--- a/cache.h
+++ b/cache.h
@@ -474,10 +474,20 @@ static inline void hashclr(unsigned char *hash)
 
 int git_mkstemp(char *path, size_t n, const char *template);
 
+/*
+ * NOTE NOTE NOTE!!
+ *
+ * PERM_UMASK, OLD_PERM_GROUP and OLD_PERM_EVERYBODY enumerations must
+ * not be changed. Old repositories have core.sharedrepository written in
+ * numeric format, and therefore these values are preserved for compatibility
+ * reasons.
+ */
 enum sharedrepo {
-	PERM_UMASK = 0,
-	PERM_GROUP,
-	PERM_EVERYBODY
+	PERM_UMASK          = 0,
+	OLD_PERM_GROUP      = 1,
+	OLD_PERM_EVERYBODY  = 2,
+	PERM_GROUP          = 0660,
+	PERM_EVERYBODY      = 0664,
 };
 int git_config_perm(const char *var, const char *value);
 int adjust_shared_perm(const char *path);
diff --git a/path.c b/path.c
index f4ed979..cfa0529 100644
--- a/path.c
+++ b/path.c
@@ -266,24 +266,22 @@ int adjust_shared_perm(const char *path)
 	if (lstat(path, &st) < 0)
 		return -1;
 	mode = st.st_mode;
-	if (mode & S_IRUSR)
-		mode |= (shared_repository == PERM_GROUP
-			 ? S_IRGRP
-			 : (shared_repository == PERM_EVERYBODY
-			    ? (S_IRGRP|S_IROTH)
-			    : 0));
-
-	if (mode & S_IWUSR)
-		mode |= S_IWGRP;
-
-	if (mode & S_IXUSR)
-		mode |= (shared_repository == PERM_GROUP
-			 ? S_IXGRP
-			 : (shared_repository == PERM_EVERYBODY
-			    ? (S_IXGRP|S_IXOTH)
-			    : 0));
-	if (S_ISDIR(mode))
+
+	if (shared_repository) {
+		mode = (mode & ~0777) | shared_repository;
+	} else {
+		/* Preserve old PERM_UMASK behaviour */
+		if (mode & S_IWUSR)
+			mode |= S_IWGRP;
+	}
+
+	if (S_ISDIR(mode)) {
 		mode |= FORCE_DIR_SET_GID;
+
+		/* Copy read bits to execute bits */
+		mode |= (shared_repository & 0444) >> 2;
+	}
+
 	if ((mode & st.st_mode) != mode && chmod(path, mode) < 0)
 		return -2;
 	return 0;
diff --git a/setup.c b/setup.c
index 3d2d958..1b4fa6a 100644
--- a/setup.c
+++ b/setup.c
@@ -428,21 +428,53 @@ const char *setup_git_directory_gently(int *nongit_ok)
 
 int git_config_perm(const char *var, const char *value)
 {
-	if (value) {
-		int i;
-		if (!strcmp(value, "umask"))
-			return PERM_UMASK;
-		if (!strcmp(value, "group"))
-			return PERM_GROUP;
-		if (!strcmp(value, "all") ||
-		    !strcmp(value, "world") ||
-		    !strcmp(value, "everybody"))
-			return PERM_EVERYBODY;
-		i = atoi(value);
-		if (i > 1)
-			return i;
+	int i;
+	char *endptr;
+
+	if (value == NULL)
+		return PERM_GROUP;
+
+	if (!strcmp(value, "umask"))
+		return PERM_UMASK;
+	if (!strcmp(value, "group"))
+		return PERM_GROUP;
+	if (!strcmp(value, "all") ||
+	    !strcmp(value, "world") ||
+	    !strcmp(value, "everybody"))
+		return PERM_EVERYBODY;
+
+	/* Parse octal numbers */
+	i = strtol(value, &endptr, 8);
+
+	/* If not an octal number, maybe true/false? */
+	if (*endptr != 0)
+		return git_config_bool(var, value) ? PERM_GROUP : PERM_UMASK;
+
+	/*
+	 * Treat values 0, 1 and 2 as compatibility cases, otherwise it is
+	 * a chmod value.
+	 */
+	switch (i) {
+	case PERM_UMASK:               /* 0 */
+		return PERM_UMASK;
+	case OLD_PERM_GROUP:           /* 1 */
+		return PERM_GROUP;
+	case OLD_PERM_EVERYBODY:       /* 2 */
+		return PERM_EVERYBODY;
 	}
-	return git_config_bool(var, value);
+
+	/* A filemode value was given: 0xxx */
+
+	if ((i & 0600) != 0600)
+		die("Problem with core.sharedRepository filemode value "
+		    "(0%.3o).\nThe owner of files must always have "
+		    "read and write permissions.", i);
+
+	/*
+	 * Mask filemode value. Others can not get write permission.
+	 * x flags for directories are handled separately.
+	 */
+	return i & 0666;
 }
 
 int check_repository_format_version(const char *var, const char *value)
-- 
1.5.4.4

Re: [PATCH] Make core.sharedRepository more generic

[Junio C Hamano]

Heikki Orsila <heikki.orsila@iki.fi> writes:

> git init --shared=0xxx, where '0xxx' is an octal number, will create
> a repository with file modes set to '0xxx'. Users with a safe umask
> value (0077) can use this option to force file modes. For example,
> '0640' is a group-readable but not group-writable regardless of
> user's umask value. Values compatible with old Git versions are written
> as they were before, for compatibility reasons. That is, "1" for
> "group" and "2" for "everybody".
>
> "git config core.sharedRepository 0xxx" is also handled.
>
> Signed-off-by: Heikki Orsila <heikki.orsila@iki.fi>
> ---
> This is version 5 of the generic shared mode patch.

Looking better.

> Btw, the current behavior (not introduced by this patch) of
> "config core.sharedRepository group" writes string "group" to the 
> repository, so it creates incompatible repositories.

Yes, but that is what the user explicitly does, so I do not see it as a
grave problem compared to the one you fixed during this iteration.

But the patch breaks t1301.  I would suggest this fix on top of your
version.

---

 path.c                 |    5 ++++-
 t/t1301-shared-repo.sh |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletions(-)

diff --git a/path.c b/path.c
index cfa0529..2ae7cd9 100644
--- a/path.c
+++ b/path.c
@@ -268,7 +268,10 @@ int adjust_shared_perm(const char *path)
 	mode = st.st_mode;
 
 	if (shared_repository) {
-		mode = (mode & ~0777) | shared_repository;
+		int tweak = shared_repository;
+		if (!(mode & S_IWUSR))
+			tweak &= ~0222;
+		mode = (mode & ~0777) | tweak;
 	} else {
 		/* Preserve old PERM_UMASK behaviour */
 		if (mode & S_IWUSR)
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 6bfe19a..586dab1 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -33,4 +33,44 @@ test_expect_success 'update-server-info honors core.sharedRepository' '
 	esac
 '
 
+for u in	0660:rw-rw---- \
+		0640:rw-r----- \
+		0600:rw------- \
+		0666:rw-rw-rw- \
+		0664:rw-rw-r--
+do
+	x=$(expr "$u" : ".*:\([rw-]*\)") &&
+	y=$(echo "$x" | sed -e "s/w/-/g") &&
+	u=$(expr "$u" : "\([0-7]*\)") &&
+	git config core.sharedrepository "$u" &&
+	umask 0277 &&
+
+	test_expect_success "shared = $u ($y) ro" '
+
+		rm -f .git/info/refs &&
+		git update-server-info &&
+		actual="$(ls -l .git/info/refs)" &&
+		actual=${actual%% *} &&
+		test "x$actual" = "x-$y" || {
+			ls -lt .git/info
+			false
+		}
+	'
+
+	umask 077 &&
+	test_expect_success "shared = $u ($x) rw" '
+
+		rm -f .git/info/refs &&
+		git update-server-info &&
+		actual="$(ls -l .git/info/refs)" &&
+		actual=${actual%% *} &&
+		test "x$actual" = "x-$x" || {
+			ls -lt .git/info
+			false
+		}
+
+	'
+
+done
+
 test_done

[PATCH] Make core.sharedRepository more generic

[Heikki Orsila]

git init --shared=0xxx, where '0xxx' is an octal number, will create
a repository with file modes set to '0xxx'. Users with a safe umask
value (0077) can use this option to force file modes. For example,
'0640' is a group-readable but not group-writable regardless of
user's umask value. Values compatible with old Git versions are written
as they were before, for compatibility reasons. That is, "1" for
"group" and "2" for "everybody".

"git config core.sharedRepository 0xxx" is also handled.

Signed-off-by: Heikki Orsila <heikki.orsila@iki.fi>
---
This is version 6 of the generic shared mode patch.

Version 6 adds test cases, and fixes a shared repository case: "u-w" -> "a-w"

Version 5 fixes backwards compatibility for old versions of Git.
PERM_GROUP and PERM_EVERYBODY are written in the old numeric format
when doing "git init --shared".

Version 4 really fixes the o+w case.

Version 3 removes a warning for the o+w case, fixes a compatibility
problem with older Git's, and corrects some style issues.

Version 2 handles the directory x flags better than version 1.

 Documentation/config.txt   |    7 ++++-
 Documentation/git-init.txt |    8 +++++-
 builtin-init-db.c          |   11 ++++++-
 cache.h                    |   16 +++++++++--
 path.c                     |   35 +++++++++++++------------
 setup.c                    |   60 +++++++++++++++++++++++++++++++++----------
 t/t1301-shared-repo.sh     |   51 +++++++++++++++++++++++++++++++++++++
 7 files changed, 150 insertions(+), 38 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fe43b12..7a24f6e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -261,7 +261,12 @@ core.sharedRepository::
 	group-writable). When 'all' (or 'world' or 'everybody'), the
 	repository will be readable by all users, additionally to being
 	group-shareable. When 'umask' (or 'false'), git will use permissions
-	reported by umask(2). See linkgit:git-init[1]. False by default.
+	reported by umask(2). When '0xxx', where '0xxx' is an octal number,
+	files in the repository will have this mode value. '0xxx' will override
+	user's umask value, and thus, users with a safe umask (0077) can use
+	this option. Examples: '0660' is equivalent to 'group'. '0640' is a
+	repository that is group-readable but not group-writable.
+	See linkgit:git-init[1]. False by default.
 
 core.warnAmbiguousRefs::
 	If true, git will warn you if the ref name you passed it is ambiguous
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 62914da..b17ae84 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -31,7 +31,7 @@ structure, some suggested "exclude patterns", and copies of non-executing
 "hook" files.  The suggested patterns and hook files are all modifiable and
 extensible.
 
---shared[={false|true|umask|group|all|world|everybody}]::
+--shared[={false|true|umask|group|all|world|everybody|0xxx}]::
 
 Specify that the git repository is to be shared amongst several users.  This
 allows users belonging to the same group to push into that
@@ -52,6 +52,12 @@ is given:
  - 'all' (or 'world' or 'everybody'): Same as 'group', but make the repository
    readable by all users.
 
+ - '0xxx': '0xxx' is an octal number and each file will have mode '0xxx'
+   Any option except 'umask' can be set using this option. '0xxx' will
+   override users umask(2) value, and thus, users with a safe umask (0077)
+   can use this option. '0640' will create a repository which is group-readable
+   but not writable. '0660' is equivalent to 'group'.
+
 By default, the configuration flag receive.denyNonFastForwards is enabled
 in shared repositories, so that you cannot force a non fast-forwarding push
 into it.
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 2854868..a76f5d3 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -400,9 +400,16 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 		char buf[10];
 		/* We do not spell "group" and such, so that
 		 * the configuration can be read by older version
-		 * of git.
+		 * of git. Note, we use octal numbers for new share modes,
+		 * and compatibility values for PERM_GROUP and
+		 * PERM_EVERYBODY.
 		 */
-		sprintf(buf, "%d", shared_repository);
+		if (shared_repository == PERM_GROUP)
+			sprintf(buf, "%d", OLD_PERM_GROUP);
+		else if (shared_repository == PERM_EVERYBODY)
+			sprintf(buf, "%d", OLD_PERM_EVERYBODY);
+		else
+			sprintf(buf, "0%o", shared_repository);
 		git_config_set("core.sharedrepository", buf);
 		git_config_set("receive.denyNonFastforwards", "true");
 	}
diff --git a/cache.h b/cache.h
index 2a1e7ec..f9c8d2b 100644
--- a/cache.h
+++ b/cache.h
@@ -474,10 +474,20 @@ static inline void hashclr(unsigned char *hash)
 
 int git_mkstemp(char *path, size_t n, const char *template);
 
+/*
+ * NOTE NOTE NOTE!!
+ *
+ * PERM_UMASK, OLD_PERM_GROUP and OLD_PERM_EVERYBODY enumerations must
+ * not be changed. Old repositories have core.sharedrepository written in
+ * numeric format, and therefore these values are preserved for compatibility
+ * reasons.
+ */
 enum sharedrepo {
-	PERM_UMASK = 0,
-	PERM_GROUP,
-	PERM_EVERYBODY
+	PERM_UMASK          = 0,
+	OLD_PERM_GROUP      = 1,
+	OLD_PERM_EVERYBODY  = 2,
+	PERM_GROUP          = 0660,
+	PERM_EVERYBODY      = 0664,
 };
 int git_config_perm(const char *var, const char *value);
 int adjust_shared_perm(const char *path);
diff --git a/path.c b/path.c
index f4ed979..2ae7cd9 100644
--- a/path.c
+++ b/path.c
@@ -266,24 +266,25 @@ int adjust_shared_perm(const char *path)
 	if (lstat(path, &st) < 0)
 		return -1;
 	mode = st.st_mode;
-	if (mode & S_IRUSR)
-		mode |= (shared_repository == PERM_GROUP
-			 ? S_IRGRP
-			 : (shared_repository == PERM_EVERYBODY
-			    ? (S_IRGRP|S_IROTH)
-			    : 0));
-
-	if (mode & S_IWUSR)
-		mode |= S_IWGRP;
-
-	if (mode & S_IXUSR)
-		mode |= (shared_repository == PERM_GROUP
-			 ? S_IXGRP
-			 : (shared_repository == PERM_EVERYBODY
-			    ? (S_IXGRP|S_IXOTH)
-			    : 0));
-	if (S_ISDIR(mode))
+
+	if (shared_repository) {
+		int tweak = shared_repository;
+		if (!(mode & S_IWUSR))
+			tweak &= ~0222;
+		mode = (mode & ~0777) | tweak;
+	} else {
+		/* Preserve old PERM_UMASK behaviour */
+		if (mode & S_IWUSR)
+			mode |= S_IWGRP;
+	}
+
+	if (S_ISDIR(mode)) {
 		mode |= FORCE_DIR_SET_GID;
+
+		/* Copy read bits to execute bits */
+		mode |= (shared_repository & 0444) >> 2;
+	}
+
 	if ((mode & st.st_mode) != mode && chmod(path, mode) < 0)
 		return -2;
 	return 0;
diff --git a/setup.c b/setup.c
index 3d2d958..1b4fa6a 100644
--- a/setup.c
+++ b/setup.c
@@ -428,21 +428,53 @@ const char *setup_git_directory_gently(int *nongit_ok)
 
 int git_config_perm(const char *var, const char *value)
 {
-	if (value) {
-		int i;
-		if (!strcmp(value, "umask"))
-			return PERM_UMASK;
-		if (!strcmp(value, "group"))
-			return PERM_GROUP;
-		if (!strcmp(value, "all") ||
-		    !strcmp(value, "world") ||
-		    !strcmp(value, "everybody"))
-			return PERM_EVERYBODY;
-		i = atoi(value);
-		if (i > 1)
-			return i;
+	int i;
+	char *endptr;
+
+	if (value == NULL)
+		return PERM_GROUP;
+
+	if (!strcmp(value, "umask"))
+		return PERM_UMASK;
+	if (!strcmp(value, "group"))
+		return PERM_GROUP;
+	if (!strcmp(value, "all") ||
+	    !strcmp(value, "world") ||
+	    !strcmp(value, "everybody"))
+		return PERM_EVERYBODY;
+
+	/* Parse octal numbers */
+	i = strtol(value, &endptr, 8);
+
+	/* If not an octal number, maybe true/false? */
+	if (*endptr != 0)
+		return git_config_bool(var, value) ? PERM_GROUP : PERM_UMASK;
+
+	/*
+	 * Treat values 0, 1 and 2 as compatibility cases, otherwise it is
+	 * a chmod value.
+	 */
+	switch (i) {
+	case PERM_UMASK:               /* 0 */
+		return PERM_UMASK;
+	case OLD_PERM_GROUP:           /* 1 */
+		return PERM_GROUP;
+	case OLD_PERM_EVERYBODY:       /* 2 */
+		return PERM_EVERYBODY;
 	}
-	return git_config_bool(var, value);
+
+	/* A filemode value was given: 0xxx */
+
+	if ((i & 0600) != 0600)
+		die("Problem with core.sharedRepository filemode value "
+		    "(0%.3o).\nThe owner of files must always have "
+		    "read and write permissions.", i);
+
+	/*
+	 * Mask filemode value. Others can not get write permission.
+	 * x flags for directories are handled separately.
+	 */
+	return i & 0666;
 }
 
 int check_repository_format_version(const char *var, const char *value)
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 6bfe19a..fe6238f 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -7,6 +7,17 @@ test_description='Test shared repository initialization'
 
 . ./test-lib.sh
 
+# User must have read permissions to the repo -> failure on --shared=0400
+test_expect_success 'shared = 0400 (faulty permission u-w)' '
+	mkdir sub && cd sub
+	if test "$?" != "0" ; then return 1 ; fi
+	git init --shared=0400
+	ret="$?"
+	cd ..
+	rm -rf sub
+	test $ret != "0"
+'
+
 test_expect_success 'shared=all' '
 	mkdir sub &&
 	cd sub &&
@@ -33,4 +44,44 @@ test_expect_success 'update-server-info honors core.sharedRepository' '
 	esac
 '
 
+for u in	0660:rw-rw---- \
+		0640:rw-r----- \
+		0600:rw------- \
+		0666:rw-rw-rw- \
+		0664:rw-rw-r--
+do
+	x=$(expr "$u" : ".*:\([rw-]*\)") &&
+	y=$(echo "$x" | sed -e "s/w/-/g") &&
+	u=$(expr "$u" : "\([0-7]*\)") &&
+	git config core.sharedrepository "$u" &&
+	umask 0277 &&
+
+	test_expect_success "shared = $u ($y) ro" '
+
+		rm -f .git/info/refs &&
+		git update-server-info &&
+		actual="$(ls -l .git/info/refs)" &&
+		actual=${actual%% *} &&
+		test "x$actual" = "x-$y" || {
+			ls -lt .git/info
+			false
+		}
+	'
+
+	umask 077 &&
+	test_expect_success "shared = $u ($x) rw" '
+
+		rm -f .git/info/refs &&
+		git update-server-info &&
+		actual="$(ls -l .git/info/refs)" &&
+		actual=${actual%% *} &&
+		test "x$actual" = "x-$x" || {
+			ls -lt .git/info
+			false
+		}
+
+	'
+
+done
+
 test_done
-- 
1.5.4.4