git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dennis Kaarsemaker <dennis@kaarsemaker.net>
To: Jeff King <peff@peff.net>
Cc: doak <doak@gmx.de>, "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: bug: 'core.logallrefupdates' is not set by default in non-bare repository
Date: Wed, 31 Aug 2016 17:32:33 +0200	[thread overview]
Message-ID: <1472657553.4265.63.camel@kaarsemaker.net> (raw)
In-Reply-To: <20160831104825.quyqb54bo5k7fdxs@sigill.intra.peff.net>

On wo, 2016-08-31 at 06:48 -0400, Jeff King wrote:
> On Wed, Aug 31, 2016 at 11:12:26AM +0200, Dennis Kaarsemaker wrote:
> 
> > 
> > That is indeed a bug. git reads the config of t1 and then thinks a
> > template config has set that value, so it won't override it.
> This is a regression in v2.9.0 due to my ae5f677 (lazily load
> core.sharedrepository, 2016-03-11).
> 
> > 
> > This is caused by git init reading the config via
> > get_shared_repository. The comment above it indicates that this may
> > not
> > be needed, and indeed not doing it makes this bug go away.
> Hrm. I'm not sure if that will work, though, because we may call
> get_shared_repository() from other code-paths (e.g., anything that
> calls
> adjust_shared_perm(), like safe_create_leading_directories).

Agreed, the diff was more to point out what triggered this (so I could
send that and run away to spend the day with jr.) than an attempt at a
patch.

> We may need to do something like turn off the
> need_shared_repository_from_config in init-db, since I think it would
> not want to ever read from the default config sources in most of its
> code-paths (OTOH, it should in theory respect core.sharedRepository
> in ~/.gitconfig, so maybe there is another more elegant way of
> handling this).

I would go even further and say that git init should completely ignore
the config of a repository you happen to be in when creating a new
repository.

> I'm out of time for the day, so it will be a while before I can dig
> further. Please feel free to figure it out while I am sleeping. :)
> 
> -Peff

I hope you slept well :)

This is what I came up with to implement my suggestion above, comments
welcome.

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 3a45f0b..d0fd3dc 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -493,6 +493,12 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 		int mkdir_tried = 0;
 	retry:
 		if (chdir(argv[0]) < 0) {
+			/*
+			 * We're creating a new repository. If we're already in another
+			 * repository, ignore its config
+			 */
+			ignore_repo_config = 1;
+			git_config_clear();
 			if (!mkdir_tried) {
 				int saved;
 				/*
@@ -500,7 +506,6 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 				 * and we know shared_repository should always be 0;
 				 * but just in case we play safe.
 				 */
-				saved = get_shared_repository();
 				set_shared_repository(0);
 				switch (safe_create_leading_directories_const(argv[0])) {
 				case SCLD_OK:
@@ -513,7 +518,6 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 					die_errno(_("cannot mkdir %s"), argv[0]);
 					break;
 				}
-				set_shared_repository(saved);
 				if (mkdir(argv[0], 0777) < 0)
 					die_errno(_("cannot mkdir %s"), argv[0]);
 				mkdir_tried = 1;
@@ -524,6 +528,11 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	} else if (0 < argc) {
 		usage(init_db_usage[0]);
 	}
+
+	need_shared_repository_from_config = 1;
+	ignore_repo_config = 0;
+	git_config_clear();
+
 	if (is_bare_repository_cfg == 1) {
 		char *cwd = xgetcwd();
 		setenv(GIT_DIR_ENVIRONMENT, cwd, argc > 0);
diff --git a/cache.h b/cache.h
index f30a441..1be16fc 100644
--- a/cache.h
+++ b/cache.h
@@ -640,6 +640,8 @@ extern int hold_locked_index(struct lock_file *, int);
 extern void set_alternate_index_output(const char *);
 
 /* Environment bits from configuration mechanism */
+extern int ignore_repo_config;
+extern int need_shared_repository_from_config;
 extern int trust_executable_bit;
 extern int trust_ctime;
 extern int check_stat;
diff --git a/config.c b/config.c
index 0dfed68..2df0189 100644
--- a/config.c
+++ b/config.c
@@ -1304,7 +1304,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
 		ret += git_config_from_file(fn, user_config, data);
 
 	current_parsing_scope = CONFIG_SCOPE_REPO;
-	if (repo_config && !access_or_die(repo_config, R_OK, 0))
+	if (repo_config && !ignore_repo_config && !access_or_die(repo_config, R_OK, 0))
 		ret += git_config_from_file(fn, repo_config, data);
 
 	current_parsing_scope = CONFIG_SCOPE_CMDLINE;
diff --git a/environment.c b/environment.c
index ca72464..f6dbba8 100644
--- a/environment.c
+++ b/environment.c
@@ -12,6 +12,8 @@
 #include "fmt-merge-msg.h"
 #include "commit.h"
 
+int ignore_repo_config = 0;
+int need_shared_repository_from_config = 1;
 int trust_executable_bit = 1;
 int trust_ctime = 1;
 int check_stat = 1;
@@ -326,7 +328,6 @@ const char *get_commit_output_encoding(void)
 }
 
 static int the_shared_repository = PERM_UMASK;
-static int need_shared_repository_from_config = 1;
 
 void set_shared_repository(int value)
 {

  reply	other threads:[~2016-08-31 15:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-31  8:09 bug: 'core.logallrefupdates' is not set by default in non-bare repository doak
2016-08-31  9:12 ` Dennis Kaarsemaker
2016-08-31 10:48   ` Jeff King
2016-08-31 15:32     ` Dennis Kaarsemaker [this message]
2016-09-02  8:04       ` Jeff King
2016-09-02  8:47         ` Jeff King
2016-09-02  9:01         ` Dennis Kaarsemaker
2016-09-02  9:11           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1472657553.4265.63.camel@kaarsemaker.net \
    --to=dennis@kaarsemaker.net \
    --cc=doak@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).