Re: [PATCH] Make tests independent of global config files

Re: [PATCH] Make tests independent of global config files

[Jeff King]

On Thu, Feb 15, 2007 at 11:43:56AM +0100, Johannes Schindelin wrote:

> This was done by setting $HOME to somewhere bogus. A better method is
> to reuse $GIT_CONFIG, which was invented for ignoring the global
> config file explicitely.
> 
> Technically, setting GIT_CONFIG=.git/config could be wrong, but it
> passes all the tests, and we can keep the tests that way.

[Yes, this is a reply to an ancient message, but your "could be wrong"
turned out to be true.]

I ran across a bug in the test suite today, and it is caused by this
change. The problem is that setting GIT_CONFIG=.git/config means that if
we change directories, then we may silently fail to read the config
file. Oops.

In particular, this is covering up an incorrect test in t1500. One of
the things it does is more or less this:

  cd .git && git rev-parse --is-bare-repository

and expects it to say "true". The test passes. All fine and dandy,
except that it _isn't_ a bare repository, and if you run the test by
hand, it returns "false."

The problem is that it's looking for .git/.git/config and doesn't find
it, meaning that it never sees the "core.bare = false" setting, and
makes a guess from its location inside the git dir.

We could set GIT_CONFIG=$(pwd)/.git/config, but then that fails if the
test creates other repos.

We could go back to setting HOME to something bogus, but then we have no
way to suppress the reading of /etc/gitconfig (the only way to do so, I
think, is to set GIT_CONFIG).

So I think we are stuck adding in some environment magic to suppress the
reading of ETC_GITCONFIG, and doing something like:

unset GIT_CONFIG
GIT_ETC_CONFIG=$(pwd)/.git/nonexistant
GIT_LOCAL_CONFIG=$(pwd)/.git/nonexistant

Thoughts?

-Peff

Re: [PATCH] Make tests independent of global config files

[Jeff King]

On Wed, Feb 06, 2008 at 03:38:17AM -0500, Jeff King wrote:

> So I think we are stuck adding in some environment magic to suppress the
> reading of ETC_GITCONFIG, and doing something like:
> 
> unset GIT_CONFIG
> GIT_ETC_CONFIG=$(pwd)/.git/nonexistant
> GIT_LOCAL_CONFIG=$(pwd)/.git/nonexistant

Actually, we don't need to suppress GIT_LOCAL_CONFIG in the same way; we
just need to make sure it isn't set. But we do need to set HOME to avoid
that lookup. Patch series will follow.

-Peff

[PATCH 1/2] allow GIT_CONFIG_ETC to override compile-time setting

[Jeff King]

This is not expected to be used by mere mortals, since the
GIT_CONFIG_LOCAL variable already accomplishes more or less
the same thing. However, to keep the testing environment
clean, it is necessary to be able to suppress the use of
this file (which may have settings that interfere with the
tests).

Signed-off-by: Jeff King <peff@peff.net>
---
Junio, more rationale for the series is available in the first message
of the thread.

 cache.h  |    1 +
 config.c |    4 +++-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/cache.h b/cache.h
index 549f4bb..d8322a1 100644
--- a/cache.h
+++ b/cache.h
@@ -208,6 +208,7 @@ static inline enum object_type object_type(unsigned int mode)
 #define TEMPLATE_DIR_ENVIRONMENT "GIT_TEMPLATE_DIR"
 #define CONFIG_ENVIRONMENT "GIT_CONFIG"
 #define CONFIG_LOCAL_ENVIRONMENT "GIT_CONFIG_LOCAL"
+#define CONFIG_ETC_ENVIRONMENT "GIT_CONFIG_ETC"
 #define EXEC_PATH_ENVIRONMENT "GIT_EXEC_PATH"
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
diff --git a/config.c b/config.c
index 498259e..2a0d005 100644
--- a/config.c
+++ b/config.c
@@ -481,7 +481,9 @@ const char *git_etc_gitconfig(void)
 {
 	static const char *system_wide;
 	if (!system_wide) {
-		system_wide = ETC_GITCONFIG;
+		system_wide = getenv(CONFIG_ETC_ENVIRONMENT);
+	        if (!system_wide)
+			system_wide = ETC_GITCONFIG;
 		if (!is_absolute_path(system_wide)) {
 			/* interpret path relative to exec-dir */
 			struct strbuf d = STRBUF_INIT;
-- 
1.5.4.26.g9be9

[PATCH 2/2] fix config reading in tests

[Jeff King]

Previously, we set the GIT_CONFIG environment variable in
our tests so that only that file was read. However, setting
it to a static value is not correct, since we are not
necessarily always in the same directory; instead, we want
the usual git config file lookup to happen.

To do this, we stop setting GIT_CONFIG, which means that we
must now suppress the reading of the system-wide and user
configs.

This exposes an incorrect test in t1500, which is also
fixed (the incorrect test worked because we were failing to
read the core.bare value from the config file, since the
GIT_CONFIG variable was pointing us to the wrong file).

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t1500-rev-parse.sh |    4 ++--
 t/test-lib.sh        |    7 +++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index e474b3f..38a2bf0 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -33,9 +33,9 @@ test_rev_parse() {
 test_rev_parse toplevel false false true ''
 
 cd .git || exit 1
-test_rev_parse .git/ true true false ''
+test_rev_parse .git/ false true false ''
 cd objects || exit 1
-test_rev_parse .git/objects/ true true false ''
+test_rev_parse .git/objects/ false true false ''
 cd ../.. || exit 1
 
 mkdir -p sub/dir || exit 1
diff --git a/t/test-lib.sh b/t/test-lib.sh
index da47bd7..40db00c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -324,8 +324,11 @@ test_done () {
 PATH=$(pwd)/..:$PATH
 GIT_EXEC_PATH=$(pwd)/..
 GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
-GIT_CONFIG=.git/config
-export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG
+unset GIT_CONFIG
+unset GIT_CONFIG_LOCAL
+HOME=$(pwd)
+GIT_CONFIG_ETC=$GIT_CONFIG_LOCAL
+export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR HOME GIT_CONFIG_ETC
 
 GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
 export GITPERLLIB
-- 
1.5.4.26.g9be9

Re: [PATCH] Make tests independent of global config files

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> We could go back to setting HOME to something bogus, but then we have no
> way to suppress the reading of /etc/gitconfig (the only way to do so, I
> think, is to set GIT_CONFIG).
>
> So I think we are stuck adding in some environment magic to suppress the
> reading of ETC_GITCONFIG, and doing something like:
>
> unset GIT_CONFIG
> GIT_ETC_CONFIG=$(pwd)/.git/nonexistant
> GIT_LOCAL_CONFIG=$(pwd)/.git/nonexistant
>
> Thoughts?

Instead how about "GIT_NO_ETC_CONFIG" when set _ignores_ /etc/gitconfig
altogether?

Then you do not have to make excuses like "eh, you can do this
more or less with local config but...".

Re: [PATCH] Make tests independent of global config files

[Jeff King]

On Wed, Feb 06, 2008 at 01:14:18AM -0800, Junio C Hamano wrote:

> > GIT_ETC_CONFIG=$(pwd)/.git/nonexistant
> 
> Instead how about "GIT_NO_ETC_CONFIG" when set _ignores_ /etc/gitconfig
> altogether?
> 
> Then you do not have to make excuses like "eh, you can do this
> more or less with local config but...".

I'm fine with that, as well. And for our purposes, a little cleaner,
since we don't have to use the hack-ish "nonexistent" file (which is
clearly a spelling nightmare).

Updated patch series to follow.

-Peff

[PATCH 1/2] allow suppressing of global and system config

[Jeff King]

The GIT_CONFIG_NOGLOBAL and GIT_CONFIG_NOSYSTEM environment
variables are magic undocumented switches that can be used
to ensure a totally clean environment. This is necessary for
running reliable tests, since those config files may contain
settings that change the outcome of tests.

Signed-off-by: Jeff King <peff@peff.net>
---
You can see that the config file lookup is more or less duplicated in
config.c and in builtin-config.c. I tried to keep the patch minimal, but
it's possible that those can be refactored into a single lookup point.

 builtin-config.c |    5 +++--
 cache.h          |    3 +++
 config.c         |   20 ++++++++++++++++++--
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/builtin-config.c b/builtin-config.c
index e4a12e3..404bb44 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -79,9 +79,10 @@ static int get_value(const char* key_, const char* regex_)
 		local = getenv(CONFIG_LOCAL_ENVIRONMENT);
 		if (!local)
 			local = repo_config = xstrdup(git_path("config"));
-		if (home)
+		if (git_config_global() && home)
 			global = xstrdup(mkpath("%s/.gitconfig", home));
-		system_wide = git_etc_gitconfig();
+		if (git_config_system())
+			system_wide = git_etc_gitconfig();
 	}
 
 	key = xstrdup(key_);
diff --git a/cache.h b/cache.h
index 549f4bb..af74e20 100644
--- a/cache.h
+++ b/cache.h
@@ -589,6 +589,9 @@ extern int git_config_set_multivar(const char *, const char *, const char *, int
 extern int git_config_rename_section(const char *, const char *);
 extern const char *git_etc_gitconfig(void);
 extern int check_repository_format_version(const char *var, const char *value);
+extern int git_env_bool(const char *, int);
+extern int git_config_system(void);
+extern int git_config_global(void);
 
 #define MAX_GITNAME (1000)
 extern char git_default_email[MAX_GITNAME];
diff --git a/config.c b/config.c
index 498259e..2f85e9d 100644
--- a/config.c
+++ b/config.c
@@ -492,6 +492,22 @@ const char *git_etc_gitconfig(void)
 	return system_wide;
 }
 
+int git_env_bool(const char *k, int def)
+{
+	const char *v = getenv(k);
+	return v ? git_config_bool(k, v) : def;
+}
+
+int git_config_system(void)
+{
+	return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
+}
+
+int git_config_global(void)
+{
+	return !git_env_bool("GIT_CONFIG_NOGLOBAL", 0);
+}
+
 int git_config(config_fn_t fn)
 {
 	int ret = 0;
@@ -504,7 +520,7 @@ int git_config(config_fn_t fn)
 	 * config file otherwise. */
 	filename = getenv(CONFIG_ENVIRONMENT);
 	if (!filename) {
-		if (!access(git_etc_gitconfig(), R_OK))
+		if (git_config_system() && !access(git_etc_gitconfig(), R_OK))
 			ret += git_config_from_file(fn, git_etc_gitconfig());
 		home = getenv("HOME");
 		filename = getenv(CONFIG_LOCAL_ENVIRONMENT);
@@ -512,7 +528,7 @@ int git_config(config_fn_t fn)
 			filename = repo_config = xstrdup(git_path("config"));
 	}
 
-	if (home) {
+	if (git_config_global() && home) {
 		char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
 		if (!access(user_config, R_OK))
 			ret = git_config_from_file(fn, user_config);
-- 
1.5.4.25.g251c56-dirty

[PATCH 2/2] fix config reading in tests

[Jeff King]

Previously, we set the GIT_CONFIG environment variable in
our tests so that only that file was read. However, setting
it to a static value is not correct, since we are not
necessarily always in the same directory; instead, we want
the usual git config file lookup to happen.

To do this, we stop setting GIT_CONFIG, which means that we
must now suppress the reading of the system-wide and user
configs.

This exposes an incorrect test in t1500, which is also
fixed (the incorrect test worked because we were failing to
read the core.bare value from the config file, since the
GIT_CONFIG variable was pointing us to the wrong file).

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t1500-rev-parse.sh |    4 ++--
 t/test-lib.sh        |    7 +++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index e474b3f..38a2bf0 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -33,9 +33,9 @@ test_rev_parse() {
 test_rev_parse toplevel false false true ''
 
 cd .git || exit 1
-test_rev_parse .git/ true true false ''
+test_rev_parse .git/ false true false ''
 cd objects || exit 1
-test_rev_parse .git/objects/ true true false ''
+test_rev_parse .git/objects/ false true false ''
 cd ../.. || exit 1
 
 mkdir -p sub/dir || exit 1
diff --git a/t/test-lib.sh b/t/test-lib.sh
index da47bd7..83889c4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -324,8 +324,11 @@ test_done () {
 PATH=$(pwd)/..:$PATH
 GIT_EXEC_PATH=$(pwd)/..
 GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
-GIT_CONFIG=.git/config
-export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG
+unset GIT_CONFIG
+unset GIT_CONFIG_LOCAL
+GIT_CONFIG_NOSYSTEM=1
+GIT_CONFIG_NOGLOBAL=1
+export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOBAL
 
 GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
 export GITPERLLIB
-- 
1.5.4.25.g251c56-dirty