{"thread":{"id":"14221","subject":"[PATCH] Only use GIT_CONFIG in \"git config\", not other programs","startedAt":"2008-06-30T07:37:47Z","lastAt":"2008-07-15T21:46:50Z","messageCount":5,"participants":["Daniel Barkalow","Junio C Hamano","Sverre Rabbelier"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"81726","messageId":"alpine.LNX.1.00.0806300328380.19665@iabervon.org","threadId":"14221","inReplyTo":null,"subject":"[PATCH] Only use GIT_CONFIG in \"git config\", not other programs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-06-30T07:37:47Z","receivedAt":"2008-06-30T07:37:47Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"For everything other than using \"git config\" to read or write a\ngit-style config file that isn't the current repo's config file,\nGIT_CONFIG was actively detrimental. Rather than argue over which\nprograms are important enough to have work anyway, just fix all of\nthem at the root.\n\nAlso removes GIT_LOCAL_CONFIG, which would only be useful for programs\nthat do want to use global git-specific config, but not the repo's own\ngit-specific config, and want to use some other, presumably\ngit-specific config. Despite being documented, I can't find any sign that \nit was ever used.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\nThis passes all of the tests, including the git-svn ones. It seems that, \nin fact, GIT_CONFIG was only used to control \"git config\" in scripts and \ntests are protected with GIT_CONFIG_NOSYSTEM and GIT_CONFIG_NOGLOBAL.\n\nAlong with this, my clone patch makes most sense, because the environment \nvariable won't do anything anyway, so we don't have to change it at all.\n\n Documentation/git-config.txt |    9 --------\n builtin-config.c             |   15 ++++++-------\n cache.h                      |    2 +-\n config.c                     |   47 +++++++++++++++++------------------------\n 4 files changed, 28 insertions(+), 45 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex c90421e..30c8432 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -191,11 +191,6 @@ variables. The '--global' and the '--system' options will limit the file used\n to the global or system-wide file respectively. The GIT_CONFIG environment\n variable has a similar effect, but you can specify any filename you want.\n \n-The GIT_CONFIG_LOCAL environment variable on the other hand only changes\n-the name used instead of the repository configuration file. The global and\n-the system-wide configuration files will still be read. (For writing options\n-this will obviously result in the same behavior as using GIT_CONFIG.)\n-\n \n ENVIRONMENT\n -----------\n@@ -205,10 +200,6 @@ GIT_CONFIG::\n \tUsing the \"--global\" option forces this to ~/.gitconfig. Using the\n \t\"--system\" option forces this to $(prefix)/etc/gitconfig.\n \n-GIT_CONFIG_LOCAL::\n-\tTake the configuration from the given file instead if .git/config.\n-\tStill read the global and the system-wide configuration files, though.\n-\n See also <<FILES>>.\n \n \ndiff --git a/builtin-config.c b/builtin-config.c\nindex 3a441ef..39f63d7 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -81,12 +81,10 @@ static int get_value(const char* key_, const char* regex_)\n \tchar *global = NULL, *repo_config = NULL;\n \tconst char *system_wide = NULL, *local;\n \n-\tlocal = getenv(CONFIG_ENVIRONMENT);\n+\tlocal = config_exclusive_filename;\n \tif (!local) {\n \t\tconst char *home = getenv(\"HOME\");\n-\t\tlocal = getenv(CONFIG_LOCAL_ENVIRONMENT);\n-\t\tif (!local)\n-\t\t\tlocal = repo_config = xstrdup(git_path(\"config\"));\n+\t\tlocal = repo_config = xstrdup(git_path(\"config\"));\n \t\tif (git_config_global() && home)\n \t\t\tglobal = xstrdup(mkpath(\"%s/.gitconfig\", home));\n \t\tif (git_config_system())\n@@ -289,6 +287,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \tchar* value;\n \tconst char *file = setup_git_directory_gently(&nongit);\n \n+\tconfig_exclusive_filename = getenv(CONFIG_ENVIRONMENT);\n+\n \twhile (1 < argc) {\n \t\tif (!strcmp(argv[1], \"--int\"))\n \t\t\ttype = T_INT;\n@@ -309,14 +309,13 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\tchar *home = getenv(\"HOME\");\n \t\t\tif (home) {\n \t\t\t\tchar *user_config = xstrdup(mkpath(\"%s/.gitconfig\", home));\n-\t\t\t\tsetenv(CONFIG_ENVIRONMENT, user_config, 1);\n-\t\t\t\tfree(user_config);\n+\t\t\t\tconfig_exclusive_filename = user_config;\n \t\t\t} else {\n \t\t\t\tdie(\"$HOME not set\");\n \t\t\t}\n \t\t}\n \t\telse if (!strcmp(argv[1], \"--system\"))\n-\t\t\tsetenv(CONFIG_ENVIRONMENT, git_etc_gitconfig(), 1);\n+\t\t\tconfig_exclusive_filename = git_etc_gitconfig();\n \t\telse if (!strcmp(argv[1], \"--file\") || !strcmp(argv[1], \"-f\")) {\n \t\t\tif (argc < 3)\n \t\t\t\tusage(git_config_set_usage);\n@@ -325,7 +324,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\t       argv[2]);\n \t\t\telse\n \t\t\t\tfile = argv[2];\n-\t\t\tsetenv(CONFIG_ENVIRONMENT, file, 1);\n+\t\t\tconfig_exclusive_filename = file;\n \t\t\targc--;\n \t\t\targv++;\n \t\t}\ndiff --git a/cache.h b/cache.h\nindex 64ef86e..bab0115 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -298,7 +298,6 @@ static inline enum object_type object_type(unsigned int mode)\n #define GRAFT_ENVIRONMENT \"GIT_GRAFT_FILE\"\n #define TEMPLATE_DIR_ENVIRONMENT \"GIT_TEMPLATE_DIR\"\n #define CONFIG_ENVIRONMENT \"GIT_CONFIG\"\n-#define CONFIG_LOCAL_ENVIRONMENT \"GIT_CONFIG_LOCAL\"\n #define EXEC_PATH_ENVIRONMENT \"GIT_EXEC_PATH\"\n #define GITATTRIBUTES_FILE \".gitattributes\"\n #define INFOATTRIBUTES_FILE \"info/attributes\"\n@@ -743,6 +742,7 @@ extern int check_repository_format_version(const char *var, const char *value, v\n extern int git_config_system(void);\n extern int git_config_global(void);\n extern int config_error_nonbool(const char *);\n+extern const char *config_exclusive_filename;\n \n #define MAX_GITNAME (1000)\n extern char git_default_email[MAX_GITNAME];\ndiff --git a/config.c b/config.c\nindex 58749bf..2862cc4 100644\n--- a/config.c\n+++ b/config.c\n@@ -16,6 +16,8 @@ static int config_linenr;\n static int config_file_eof;\n static int zlib_compression_seen;\n \n+const char *config_exclusive_filename = NULL;\n+\n static int get_next_char(void)\n {\n \tint c;\n@@ -611,31 +613,28 @@ int git_config(config_fn_t fn, void *data)\n {\n \tint ret = 0;\n \tchar *repo_config = NULL;\n-\tconst char *home = NULL, *filename;\n+\tconst char *home = NULL;\n \n \t/* $GIT_CONFIG makes git read _only_ the given config file,\n \t * $GIT_CONFIG_LOCAL will make it process it in addition to the\n \t * global config file, the same way it would the per-repository\n \t * config file otherwise. */\n-\tfilename = getenv(CONFIG_ENVIRONMENT);\n-\tif (!filename) {\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\t\t\tdata);\n-\t\thome = getenv(\"HOME\");\n-\t\tfilename = getenv(CONFIG_LOCAL_ENVIRONMENT);\n-\t\tif (!filename)\n-\t\t\tfilename = repo_config = xstrdup(git_path(\"config\"));\n-\t}\n+\tif (config_exclusive_filename)\n+\t\treturn git_config_from_file(fn, config_exclusive_filename, data);\n+\tif (git_config_system() && !access(git_etc_gitconfig(), R_OK))\n+\t\tret += git_config_from_file(fn, git_etc_gitconfig(),\n+\t\t\t\t\t    data);\n \n+\thome = getenv(\"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, data);\n+\t\t\tret += git_config_from_file(fn, user_config, data);\n \t\tfree(user_config);\n \t}\n \n-\tret += git_config_from_file(fn, filename, data);\n+\trepo_config = xstrdup(git_path(\"config\"));\n+\tret += git_config_from_file(fn, repo_config, data);\n \tfree(repo_config);\n \treturn ret;\n }\n@@ -873,13 +872,10 @@ int git_config_set_multivar(const char* key, const char* value,\n \tstruct lock_file *lock = NULL;\n \tconst char* last_dot = strrchr(key, '.');\n \n-\tconfig_filename = getenv(CONFIG_ENVIRONMENT);\n-\tif (!config_filename) {\n-\t\tconfig_filename = getenv(CONFIG_LOCAL_ENVIRONMENT);\n-\t\tif (!config_filename)\n-\t\t\tconfig_filename  = git_path(\"config\");\n-\t}\n-\tconfig_filename = xstrdup(config_filename);\n+\tif (config_exclusive_filename)\n+\t\tconfig_filename = xstrdup(config_exclusive_filename);\n+\telse\n+\t\tconfig_filename = xstrdup(git_path(\"config\"));\n \n \t/*\n \t * Since \"key\" actually contains the section name and the real\n@@ -1136,13 +1132,10 @@ int git_config_rename_section(const char *old_name, const char *new_name)\n \tint out_fd;\n \tchar buf[1024];\n \n-\tconfig_filename = getenv(CONFIG_ENVIRONMENT);\n-\tif (!config_filename) {\n-\t\tconfig_filename = getenv(CONFIG_LOCAL_ENVIRONMENT);\n-\t\tif (!config_filename)\n-\t\t\tconfig_filename  = git_path(\"config\");\n-\t}\n-\tconfig_filename = xstrdup(config_filename);\n+\tif (config_exclusive_filename)\n+\t\tconfig_filename = xstrdup(config_exclusive_filename);\n+\telse\n+\t\tconfig_filename = xstrdup(git_path(\"config\"));\n \tout_fd = hold_lock_file_for_update(lock, config_filename, 0);\n \tif (out_fd < 0) {\n \t\tret = error(\"could not lock config file %s\", config_filename);\n-- \n1.5.6.1.103.g1c643\n"},{"id":"81729","messageId":"7vskuv1hj1.fsf@gitster.siamese.dyndns.org","threadId":"14221","inReplyTo":"alpine.LNX.1.00.0806300328380.19665@iabervon.org","subject":"Re: [PATCH] Only use GIT_CONFIG in \"git config\", not other programs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-30T07:57:54Z","receivedAt":"2008-06-30T07:57:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hmm, I actually think this is a good thing to do in the sense that it\nvastly simplifies the user experience ;-).  Two less things to explain.\n\nCare to write up a snippet for 1.6.0 release notes, as this is quite a\nlarge user visible backward incompatible change, even though it would be\nan improvement?\n"},{"id":"81763","messageId":"alpine.LNX.1.00.0806301247110.19665@iabervon.org","threadId":"14221","inReplyTo":"7vskuv1hj1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Only use GIT_CONFIG in \"git config\", not other programs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-06-30T17:28:03Z","receivedAt":"2008-06-30T17:28:03Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 30 Jun 2008, Junio C Hamano wrote:\n\n> Hmm, I actually think this is a good thing to do in the sense that it\n> vastly simplifies the user experience ;-).  Two less things to explain.\n> \n> Care to write up a snippet for 1.6.0 release notes, as this is quite a\n> large user visible backward incompatible change, even though it would be\n> an improvement?\n\nSure:\n\n* GIT_CONFIG, which was only documented as affecting \"git config\", but \n  actually affected all git commands, now only affects \"git config\". \n  GIT_LOCAL_CONFIG, also only documented as affecting \"git config\" and not \n  different from GIT_CONFIG in a useful way, is removed.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"83420","messageId":"bd6139dc0807151056q61a9379l84761fe021d4541c@mail.gmail.com","threadId":"14221","inReplyTo":"alpine.LNX.1.00.0806300328380.19665@iabervon.org","subject":"Re: [PATCH] Only use GIT_CONFIG in \"git config\", not other programs","fromName":"Sverre Rabbelier","fromEmail":"alturin@gmail.com","sentAt":"2008-07-15T17:56:01Z","receivedAt":"2008-07-15T17:56:01Z","isPatch":true,"sender":{"key":"alturin@gmail.com","avatar":null},"body":"On Mon, Jun 30, 2008 at 9:37 AM, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> @@ -611,31 +613,28 @@ int git_config(config_fn_t fn, void *data)\n>  {\n>        int ret = 0;\n>        char *repo_config = NULL;\n> -       const char *home = NULL, *filename;\n> +       const char *home = NULL;\n>\n>        /* $GIT_CONFIG makes git read _only_ the given config file,\n>         * $GIT_CONFIG_LOCAL will make it process it in addition to the\n>         * global config file, the same way it would the per-repository\n>         * config file otherwise. */\n\nI noticed today while looking at config.c (because of the 'git config\noddity' thread) that this reference to GIT_CONFIG_LOCAL was not\nremoved? On latest next:\n$ grep GIT_CONFIG_LOCAL *\nconfig.c:\t * $GIT_CONFIG_LOCAL will make it process it in addition to the\ngit-svn:\t\tmy $file = $ENV{GIT_CONFIG} || $ENV{GIT_CONFIG_LOCAL} ||\ngit-svn.perl:\t\tmy $file = $ENV{GIT_CONFIG} || $ENV{GIT_CONFIG_LOCAL} ||\n\nI doubt that this stray comment will hurt anyone, but it's probably\nnicer not to let cruft like this accumulate, I think?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"83434","messageId":"alpine.LNX.1.00.0807151733430.19665@iabervon.org","threadId":"14221","inReplyTo":"bd6139dc0807151056q61a9379l84761fe021d4541c@mail.gmail.com","subject":"Re: [PATCH] Only use GIT_CONFIG in \"git config\", not other programs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-07-15T21:46:50Z","receivedAt":"2008-07-15T21:46:50Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 15 Jul 2008, Sverre Rabbelier wrote:\n\n> On Mon, Jun 30, 2008 at 9:37 AM, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> > @@ -611,31 +613,28 @@ int git_config(config_fn_t fn, void *data)\n> >  {\n> >        int ret = 0;\n> >        char *repo_config = NULL;\n> > -       const char *home = NULL, *filename;\n> > +       const char *home = NULL;\n> >\n> >        /* $GIT_CONFIG makes git read _only_ the given config file,\n> >         * $GIT_CONFIG_LOCAL will make it process it in addition to the\n> >         * global config file, the same way it would the per-repository\n> >         * config file otherwise. */\n> \n> I noticed today while looking at config.c (because of the 'git config\n> oddity' thread) that this reference to GIT_CONFIG_LOCAL was not\n> removed?\n\nActually, it looks like that whole comment is obsolete; only \nbuiltin-config.c cares about any of this stuff.\n\n> On latest next:\n> $ grep GIT_CONFIG_LOCAL *\n> config.c:\t * $GIT_CONFIG_LOCAL will make it process it in addition to the\n> git-svn:\t\tmy $file = $ENV{GIT_CONFIG} || $ENV{GIT_CONFIG_LOCAL} ||\n> git-svn.perl:\t\tmy $file = $ENV{GIT_CONFIG} || $ENV{GIT_CONFIG_LOCAL} ||\n\nI'm not sure if git-svn is doing the right thing here, either.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}