config: only read .git/config from configured repos
authorJeff King <peff@peff.net>
Tue, 13 Sep 2016 03:24:15 +0000 (20:24 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 13 Sep 2016 22:45:45 +0000 (15:45 -0700)
When git_config() runs, it looks in the system, user-wide,
and repo-level config files. It gets the latter by calling
git_pathdup(), which in turn calls get_git_dir(). If we
haven't set up the git repository yet, this may simply
return ".git", and we will look at ".git/config". This
seems like it would be helpful (presumably we haven't set up
the repository yet, so it tries to find it), but it turns
out to be a bad idea for a few reasons:

- it's not sufficient, and therefore hides bugs in a
confusing way. Config will be respected if commands are
run from the top-level of the working tree, but not from
a subdirectory.

- it's not always true that we haven't set up the
repository _yet_; we may not want to do it at all. For
instance, if you run "git init /some/path" from inside
another repository, it should not load config from the
existing repository.

- there might be a path ".git/config", but it is not the
actual repository we would find via setup_git_directory().
This may happen, e.g., if you are storing a git
repository inside another git repository, but have
munged one of the files in such a way that the
inner repository is not valid (e.g., by removing HEAD).

We have at least two bugs of the second type in git-init,
introduced by ae5f677 (lazily load core.sharedrepository,
2016-03-11). It causes init to use git_configset(), which
loads all of the config, including values from the current
repo (if any). This shows up in two ways:

1. If we happen to be in an existing repository directory,
we'll read and respect core.sharedrepository from it,
even though it should have no bearing on the new
repository. A new test in t1301 covers this.

2. Similarly, if we're in an existing repo that sets
core.logallrefupdates, that will cause init to fail to
set it in a newly created repository (because it thinks
that the user's templates already did so). A new test
in t0001 covers this.

We also need to adjust an existing test in t1302, which
gives another example of why this patch is an improvement.

That test creates an embedded repository with a bogus
core.repositoryformatversion of "99". It wants to make sure
that we actually stop at the bogus repo rather than
continuing upward to find the outer repo. So it checks that
"git config core.repositoryformatversion" returns 99. But
that only works because we blindly read ".git/config", even
though we _know_ we're in a repository whose vintage we do
not understand.

After this patch, we avoid reading config from the unknown
vintage repository at all, which is a safer choice. But we
need to tweak the test, since core.repositoryformatversion
will not return 99; it will claim that it could not find the
variable at all.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
cache.h
config.c
environment.c
t/t0001-init.sh
t/t1301-shared-repo.sh
t/t1302-repo-version.sh
diff --git a/cache.h b/cache.h
index 5e50887d63d2cd3883eab8cdea12ad5ee8e19c4d..67ec356c7a787e6316d2e53138582ea78c366a72 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -453,6 +453,12 @@ static inline enum object_type object_type(unsigned int mode)
  */
 extern const char * const local_repo_env[];
 
+/*
+ * Returns true iff we have a configured git repository (either via
+ * setup_git_directory, or in the environment via $GIT_DIR).
+ */
+int have_git_dir(void);
+
 extern int is_bare_repository_cfg;
 extern int is_bare_repository(void);
 extern int is_inside_git_dir(void);
index 1d2331aa085891cb3fe344e030a0fec5cbe62272..5e50fb1a31a5b4e0bb3c4cf517023c447c2f925f 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1194,7 +1194,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
        int ret = 0, found = 0;
        char *xdg_config = xdg_config_home("config");
        char *user_config = expand_user_path("~/.gitconfig");
-       char *repo_config = git_pathdup("config");
+       char *repo_config = have_git_dir() ? git_pathdup("config") : NULL;
 
        if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0)) {
                ret += git_config_from_file(fn, git_etc_gitconfig(),
index bccdee9d0b2942763807e69311a730cc8e23d66c..20a870c3d72101824e452ca68017e5150bd9de87 100644 (file)
@@ -194,6 +194,13 @@ int is_bare_repository(void)
        return is_bare_repository_cfg && !get_git_work_tree();
 }
 
+int have_git_dir(void)
+{
+       return startup_info->have_repository
+               || git_dir
+               || getenv(GIT_DIR_ENVIRONMENT);
+}
+
 const char *get_git_dir(void)
 {
        if (!git_dir)
index a6fdd5ef3a66f06f5b4a787ca8ad8db2ab4da96f..8ffbbea4d65dbd8328563da53ef2959dd39ce796 100755 (executable)
@@ -384,4 +384,13 @@ test_expect_success MINGW 'bare git dir not hidden' '
        ! is_hidden newdir
 '
 
+test_expect_success 'remote init from does not use config from cwd' '
+       rm -rf newdir &&
+       test_config core.logallrefupdates true &&
+       git init newdir &&
+       echo true >expect &&
+       git -C newdir config --bool core.logallrefupdates >actual &&
+       test_cmp expect actual
+'
+
 test_done
index ac108754088d34758a75f14943516c4a701f0ca8..7c28642f2e76046f25a36722e12fe87a5e4cea06 100755 (executable)
@@ -172,4 +172,13 @@ test_expect_success POSIXPERM 'forced modes' '
        }" actual)"
 '
 
+test_expect_success POSIXPERM 'remote init does not use config from cwd' '
+       git config core.sharedrepository 0666 &&
+       umask 0022 &&
+       git init --bare child.git &&
+       echo "-rw-r--r--" >expect &&
+       modebits child.git/config >actual &&
+       test_cmp expect actual
+'
+
 test_done
index f859809b3613dc8fa67f1acb1f8127ee9bbd4391..ce4cff13bbced58add641a463321f36673121fd7 100755 (executable)
@@ -32,9 +32,7 @@ test_expect_success 'gitdir selection on normal repos' '
 
 test_expect_success 'gitdir selection on unsupported repo' '
        # Make sure it would stop at test2, not trash
-       echo 99 >expect &&
-       git -C test2 config core.repositoryformatversion >actual &&
-       test_cmp expect actual
+       test_expect_code 1 git -C test2 config core.repositoryformatversion >actual
 '
 
 test_expect_success 'gitdir not required mode' '