[PATCH] Introduce core.sharedrepository

[PATCH] Introduce core.sharedrepository

[Johannes Schindelin]

This is the second attempt, after Junio convinced me that my simple approach
to set umask was misguided.

If the config variable 'core.sharedrepository' is set, the directories

	$GIT_DIR/objects/
	$GIT_DIR/objects/??
	$GIT_DIR/objects/pack
	$GIT_DIR/refs
	$GIT_DIR/refs/heads
	$GIT_DIR/refs/heads/tags

are set group writable (and g+s, since the git group may be not the primary
group of all users).

Since all files are written as lock files first,
and then moved to their destination, they do not have to be group writable.
Indeed, if this leads to problems you found a bug.

Note that -- as in my first attempt -- the config variable is set in the
function which checks the repository format. If this were done in
git_default_config instead, a lot of programs would need to be modified
to call git_config(git_default_config) first.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 cache.h     |    1 +
 setup.c     |    4 ++++
 sha1_file.c |   21 +++++++++++++++++++++
 3 files changed, 26 insertions(+), 0 deletions(-)

1a164f16e90378ba66e70e6082266645d3b45c57
diff --git a/cache.h b/cache.h
index cb87bec..0f875dd 100644
--- a/cache.h
+++ b/cache.h
@@ -159,6 +159,7 @@ extern void rollback_index_file(struct c
 extern int trust_executable_bit;
 extern int only_use_symrefs;
 extern int diff_rename_limit_default;
+extern int shared_repository;
 
 #define GIT_REPO_VERSION 0
 extern int repository_format_version;
diff --git a/setup.c b/setup.c
index d3556ed..3de372e 100644
--- a/setup.c
+++ b/setup.c
@@ -1,5 +1,7 @@
 #include "cache.h"
 
+int shared_repository = 0;
+
 const char *prefix_path(const char *prefix, int len, const char *path)
 {
 	const char *orig = path;
@@ -180,6 +182,8 @@ int check_repository_format_version(cons
 {
        if (strcmp(var, "core.repositoryformatversion") == 0)
                repository_format_version = git_config_int(var, value);
+	else if (strcmp(var, "core.sharedrepository") == 0)
+		shared_repository = git_config_bool(var, value);
        return 0;
 }
 
diff --git a/sha1_file.c b/sha1_file.c
index d451a94..e109a07 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -48,6 +48,21 @@ int get_sha1_hex(const char *hex, unsign
 	return 0;
 }
 
+static int make_group_writable(const char *path)
+{
+	struct stat st;
+
+	if (lstat(path, &st) < 0)
+		return -1;
+	if (st.st_mode & S_IWUSR)
+		st.st_mode |= S_IWGRP;
+	if (S_ISDIR(st.st_mode))
+		st.st_mode |= S_ISGID;
+	if (chmod(path, st.st_mode) < 0)
+		return -2;
+	return 0;
+}
+
 int safe_create_leading_directories(char *path)
 {
 	char *pos = path;
@@ -64,6 +79,10 @@ int safe_create_leading_directories(char
 				*pos = '/';
 				return -1;
 			}
+		if (shared_repository && make_group_writable(path)) {
+			*pos = '/';
+			return -2;
+		}
 		*pos++ = '/';
 	}
 	return 0;
@@ -1241,6 +1260,8 @@ static int link_temp_to_file(const char 
 		if (dir) {
 			*dir = 0;
 			mkdir(filename, 0777);
+			if (shared_repository && make_group_writable(filename))
+				return -2;
 			*dir = '/';
 			if (!link(tmpfile, filename))
 				return 0;
-- 
1.0.GIT

Re: [PATCH] Introduce core.sharedrepository

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> +static int make_group_writable(const char *path)
> +{
> +	struct stat st;
> +
> +	if (lstat(path, &st) < 0)
> +		return -1;
> +	if (st.st_mode & S_IWUSR)
> +		st.st_mode |= S_IWGRP;
> +	if (S_ISDIR(st.st_mode))
> +		st.st_mode |= S_ISGID;
> +	if (chmod(path, st.st_mode) < 0)
> +		return -2;
> +	return 0;
> +}

Perhaps g+sx not just g+s on directories.

> +
>  int safe_create_leading_directories(char *path)
>  {
>  	char *pos = path;
> @@ -64,6 +79,10 @@ int safe_create_leading_directories(char
>  				*pos = '/';
>  				return -1;
>  			}
> +		if (shared_repository && make_group_writable(path)) {
> +			*pos = '/';
> +			return -2;
> +		}

Wouldn't this safe_create_leading_directories("/pub/git.git/objects")
try chmod "/pub" and fail?

Re: [PATCH] Introduce core.sharedrepository

[Johannes Schindelin]

Hi,

On Thu, 22 Dec 2005, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +static int make_group_writable(const char *path)
> > +{
> > +	struct stat st;
> > +
> > +	if (lstat(path, &st) < 0)
> > +		return -1;
> > +	if (st.st_mode & S_IWUSR)
> > +		st.st_mode |= S_IWGRP;
> > +	if (S_ISDIR(st.st_mode))
> > +		st.st_mode |= S_ISGID;
> > +	if (chmod(path, st.st_mode) < 0)
> > +		return -2;
> > +	return 0;
> > +}
> 
> Perhaps g+sx not just g+s on directories.

Okay:

        if (S_ISDIR(st.st_mode))
                st.st_mode |= S_ISGID | S_IXGRP;

> > +
> >  int safe_create_leading_directories(char *path)
> >  {
> >  	char *pos = path;
> > @@ -64,6 +79,10 @@ int safe_create_leading_directories(char
> >  				*pos = '/';
> >  				return -1;
> >  			}
> > +		if (shared_repository && make_group_writable(path)) {
> > +			*pos = '/';
> > +			return -2;
> > +		}
> 
> Wouldn't this safe_create_leading_directories("/pub/git.git/objects")
> try chmod "/pub" and fail?

Yes.

How about an "else" in front of the "if"? Like so:

                if (mkdir(path, 0777) < 0)
                        if (errno != EEXIST) {
                                *pos = '/';
                                return -1;
                        }
                else if (shared_repository && make_group_writable(path)) {
                        *pos = '/';
                        return -2;
                }

You want me to resend the patch?

Ciao,
Dscho

Re: [PATCH] Introduce core.sharedrepository

[Johannes Schindelin]

Hi,

On Thu, 22 Dec 2005, Johannes Schindelin wrote:

>                 if (mkdir(path, 0777) < 0)
>                         if (errno != EEXIST) {
>                                 *pos = '/';
>                                 return -1;
>                         }
>                 else if (shared_repository && make_group_writable(path)) {
>                         *pos = '/';
>                         return -2;
>                 }

D'oh! Time to head to bed now:

                if (mkdir(path, 0777) < 0) {
                        if (errno != EEXIST) {
                                *pos = '/';
                                return -1;
                        }
                } else if (shared_repository && make_group_writable(path)) {
                        *pos = '/';
                        return -2;
                }

Ciao,
Dscho

Re: [PATCH] Introduce core.sharedrepository

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> D'oh! Time to head to bed now:

Likewise.

I've merged up your patches yesterday after some churning and
they are in the pu branch.  I hope I did not break them too much
;-).

Re: [PATCH] Introduce core.sharedrepository

[Johannes Schindelin]

Hi,

On Fri, 23 Dec 2005, Junio C Hamano wrote:

> I've merged up your patches yesterday after some churning and
> they are in the pu branch.  I hope I did not break them too much
> ;-).

Looks good. Will test.

Ciao,
Dscho

Re: [PATCH] Introduce core.sharedrepository

[Johannes Schindelin]

Hi,

On Fri, 23 Dec 2005, Johannes Schindelin wrote:

> Hi,
> 
> On Fri, 23 Dec 2005, Junio C Hamano wrote:
> 
> > I've merged up your patches yesterday after some churning and
> > they are in the pu branch.  I hope I did not break them too much
> > ;-).
> 
> Looks good. Will test.

Tested. Looks even better.

Ciao,
Dscho