# [PATCH] Introduce core.sharedrepository 7 messages from 2005-12-22 to 2005-12-23. Participants: Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/2911 ## Johannes Schindelin, 2005-12-22 22:13 Subject: [PATCH] Introduce core.sharedrepository Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512222313070.12044%40wbgn013.biozentrum.uni-wuerzburg.de ``` 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 --- 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 ``` ## Junio C Hamano, 2005-12-22 22:40 Subject: Re: [PATCH] Introduce core.sharedrepository Message-ID: <7vaceshi18.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vaceshi18.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin 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? ``` ## Johannes Schindelin, 2005-12-22 22:59 Subject: Re: [PATCH] Introduce core.sharedrepository Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512222356540.12681%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7vaceshi18.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Thu, 22 Dec 2005, Junio C Hamano wrote: > Johannes Schindelin 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 ``` ## Johannes Schindelin, 2005-12-22 23:04 Subject: Re: [PATCH] Introduce core.sharedrepository Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512230003560.12795%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: ``` 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 ``` ## Junio C Hamano, 2005-12-23 09:10 Subject: Re: [PATCH] Introduce core.sharedrepository Message-ID: <7vbqz89o0y.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vbqz89o0y.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin 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 ;-). ``` ## Johannes Schindelin, 2005-12-23 10:42 Subject: Re: [PATCH] Introduce core.sharedrepository Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512231141410.28090%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7vbqz89o0y.fsf@assigned-by-dhcp.cox.net> ``` 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 ``` ## Johannes Schindelin, 2005-12-23 11:07 Subject: Re: [PATCH] Introduce core.sharedrepository Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512231206590.18905%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: ``` 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 ```