Re: [PATCH v2] git init: --bare/--shared overrides system/global config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



If core.bare or core.sharedRepository are set in /etc/gitconfig or
~/.gitconfig, then 'git init' will read the values when constructing a
new config file; reading them, however, will override the values
specified on the command line.  In the case of --bare, this ends up
causing a segfault, without the repository being properly initialised;
in the case of --shared, the permissions are set according to the
existing config settings, not what was specified on the command line.

This fix saves any specified values for --bare and --shared prior to
reading existing config settings, and restores them after reading but
before writing the new config file.  core.bare is ignored in all
situations, while core.sharedRepository will only be used if --shared
is not specified to git init.

Also includes testcases which use a specified global config file
override, demonstrating the former failure scenario.

Signed-off-by: Deskin Miller <deskinm@xxxxxxxxx>
---
On Mon, Oct 06, 2008 at 07:14:52AM -0700, Shawn O. Pearce wrote:
> Deskin Miller <deskinm@xxxxxxxxx> wrote:
> > From b6144562983703079a1eba8cdac3506c18d751a3 Mon Sep 17 00:00:00 2001
> > From: Deskin Miller <deskinm@xxxxxxxxx>
> > Date: Sat, 4 Oct 2008 20:07:44 -0400
> 
> FWIW please don't include these lines in the commit message part
> of the patch email. [...]

Alright, sorry for the messiness; I guess I thought preserving the original
commit date was important?  Won't happen again.

> > diff --git a/builtin-init-db.c b/builtin-init-db.c
> > index 8140c12..38e282c 100644
> > --- a/builtin-init-db.c
> > +++ b/builtin-init-db.c
> > @@ -191,6 +194,8 @@ static int create_default_files(const char *template_path)
> >  	copy_templates(template_path);
> >  
> >  	git_config(git_default_config, NULL);
> > +	is_bare_repository_cfg = init_is_bare_repository;
> > +	shared_repository = init_shared_repository;
> 
> Is this really the right thing to do?  It seems like it would prevent
> a user from setting core.sharedRepository = group in their template
> and thus always have a shared repository on their system.
 
You're right.  Fixed in this version: core.bare ignores any config, while
--shared will override config settings, but init will use the config settings
if --shared is not specified.
 
> A second related test would be a ~/.gitconfig which sets
> core.sharedRepository = 0666 and then does "git init".  I think
> the right outcome is a repository which has that set.

Good suggestion, I added such a case in this version.  My first version fails
this new testcase, while maint fails the original testcase I wrote.

 builtin-init-db.c |   12 ++++++++++--
 t/t0001-init.sh   |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/builtin-init-db.c b/builtin-init-db.c
index 8140c12..d30c3fe 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -17,6 +17,9 @@
 #define TEST_FILEMODE 1
 #endif
 
+static int init_is_bare_repository = 0;
+static int init_shared_repository = -1;
+
 static void safe_create_dir(const char *dir, int share)
 {
 	if (mkdir(dir, 0777) < 0) {
@@ -191,6 +194,9 @@ static int create_default_files(const char *template_path)
 	copy_templates(template_path);
 
 	git_config(git_default_config, NULL);
+	is_bare_repository_cfg = init_is_bare_repository;
+	if (init_shared_repository != -1)
+		shared_repository = init_shared_repository;
 
 	/*
 	 * We would have created the above under user's umask -- under
@@ -277,6 +283,8 @@ int init_db(const char *template_dir, unsigned int flags)
 
 	safe_create_dir(get_git_dir(), 0);
 
+	init_is_bare_repository = is_bare_repository();
+
 	/* Check to see if the repository version is right.
 	 * Note that a newly created repository does not have
 	 * config file, so this will not fail.  What we are catching
@@ -381,9 +389,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
 						sizeof(git_dir)), 0);
 		} else if (!strcmp(arg, "--shared"))
-			shared_repository = PERM_GROUP;
+			init_shared_repository = PERM_GROUP;
 		else if (!prefixcmp(arg, "--shared="))
-			shared_repository = git_config_perm("arg", arg+9);
+			init_shared_repository = git_config_perm("arg", arg+9);
 		else if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet"))
 			flags |= INIT_DB_QUIET;
 		else
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 620da5b..5ac0a27 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -167,4 +167,36 @@ test_expect_success 'init with --template (blank)' '
 	! test -f template-blank/.git/info/exclude
 '
 
+test_expect_success 'init --bare/--shared overrides system/global config' '
+	(
+		HOME="`pwd`" &&
+		export HOME &&
+		test_config="$HOME"/.gitconfig &&
+		unset GIT_CONFIG_NOGLOBAL &&
+		git config -f "$test_config" core.bare false &&
+		git config -f "$test_config" core.sharedRepository 0640 &&
+		mkdir init-bare-shared-override &&
+		cd init-bare-shared-override &&
+		git init --bare --shared=0666
+	) &&
+	check_config init-bare-shared-override true unset &&
+	test x0666 = \
+	x`git config -f init-bare-shared-override/config core.sharedRepository`
+'
+
+test_expect_success 'init honors global core.sharedRepository' '
+	(
+		HOME="`pwd`" &&
+		export HOME &&
+		test_config="$HOME"/.gitconfig &&
+		unset GIT_CONFIG_NOGLOBAL &&
+		git config -f "$test_config" core.sharedRepository 0666 &&
+		mkdir shared-honor-global &&
+		cd shared-honor-global &&
+		git init
+	) &&
+	test x0666 = \
+	x`git config -f shared-honor-global/.git/config core.sharedRepository`
+'
+
 test_done
-- 
1.6.0.2.307.gc427

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux