{"thread":{"id":"11911","subject":"Re: [PATCH] Make tests independent of global config files","startedAt":"2008-02-06T08:38:17Z","lastAt":"2008-02-06T10:11:53Z","messageCount":8,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"67628","messageId":"20080206083817.GA19710@coredump.intra.peff.net","threadId":"11911","inReplyTo":null,"subject":"Re: [PATCH] Make tests independent of global config files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-06T08:38:17Z","receivedAt":"2008-02-06T08:38:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 15, 2007 at 11:43:56AM +0100, Johannes Schindelin wrote:\n\n> This was done by setting $HOME to somewhere bogus. A better method is\n> to reuse $GIT_CONFIG, which was invented for ignoring the global\n> config file explicitely.\n> \n> Technically, setting GIT_CONFIG=.git/config could be wrong, but it\n> passes all the tests, and we can keep the tests that way.\n\n[Yes, this is a reply to an ancient message, but your \"could be wrong\"\nturned out to be true.]\n\nI ran across a bug in the test suite today, and it is caused by this\nchange. The problem is that setting GIT_CONFIG=.git/config means that if\nwe change directories, then we may silently fail to read the config\nfile. Oops.\n\nIn particular, this is covering up an incorrect test in t1500. One of\nthe things it does is more or less this:\n\n  cd .git && git rev-parse --is-bare-repository\n\nand expects it to say \"true\". The test passes. All fine and dandy,\nexcept that it _isn't_ a bare repository, and if you run the test by\nhand, it returns \"false.\"\n\nThe problem is that it's looking for .git/.git/config and doesn't find\nit, meaning that it never sees the \"core.bare = false\" setting, and\nmakes a guess from its location inside the git dir.\n\nWe could set GIT_CONFIG=$(pwd)/.git/config, but then that fails if the\ntest creates other repos.\n\nWe could go back to setting HOME to something bogus, but then we have no\nway to suppress the reading of /etc/gitconfig (the only way to do so, I\nthink, is to set GIT_CONFIG).\n\nSo I think we are stuck adding in some environment magic to suppress the\nreading of ETC_GITCONFIG, and doing something like:\n\nunset GIT_CONFIG\nGIT_ETC_CONFIG=$(pwd)/.git/nonexistant\nGIT_LOCAL_CONFIG=$(pwd)/.git/nonexistant\n\nThoughts?\n\n-Peff\n"},{"id":"67631","messageId":"20080206090453.GA6762@coredump.intra.peff.net","threadId":"11911","inReplyTo":"20080206083817.GA19710@coredump.intra.peff.net","subject":"Re: [PATCH] Make tests independent of global config files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-06T09:04:53Z","receivedAt":"2008-02-06T09:04:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 06, 2008 at 03:38:17AM -0500, Jeff King wrote:\n\n> So I think we are stuck adding in some environment magic to suppress the\n> reading of ETC_GITCONFIG, and doing something like:\n> \n> unset GIT_CONFIG\n> GIT_ETC_CONFIG=$(pwd)/.git/nonexistant\n> GIT_LOCAL_CONFIG=$(pwd)/.git/nonexistant\n\nActually, we don't need to suppress GIT_LOCAL_CONFIG in the same way; we\njust need to make sure it isn't set. But we do need to set HOME to avoid\nthat lookup. Patch series will follow.\n\n-Peff\n"},{"id":"67632","messageId":"20080206090736.GA3258@coredump.intra.peff.net","threadId":"11911","inReplyTo":"20080206090453.GA6762@coredump.intra.peff.net","subject":"[PATCH 1/2] allow GIT_CONFIG_ETC to override compile-time setting","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-06T09:07:36Z","receivedAt":"2008-02-06T09:07:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is not expected to be used by mere mortals, since the\nGIT_CONFIG_LOCAL variable already accomplishes more or less\nthe same thing. However, to keep the testing environment\nclean, it is necessary to be able to suppress the use of\nthis file (which may have settings that interfere with the\ntests).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nJunio, more rationale for the series is available in the first message\nof the thread.\n\n cache.h  |    1 +\n config.c |    4 +++-\n 2 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 549f4bb..d8322a1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -208,6 +208,7 @@ static inline enum object_type object_type(unsigned int mode)\n #define TEMPLATE_DIR_ENVIRONMENT \"GIT_TEMPLATE_DIR\"\n #define CONFIG_ENVIRONMENT \"GIT_CONFIG\"\n #define CONFIG_LOCAL_ENVIRONMENT \"GIT_CONFIG_LOCAL\"\n+#define CONFIG_ETC_ENVIRONMENT \"GIT_CONFIG_ETC\"\n #define EXEC_PATH_ENVIRONMENT \"GIT_EXEC_PATH\"\n #define GITATTRIBUTES_FILE \".gitattributes\"\n #define INFOATTRIBUTES_FILE \"info/attributes\"\ndiff --git a/config.c b/config.c\nindex 498259e..2a0d005 100644\n--- a/config.c\n+++ b/config.c\n@@ -481,7 +481,9 @@ const char *git_etc_gitconfig(void)\n {\n \tstatic const char *system_wide;\n \tif (!system_wide) {\n-\t\tsystem_wide = ETC_GITCONFIG;\n+\t\tsystem_wide = getenv(CONFIG_ETC_ENVIRONMENT);\n+\t        if (!system_wide)\n+\t\t\tsystem_wide = ETC_GITCONFIG;\n \t\tif (!is_absolute_path(system_wide)) {\n \t\t\t/* interpret path relative to exec-dir */\n \t\t\tstruct strbuf d = STRBUF_INIT;\n-- \n1.5.4.26.g9be9\n"},{"id":"67633","messageId":"20080206090755.GB3258@coredump.intra.peff.net","threadId":"11911","inReplyTo":"20080206090453.GA6762@coredump.intra.peff.net","subject":"[PATCH 2/2] fix config reading in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-06T09:07:55Z","receivedAt":"2008-02-06T09:07:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Previously, we set the GIT_CONFIG environment variable in\nour tests so that only that file was read. However, setting\nit to a static value is not correct, since we are not\nnecessarily always in the same directory; instead, we want\nthe usual git config file lookup to happen.\n\nTo do this, we stop setting GIT_CONFIG, which means that we\nmust now suppress the reading of the system-wide and user\nconfigs.\n\nThis exposes an incorrect test in t1500, which is also\nfixed (the incorrect test worked because we were failing to\nread the core.bare value from the config file, since the\nGIT_CONFIG variable was pointing us to the wrong file).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t1500-rev-parse.sh |    4 ++--\n t/test-lib.sh        |    7 +++++--\n 2 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex e474b3f..38a2bf0 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -33,9 +33,9 @@ test_rev_parse() {\n test_rev_parse toplevel false false true ''\n \n cd .git || exit 1\n-test_rev_parse .git/ true true false ''\n+test_rev_parse .git/ false true false ''\n cd objects || exit 1\n-test_rev_parse .git/objects/ true true false ''\n+test_rev_parse .git/objects/ false true false ''\n cd ../.. || exit 1\n \n mkdir -p sub/dir || exit 1\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex da47bd7..40db00c 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -324,8 +324,11 @@ test_done () {\n PATH=$(pwd)/..:$PATH\n GIT_EXEC_PATH=$(pwd)/..\n GIT_TEMPLATE_DIR=$(pwd)/../templates/blt\n-GIT_CONFIG=.git/config\n-export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG\n+unset GIT_CONFIG\n+unset GIT_CONFIG_LOCAL\n+HOME=$(pwd)\n+GIT_CONFIG_ETC=$GIT_CONFIG_LOCAL\n+export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR HOME GIT_CONFIG_ETC\n \n GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git\n export GITPERLLIB\n-- \n1.5.4.26.g9be9\n"},{"id":"67635","messageId":"7vodau1m6t.fsf@gitster.siamese.dyndns.org","threadId":"11911","inReplyTo":"20080206083817.GA19710@coredump.intra.peff.net","subject":"Re: [PATCH] Make tests independent of global config files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-06T09:14:18Z","receivedAt":"2008-02-06T09:14:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> We could go back to setting HOME to something bogus, but then we have no\n> way to suppress the reading of /etc/gitconfig (the only way to do so, I\n> think, is to set GIT_CONFIG).\n>\n> So I think we are stuck adding in some environment magic to suppress the\n> reading of ETC_GITCONFIG, and doing something like:\n>\n> unset GIT_CONFIG\n> GIT_ETC_CONFIG=$(pwd)/.git/nonexistant\n> GIT_LOCAL_CONFIG=$(pwd)/.git/nonexistant\n>\n> Thoughts?\n\nInstead how about \"GIT_NO_ETC_CONFIG\" when set _ignores_ /etc/gitconfig\naltogether?\n\nThen you do not have to make excuses like \"eh, you can do this\nmore or less with local config but...\".\n"},{"id":"67637","messageId":"20080206092753.GA32264@coredump.intra.peff.net","threadId":"11911","inReplyTo":"7vodau1m6t.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Make tests independent of global config files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-06T09:27:54Z","receivedAt":"2008-02-06T09:27:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 06, 2008 at 01:14:18AM -0800, Junio C Hamano wrote:\n\n> > GIT_ETC_CONFIG=$(pwd)/.git/nonexistant\n> \n> Instead how about \"GIT_NO_ETC_CONFIG\" when set _ignores_ /etc/gitconfig\n> altogether?\n> \n> Then you do not have to make excuses like \"eh, you can do this\n> more or less with local config but...\".\n\nI'm fine with that, as well. And for our purposes, a little cleaner,\nsince we don't have to use the hack-ish \"nonexistent\" file (which is\nclearly a spelling nightmare).\n\nUpdated patch series to follow.\n\n-Peff\n"},{"id":"67645","messageId":"20080206101118.GA1123@coredump.intra.peff.net","threadId":"11911","inReplyTo":"20080206092753.GA32264@coredump.intra.peff.net","subject":"[PATCH 1/2] allow suppressing of global and system config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-06T10:11:18Z","receivedAt":"2008-02-06T10:11:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The GIT_CONFIG_NOGLOBAL and GIT_CONFIG_NOSYSTEM environment\nvariables are magic undocumented switches that can be used\nto ensure a totally clean environment. This is necessary for\nrunning reliable tests, since those config files may contain\nsettings that change the outcome of tests.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nYou can see that the config file lookup is more or less duplicated in\nconfig.c and in builtin-config.c. I tried to keep the patch minimal, but\nit's possible that those can be refactored into a single lookup point.\n\n builtin-config.c |    5 +++--\n cache.h          |    3 +++\n config.c         |   20 ++++++++++++++++++--\n 3 files changed, 24 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-config.c b/builtin-config.c\nindex e4a12e3..404bb44 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -79,9 +79,10 @@ static int get_value(const char* key_, const char* regex_)\n \t\tlocal = getenv(CONFIG_LOCAL_ENVIRONMENT);\n \t\tif (!local)\n \t\t\tlocal = repo_config = xstrdup(git_path(\"config\"));\n-\t\tif (home)\n+\t\tif (git_config_global() && home)\n \t\t\tglobal = xstrdup(mkpath(\"%s/.gitconfig\", home));\n-\t\tsystem_wide = git_etc_gitconfig();\n+\t\tif (git_config_system())\n+\t\t\tsystem_wide = git_etc_gitconfig();\n \t}\n \n \tkey = xstrdup(key_);\ndiff --git a/cache.h b/cache.h\nindex 549f4bb..af74e20 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -589,6 +589,9 @@ extern int git_config_set_multivar(const char *, const char *, const char *, int\n extern int git_config_rename_section(const char *, const char *);\n extern const char *git_etc_gitconfig(void);\n extern int check_repository_format_version(const char *var, const char *value);\n+extern int git_env_bool(const char *, int);\n+extern int git_config_system(void);\n+extern int git_config_global(void);\n \n #define MAX_GITNAME (1000)\n extern char git_default_email[MAX_GITNAME];\ndiff --git a/config.c b/config.c\nindex 498259e..2f85e9d 100644\n--- a/config.c\n+++ b/config.c\n@@ -492,6 +492,22 @@ const char *git_etc_gitconfig(void)\n \treturn system_wide;\n }\n \n+int git_env_bool(const char *k, int def)\n+{\n+\tconst char *v = getenv(k);\n+\treturn v ? git_config_bool(k, v) : def;\n+}\n+\n+int git_config_system(void)\n+{\n+\treturn !git_env_bool(\"GIT_CONFIG_NOSYSTEM\", 0);\n+}\n+\n+int git_config_global(void)\n+{\n+\treturn !git_env_bool(\"GIT_CONFIG_NOGLOBAL\", 0);\n+}\n+\n int git_config(config_fn_t fn)\n {\n \tint ret = 0;\n@@ -504,7 +520,7 @@ int git_config(config_fn_t fn)\n \t * config file otherwise. */\n \tfilename = getenv(CONFIG_ENVIRONMENT);\n \tif (!filename) {\n-\t\tif (!access(git_etc_gitconfig(), R_OK))\n+\t\tif (git_config_system() && !access(git_etc_gitconfig(), R_OK))\n \t\t\tret += git_config_from_file(fn, git_etc_gitconfig());\n \t\thome = getenv(\"HOME\");\n \t\tfilename = getenv(CONFIG_LOCAL_ENVIRONMENT);\n@@ -512,7 +528,7 @@ int git_config(config_fn_t fn)\n \t\t\tfilename = repo_config = xstrdup(git_path(\"config\"));\n \t}\n \n-\tif (home) {\n+\tif (git_config_global() && home) {\n \t\tchar *user_config = xstrdup(mkpath(\"%s/.gitconfig\", home));\n \t\tif (!access(user_config, R_OK))\n \t\t\tret = git_config_from_file(fn, user_config);\n-- \n1.5.4.25.g251c56-dirty\n"},{"id":"67646","messageId":"20080206101153.GB1123@coredump.intra.peff.net","threadId":"11911","inReplyTo":"20080206092753.GA32264@coredump.intra.peff.net","subject":"[PATCH 2/2] fix config reading in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-06T10:11:53Z","receivedAt":"2008-02-06T10:11:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Previously, we set the GIT_CONFIG environment variable in\nour tests so that only that file was read. However, setting\nit to a static value is not correct, since we are not\nnecessarily always in the same directory; instead, we want\nthe usual git config file lookup to happen.\n\nTo do this, we stop setting GIT_CONFIG, which means that we\nmust now suppress the reading of the system-wide and user\nconfigs.\n\nThis exposes an incorrect test in t1500, which is also\nfixed (the incorrect test worked because we were failing to\nread the core.bare value from the config file, since the\nGIT_CONFIG variable was pointing us to the wrong file).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t1500-rev-parse.sh |    4 ++--\n t/test-lib.sh        |    7 +++++--\n 2 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex e474b3f..38a2bf0 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -33,9 +33,9 @@ test_rev_parse() {\n test_rev_parse toplevel false false true ''\n \n cd .git || exit 1\n-test_rev_parse .git/ true true false ''\n+test_rev_parse .git/ false true false ''\n cd objects || exit 1\n-test_rev_parse .git/objects/ true true false ''\n+test_rev_parse .git/objects/ false true false ''\n cd ../.. || exit 1\n \n mkdir -p sub/dir || exit 1\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex da47bd7..83889c4 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -324,8 +324,11 @@ test_done () {\n PATH=$(pwd)/..:$PATH\n GIT_EXEC_PATH=$(pwd)/..\n GIT_TEMPLATE_DIR=$(pwd)/../templates/blt\n-GIT_CONFIG=.git/config\n-export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG\n+unset GIT_CONFIG\n+unset GIT_CONFIG_LOCAL\n+GIT_CONFIG_NOSYSTEM=1\n+GIT_CONFIG_NOGLOBAL=1\n+export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOBAL\n \n GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git\n export GITPERLLIB\n-- \n1.5.4.25.g251c56-dirty\n"}]}