{"thread":{"id":"55455","subject":"[PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","startedAt":"2021-04-08T14:18:20Z","lastAt":"2021-04-23T09:32:24Z","messageCount":52,"participants":["Patrick Steinhardt","Eric Sunshine","Junio C Hamano","Jeff King","Ævar Arnfjörð Bjarmason","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"421218","messageId":"a23382059bb57022dd1e40d1c2c9a11307b0ff3b.1617891426.git.ps@pks.im","threadId":"55455","inReplyTo":null,"subject":"[PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-08T14:17:23Z","receivedAt":"2021-04-08T14:18:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While it's already possible to stop git from reading the system config\nvia GIT_CONFIG_NOSYSTEM, doing the same for global config files requires\nthe user to unset both HOME and XDG_CONFIG_HOME. This is an awkward\ninterface and may even pose a problem e.g. when git hooks rely on these\nvariables to be present.\n\nIntroduce a new GIT_CONFIG_NOGLOBAL envvar, which is the simple\nequivalent to GIT_CONFIG_NOSYSTEM. If set to true, git will skip reading\nboth `~/.gitconfig` and `$XDG_CONFIG_HOME/git/config`.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/git-config.txt | 4 ++++\n Documentation/git.txt | 16 ++++++++++++----\n config.c | 9 +++++++--\n t/t1300-config.sh | 31 +++++++++++++++++++++++++++++++\n 4 files changed, 54 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 4b4cc5c5e8..88cd064abb 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -340,6 +340,10 @@ 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_NOGLOBAL::\n+\tWhether to skip reading settings from the global ~/.gitconfig and\n+\t$XDG_CONFIG_HOME/git/config files. See linkgit:git[1] for details.\n+\n GIT_CONFIG_NOSYSTEM::\n \tWhether to skip reading settings from the system-wide\n \t$(prefix)/etc/gitconfig file. See linkgit:git[1] for details.\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 3a9c44987f..4462bd2da9 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -670,13 +670,21 @@ for further details.\n \tIf this environment variable is set to `0`, git will not prompt\n \ton the terminal (e.g., when asking for HTTP authentication).\n \n+`GIT_CONFIG_NOGLOBAL`::\n+\tWhether to skip reading settings from the system-wide `~/.gitconfig`\n+\tand `$XDG_CONFIG_HOME/git/config` files. This environment variable can\n+\tbe used along with `$GIT_CONFIG_NOSYSTEM` to create a predictable\n+\tenvironment for a picky script, or you can set it temporarily to avoid\n+\tusing a buggy global config file while waiting for someone with\n+\tsufficient permissions to fix it.\n+\n `GIT_CONFIG_NOSYSTEM`::\n \tWhether to skip reading settings from the system-wide\n \t`$(prefix)/etc/gitconfig` file. This environment variable can\n-\tbe used along with `$HOME` and `$XDG_CONFIG_HOME` to create a\n-\tpredictable environment for a picky script, or you can set it\n-\ttemporarily to avoid using a buggy `/etc/gitconfig` file while\n-\twaiting for someone with sufficient permissions to fix it.\n+\tbe used along with `$GIT_CONFIG_NOGLOBAL` to create a predictable\n+\tenvironment for a picky script, or you can set it temporarily to avoid\n+\tusing a buggy `/etc/gitconfig` file while waiting for someone with\n+\tsufficient permissions to fix it.\n \n `GIT_FLUSH`::\n \tIf this environment variable is set to \"1\", then commands such\ndiff --git a/config.c b/config.c\nindex 6428393a41..19c1b31c75 100644\n--- a/config.c\n+++ b/config.c\n@@ -1879,6 +1879,11 @@ int git_config_system(void)\n \treturn !git_env_bool(\"GIT_CONFIG_NOSYSTEM\", 0);\n }\n \n+static int git_config_global(void)\n+{\n+\treturn !git_env_bool(\"GIT_CONFIG_NOGLOBAL\", 0);\n+}\n+\n static int do_git_config_sequence(const struct config_options *opts,\n \t\t\t\t config_fn_t fn, void *data)\n {\n@@ -1903,10 +1908,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\t\t\t\t data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n-\tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n+\tif (git_config_global() && xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n \t\tret += git_config_from_file(fn, xdg_config, data);\n \n-\tif (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))\n+\tif (git_config_global() && user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))\n \t\tret += git_config_from_file(fn, user_config, data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_LOCAL;\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex e0dd5d65ce..0754189974 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -2059,6 +2059,37 @@ test_expect_success '--show-scope with --show-origin' '\n \ttest_cmp expect output\n '\n \n+test_expect_success 'GIT_CONFIG_NOGLOBAL' '\n+\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n+\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n+\t[home]\n+\t\tconfig = true\n+\tEOF\n+\tmkdir -p \"$HOME\"/.config/git &&\n+\tcat >\"$HOME\"/.config/git/config <<-EOF &&\n+\t[xdg]\n+\t\tconfig = true\n+\tEOF\n+\tcat >.git/config <<-EOF &&\n+\t[local]\n+\t\tconfig = true\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\tglobal\txdg.config=true\n+\tglobal\thome.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tcat >expect <<-EOF &&\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_NOGLOBAL=true git config --show-scope --list >output &&\n+\ttest_cmp expect output\n+'\n+\n for opt in --local --worktree\n do\n \ttest_expect_success \"$opt requires a repo\" '\n-- \n2.31.1\n\n"},{"id":"421242","messageId":"CAPig+cT=xbftpV+k941ikkZR0RvQsXL6XA0Oono2_sDHsb5TTA@mail.gmail.com","threadId":"55455","inReplyTo":"a23382059bb57022dd1e40d1c2c9a11307b0ff3b.1617891426.git.ps@pks.im","subject":"Re: [PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-04-08T16:44:41Z","receivedAt":"2021-04-08T16:44:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 8, 2021 at 10:18 AM Patrick Steinhardt wrote:\n> While it's already possible to stop git from reading the system config\n> via GIT_CONFIG_NOSYSTEM, doing the same for global config files requires\n> the user to unset both HOME and XDG_CONFIG_HOME. This is an awkward\n> interface and may even pose a problem e.g. when git hooks rely on these\n> variables to be present.\n>\n> Introduce a new GIT_CONFIG_NOGLOBAL envvar, which is the simple\n> equivalent to GIT_CONFIG_NOSYSTEM. If set to true, git will skip reading\n> both `~/.gitconfig` and `$XDG_CONFIG_HOME/git/config`.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> @@ -670,13 +670,21 @@ for further details.\n> +`GIT_CONFIG_NOGLOBAL`::\n> + Whether to skip reading settings from the system-wide `~/.gitconfig`\n> + and `$XDG_CONFIG_HOME/git/config` files. This environment variable can\n> + be used along with `$GIT_CONFIG_NOSYSTEM` to create a predictable\n> + environment for a picky script, or you can set it temporarily to avoid\n> + using a buggy global config file while waiting for someone with\n> + sufficient permissions to fix it.\n\nNot necessarily a new problem since you mostly copied the new text\nfrom GIT_CONFIG_NOSYSTEM, but this doesn't tell the reader what value\nto assign to this variable. As currently written, I would end up\nhaving to consult the source code to figure out how to use this\nvariable, which makes the documentation less useful than it should be.\nPerhaps it could be rewritten something like:\n\n If set to any value, suppress reading global configuration\n from `~/.gitconfig` and `$XDG_CONFIG_HOME/git/config`\n files. This environment variable...\n\nThe bit about waiting for someone to fix a buggy ~/.gitconfig is\nsomewhat questionable; it might make sense to drop everything after\n\"picky script\".\n\n> `GIT_CONFIG_NOSYSTEM`::\n> Whether to skip reading settings from the system-wide\n> `$(prefix)/etc/gitconfig` file. This environment variable can\n> - be used along with `$HOME` and `$XDG_CONFIG_HOME` to create a\n> - predictable environment for a picky script, or you can set it\n> - temporarily to avoid using a buggy `/etc/gitconfig` file while\n> - waiting for someone with sufficient permissions to fix it.\n> + be used along with `$GIT_CONFIG_NOGLOBAL` to create a predictable\n> + environment for a picky script, or you can set it temporarily to avoid\n> + using a buggy `/etc/gitconfig` file while waiting for someone with\n> + sufficient permissions to fix it.\n\nThis suffers the same problem of not telling the reader what value to\nassign. A similar rewrite could improve it, as well.\n"},{"id":"421244","messageId":"xmqqczv4vgck.fsf@gitster.g","threadId":"55455","inReplyTo":"a23382059bb57022dd1e40d1c2c9a11307b0ff3b.1617891426.git.ps@pks.im","subject":"Re: [PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-08T17:25:15Z","receivedAt":"2021-04-08T17:25:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> While it's already possible to stop git from reading the system config\n> via GIT_CONFIG_NOSYSTEM, doing the same for global config files requires\n> the user to unset both HOME and XDG_CONFIG_HOME. This is an awkward\n> interface and may even pose a problem e.g. when git hooks rely on these\n> variables to be present.\n\nYeah, having to unset HOME would affect not just Git. Git may call\nout something else (e.g. an editor) that may want to know where HOME\nis, Git may be used in something else (e.g. an IDE) that may want to\nknow where HOME is. Same for XDG_CONFIG_HOME. If it is a valid need\nto not reading from $HOME/.gitconfig, unsetting HOME is an unacceptable\napproach to satisfying that need.\n\n> Introduce a new GIT_CONFIG_NOGLOBAL envvar, which is the simple\n> equivalent to GIT_CONFIG_NOSYSTEM. If set to true, git will skip reading\n> both `~/.gitconfig` and `$XDG_CONFIG_HOME/git/config`.\n\nI do not think we'd add an unbounded number of new configuration\nsources to the existing three-level hierarchy of system-wide\n(/etc/gitconfig), per-user ($HOME/.gitconfig), and local\n(.git/config), so it is not too bad, from scalability point of view,\nto keep adding new GIT_CONFIG_NO$FROTZ variables.\n\nLet me go in a couple of different tangents a bit, thinking aloud.\n\nTangent One. I wonder if the presense of includeIf..path\nchanges the \"we won't have unbounded, so adding another here is OK\"\nreasoning. If somebody wanted to say \"Do not use the paths that\nmatch this and that pattern\", it is likely that we'd end up having\nto support a single variable that allows more than one \"value\". In\na straw-man syntax \"GIT_CONFIG_EXCLUDE='/home/gitster/*\n/home/peff/*'\" might be a way to say \"do not use files that are\nunder these two directories.\n\nAnd when that happens, we'd probably notice that it is easier for\nusers to configure, if they can treat 'system' and 'global' as just\nanother special pattern to place on that list. //system and //global\nmight be the syntax we'd choose when time comes, i.e.\n\n\tGIT_CONFIG_EXCLUDE='//system //global'\n\nmight become a more scalable replacement for\n\n\tGIT_CONFIG_NOSYSTEM=yes GIT_CONFIG_NOHOME=yes\n\nTangent Two. One thing we never managed to properly test in our\ntest suite is the unctioning of GIT_CONFIG_NOSYSTEM. As we do not\nwant to get broken by whatever is in /etc/gitconfig, all our tests\nrun with the environment variable set. For the same reason, in\norder to avoid getting influenced by whatever the tester has in\n$HOME/.gitconfig, we export HOME set to the throw-away directory\ncreated during the test and control what is in the config file in\nthat directory. In hindsight, it might have been a better design to\nuse GIT_CONFIG_SYSTEM variable that points at a path to be used as a\nreplacement for /etc/gitconfig when it is set; pointing /dev/null\nwith the variable would have been the natural way to say \"do not use\nanything from system configuration\". And GIT_CONFIG_GLOBAL can\neasily follow the same pattern.\n\nSo, from these two perspective, for the longer term end-user\nexperience, I am not 100% onboard with GIT_CONFIG_NOGLOBAL. An\nalternative that allows us to move away from GIT_CONFIG_NOSYSTEM in\nthe direction to the tangent #2 would be not to repeat the same\nmistake by doing GIT_CONFIG_NOGLOBAL, and instead adding\nGIT_CONFIG_GLOBAL, which is\n\n (1) when not set, it does not do anything,\n\n (2) when set to \"/dev/null\" (a literal string; you do not have to\n spell it \"NUL\" when on Windows), it acts just like the case\n where your GIT_CONFIG_NOSYSTEM is set,\n\n (3) when set to any other string, it is taken as a filename that\n has the global configuration. Unlike $HOME/.gitconfig or\n $XDG_HOME/git/config, it is an error if the named file does not\n exist (this is to catch misconfiguration).\n\nAnd once this is accepted by users and established as a pattern, we\ncould migrate GIT_CONFIG_NOSYSTEM to GIT_CONFIG_SYSTEM=/dev/null\n\n\nHaving said all that (meaning: I am not 100% onboard with _NOGLOBAL\nand think _GLOBAL=/dev/null might be a better design), let's give it\na review under the assumption that _NOGLOBAL is the design we would\nwant to choose.\n\n> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\n> index 4b4cc5c5e8..88cd064abb 100644\n> --- a/Documentation/git-config.txt\n> +++ b/Documentation/git-config.txt\n> @@ -340,6 +340,10 @@ 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_NOGLOBAL::\n> +\tWhether to skip reading settings from the global ~/.gitconfig and\n> +\t$XDG_CONFIG_HOME/git/config files. See linkgit:git[1] for details.\n> +\n> GIT_CONFIG_NOSYSTEM::\n> \tWhether to skip reading settings from the system-wide\n> \t$(prefix)/etc/gitconfig file. See linkgit:git[1] for details.\n\nOK. The new one mimics existing _NOSYSTEM and there is nothing\nsurprising in the new description.\n\n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 3a9c44987f..4462bd2da9 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -670,13 +670,21 @@ for further details.\n> \tIf this environment variable is set to `0`, git will not prompt\n> \ton the terminal (e.g., when asking for HTTP authentication).\n> \n> +`GIT_CONFIG_NOGLOBAL`::\n> +\tWhether to skip reading settings from the system-wide `~/.gitconfig`\n> +\tand `$XDG_CONFIG_HOME/git/config` files. This environment variable can\n> +\tbe used along with `$GIT_CONFIG_NOSYSTEM` to create a predictable\n> +\tenvironment for a picky script, or you can set it temporarily to avoid\n> +\tusing a buggy global config file while waiting for someone with\n> +\tsufficient permissions to fix it.\n\n\"while waiting for someone with permissions\" is a good backstory for\ninventing _NOSYSTEM because you might not be able to futz with\n/etc/gitconfig, but does not sound like an appropriate reasoning for\n_NOGLOBAL that is about $HOME/.gitconfig or $HOME/.config/git/config.\n\n> `GIT_CONFIG_NOSYSTEM`::\n> \tWhether to skip reading settings from the system-wide\n> \t`$(prefix)/etc/gitconfig` file. This environment variable can\n> -\tbe used along with `$HOME` and `$XDG_CONFIG_HOME` to create a\n> -\tpredictable environment for a picky script, or you can set it\n> -\ttemporarily to avoid using a buggy `/etc/gitconfig` file while\n> -\twaiting for someone with sufficient permissions to fix it.\n> +\tbe used along with `$GIT_CONFIG_NOGLOBAL` to create a predictable\n> +\tenvironment for a picky script, or you can set it temporarily to avoid\n> +\tusing a buggy `/etc/gitconfig` file while waiting for someone with\n> +\tsufficient permissions to fix it.\n\n> diff --git a/config.c b/config.c\n> index 6428393a41..19c1b31c75 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1879,6 +1879,11 @@ int git_config_system(void)\n> \treturn !git_env_bool(\"GIT_CONFIG_NOSYSTEM\", 0);\n> }\n> \n> +static int git_config_global(void)\n> +{\n> +\treturn !git_env_bool(\"GIT_CONFIG_NOGLOBAL\", 0);\n> +}\n> +\n> static int do_git_config_sequence(const struct config_options *opts,\n> \t\t\t\t config_fn_t fn, void *data)\n> {\n> @@ -1903,10 +1908,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n> \t\t\t\t\t data);\n> \n> \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n> -\tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n> +\tif (git_config_global() && xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n> \t\tret += git_config_from_file(fn, xdg_config, data);\n> \n> -\tif (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))\n> +\tif (git_config_global() && user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))\n> \t\tret += git_config_from_file(fn, user_config, data);\n> \n> \tcurrent_parsing_scope = CONFIG_SCOPE_LOCAL;\n\nThe implementation itself is quite straightforward for _NOSYSTEM; I\nsee nothing wrong in the code change for the given design.\n\n> diff --git a/t/t1300-config.sh b/t/t1300-config.sh\n> index e0dd5d65ce..0754189974 100755\n> --- a/t/t1300-config.sh\n> +++ b/t/t1300-config.sh\n> @@ -2059,6 +2059,37 @@ test_expect_success '--show-scope with --show-origin' '\n> \ttest_cmp expect output\n> '\n> \n> +test_expect_success 'GIT_CONFIG_NOGLOBAL' '\n> +\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n> +\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n> +\t[home]\n> +\t\tconfig = true\n> +\tEOF\n> +\tmkdir -p \"$HOME\"/.config/git &&\n> +\tcat >\"$HOME\"/.config/git/config <<-EOF &&\n> +\t[xdg]\n> +\t\tconfig = true\n> +\tEOF\n> +\tcat >.git/config <<-EOF &&\n> +\t[local]\n> +\t\tconfig = true\n> +\tEOF\n> +\n> +\tcat >expect <<-EOF &&\n> +\tglobal\txdg.config=true\n> +\tglobal\thome.config=true\n> +\tlocal\tlocal.config=true\n> +\tEOF\n> +\tgit config --show-scope --list >output &&\n> +\ttest_cmp expect output &&\n> +\n> +\tcat >expect <<-EOF &&\n> +\tlocal\tlocal.config=true\n> +\tEOF\n> +\tGIT_CONFIG_NOGLOBAL=true git config --show-scope --list >output &&\n> +\ttest_cmp expect output\n> +'\n\nThis test was what initially piqued my curiosity, as the fact that\nthe result filtered with the new mechanism has only 'local' misled\nme to incorrectly think that we are suppressing both 'system' and\n'global' with the single variable. Until I realized that we cannot\ntest the 'system' configuration in our test suite, that is.\n\nThanks.\n"},{"id":"421296","messageId":"YG+PLROZbSo32F3k@coredump.intra.peff.net","threadId":"55455","inReplyTo":"xmqqczv4vgck.fsf@gitster.g","subject":"Re: [PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-08T23:18:05Z","receivedAt":"2021-04-08T23:18:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 08, 2021 at 10:25:15AM -0700, Junio C Hamano wrote:\n\n> Patrick Steinhardt writes:\n> \n> > While it's already possible to stop git from reading the system config\n> > via GIT_CONFIG_NOSYSTEM, doing the same for global config files requires\n> > the user to unset both HOME and XDG_CONFIG_HOME. This is an awkward\n> > interface and may even pose a problem e.g. when git hooks rely on these\n> > variables to be present.\n> \n> Yeah, having to unset HOME would affect not just Git. Git may call\n> out something else (e.g. an editor) that may want to know where HOME\n> is, Git may be used in something else (e.g. an IDE) that may want to\n> know where HOME is. Same for XDG_CONFIG_HOME. If it is a valid need\n> to not reading from $HOME/.gitconfig, unsetting HOME is an unacceptable\n> approach to satisfying that need.\n\nWe actually used to have GIT_CONFIG_NOGLOBAL, which was used in the test\nsuite. But after switching to setting $HOME, it went away in 8f323c00dd\n(config: drop support for GIT_CONFIG_NOGLOBAL, 2011-03-15). I agree that\nit's a little more awkward these days because of $XDG_CONFIG_HOME (and\nalso because it influences other programs besides Git).\n\nI'm not particularly opposed to re-adding it, but I do think having an\nenvironment variable for \"read this file instead\", as you suggested\nbelow, would be much better. It's a superset of the \"no\" functionality,\nand would also let us improve our test coverage (especially if we add a\nmatching one for \"system\" config).\n\nLooking at your suggestion:\n\n> So, from these two perspective, for the longer term end-user\n> experience, I am not 100% onboard with GIT_CONFIG_NOGLOBAL. An\n> alternative that allows us to move away from GIT_CONFIG_NOSYSTEM in\n> the direction to the tangent #2 would be not to repeat the same\n> mistake by doing GIT_CONFIG_NOGLOBAL, and instead adding\n> GIT_CONFIG_GLOBAL, which is\n> \n> (1) when not set, it does not do anything,\n> \n> (2) when set to \"/dev/null\" (a literal string; you do not have to\n> spell it \"NUL\" when on Windows), it acts just like the case\n> where your GIT_CONFIG_NOSYSTEM is set,\n> \n> (3) when set to any other string, it is taken as a filename that\n> has the global configuration. Unlike $HOME/.gitconfig or\n> $XDG_HOME/git/config, it is an error if the named file does not\n> exist (this is to catch misconfiguration).\n> \n> And once this is accepted by users and established as a pattern, we\n> could migrate GIT_CONFIG_NOSYSTEM to GIT_CONFIG_SYSTEM=/dev/null\n\nThat seems pretty reasonable. I'm on the fence on your (3). Conceivably\nsomebody could want to override the baked-in defaults without being sure\nthe file is present. But I'm not sure how useful that would be in\npractice.\n\nSome other things to consider:\n\n - does setting GIT_CONFIG_GLOBAL override both the $HOME and\n $XDG_CONFIG_HOME? If the plan is to override them, that makes sense.\n But we do usually read from both of them, so conceivably you might\n want to override just one? That's probably over-engineering, though.\n\n - if we have config for \"read from this file instead of the system\n config\" and \"read from this instead of the user-level config\", then\n I wonder if people will want \"read this instead of the repo config\".\n We have resisted having GIT_CONFIG mean that for many years, because\n I think it gets awkward in some cases (e.g., we'd still want to read\n it for core.repositoryformatversion, etc). I'm OK with drawing the\n line there and saying it's not a support feature, but we should be\n prepared to explain it to users (in the docs or at least in the\n commit message adding the system/global override variables).\n\n-Peff\n"},{"id":"421298","messageId":"8735w0icb9.fsf@evledraar.gmail.com","threadId":"55455","inReplyTo":"a23382059bb57022dd1e40d1c2c9a11307b0ff3b.1617891426.git.ps@pks.im","subject":"Re: [PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-04-08T23:30:50Z","receivedAt":"2021-04-08T23:30:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 08 2021, Patrick Steinhardt wrote:\n\n> +`GIT_CONFIG_NOGLOBAL`::\n> +\tWhether to skip reading settings from the system-wide `~/.gitconfig`\n> +\tand `$XDG_CONFIG_HOME/git/config` files. This environment variable can\n\nLet's not call ~/.gitconfig system-wide with /etc/gitconfig being,\nsaying \"global\" is consistent with git-config's own\n--global/--system/--local etc. Still a bit odd, but at least the same\nnomenclature.\n"},{"id":"421299","messageId":"87zgy8gxjy.fsf@evledraar.gmail.com","threadId":"55455","inReplyTo":"xmqqczv4vgck.fsf@gitster.g","subject":"Re: [PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-04-08T23:34:57Z","receivedAt":"2021-04-08T23:35:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 08 2021, Junio C Hamano wrote:\n\n> Patrick Steinhardt writes:\n>> +test_expect_success 'GIT_CONFIG_NOGLOBAL' '\n>> +\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n>> +\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n>> +\t[home]\n>> +\t\tconfig = true\n>> +\tEOF\n>> +\tmkdir -p \"$HOME\"/.config/git &&\n>> +\tcat >\"$HOME\"/.config/git/config <<-EOF &&\n>> +\t[xdg]\n>> +\t\tconfig = true\n>> +\tEOF\n>> +\tcat >.git/config <<-EOF &&\n>> +\t[local]\n>> +\t\tconfig = true\n>> +\tEOF\n>> +\n>> +\tcat >expect <<-EOF &&\n>> +\tglobal\txdg.config=true\n>> +\tglobal\thome.config=true\n>> +\tlocal\tlocal.config=true\n>> +\tEOF\n>> +\tgit config --show-scope --list >output &&\n>> +\ttest_cmp expect output &&\n>> +\n>> +\tcat >expect <<-EOF &&\n>> +\tlocal\tlocal.config=true\n>> +\tEOF\n>> +\tGIT_CONFIG_NOGLOBAL=true git config --show-scope --list >output &&\n>> +\ttest_cmp expect output\n>> +'\n>\n> This test was what initially piqued my curiosity, as the fact that\n> the result filtered with the new mechanism has only 'local' misled\n> me to incorrectly think that we are suppressing both 'system' and\n> 'global' with the single variable. Until I realized that we cannot\n> test the 'system' configuration in our test suite, that is.\n\nI haven't tested this, but that seems like a thing we want to mock in\nthe test suite by just having git_etc_gitconfig() check a GIT_TEST_*\nvariable.\n\nI had a git_env_str() in [1] that we could use for that, or maybe it\nshould be git_env_path() in this case with the same --path handling, or\njust do what repo_default_branch_name() does.\n\n1. https://lore.kernel.org/git/20201113161320.16458-1-avarab@gmail.com/\n"},{"id":"421303","messageId":"87wntcgxbp.fsf@evledraar.gmail.com","threadId":"55455","inReplyTo":"xmqqczv4vgck.fsf@gitster.g","subject":"Re: [PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-04-08T23:39:54Z","receivedAt":"2021-04-08T23:39:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Apr 08 2021, Junio C Hamano wrote:\n\n> Patrick Steinhardt writes:\n> [...]\n>> Introduce a new GIT_CONFIG_NOGLOBAL envvar, which is the simple\n>> equivalent to GIT_CONFIG_NOSYSTEM. If set to true, git will skip reading\n>> both `~/.gitconfig` and `$XDG_CONFIG_HOME/git/config`.\n>\n> I do not think we'd add an unbounded number of new configuration\n> sources to the existing three-level hierarchy of system-wide\n> (/etc/gitconfig), per-user ($HOME/.gitconfig), and local\n> (.git/config), so it is not too bad, from scalability point of view,\n> to keep adding new GIT_CONFIG_NO$FROTZ variables.\n>\n> Let me go in a couple of different tangents a bit, thinking aloud.\n>\n> Tangent One. I wonder if the presense of includeIf..path\n> changes the \"we won't have unbounded, so adding another here is OK\"\n> reasoning. If somebody wanted to say \"Do not use the paths that\n> match this and that pattern\", it is likely that we'd end up having\n> to support a single variable that allows more than one \"value\". In\n> a straw-man syntax \"GIT_CONFIG_EXCLUDE='/home/gitster/*\n> /home/peff/*'\" might be a way to say \"do not use files that are\n> under these two directories.\n>\n> And when that happens, we'd probably notice that it is easier for\n> users to configure, if they can treat 'system' and 'global' as just\n> another special pattern to place on that list. //system and //global\n> might be the syntax we'd choose when time comes, i.e.\n>\n> \tGIT_CONFIG_EXCLUDE='//system //global'\n>\n> might become a more scalable replacement for\n>\n> \tGIT_CONFIG_NOSYSTEM=yes GIT_CONFIG_NOHOME=yes\n>\n> Tangent Two. One thing we never managed to properly test in our\n> test suite is the unctioning of GIT_CONFIG_NOSYSTEM. As we do not\n> want to get broken by whatever is in /etc/gitconfig, all our tests\n> run with the environment variable set. For the same reason, in\n> order to avoid getting influenced by whatever the tester has in\n> $HOME/.gitconfig, we export HOME set to the throw-away directory\n> created during the test and control what is in the config file in\n> that directory. In hindsight, it might have been a better design to\n> use GIT_CONFIG_SYSTEM variable that points at a path to be used as a\n> replacement for /etc/gitconfig when it is set; pointing /dev/null\n> with the variable would have been the natural way to say \"do not use\n> anything from system configuration\". And GIT_CONFIG_GLOBAL can\n> easily follow the same pattern.\n>\n> So, from these two perspective, for the longer term end-user\n> experience, I am not 100% onboard with GIT_CONFIG_NOGLOBAL. An\n> alternative that allows us to move away from GIT_CONFIG_NOSYSTEM in\n> the direction to the tangent #2 would be not to repeat the same\n> mistake by doing GIT_CONFIG_NOGLOBAL, and instead adding\n> GIT_CONFIG_GLOBAL, which is\n>\n> (1) when not set, it does not do anything,\n>\n> (2) when set to \"/dev/null\" (a literal string; you do not have to\n> spell it \"NUL\" when on Windows), it acts just like the case\n> where your GIT_CONFIG_NOSYSTEM is set,\n>\n> (3) when set to any other string, it is taken as a filename that\n> has the global configuration. Unlike $HOME/.gitconfig or\n> $XDG_HOME/git/config, it is an error if the named file does not\n> exist (this is to catch misconfiguration).\n>\n> And once this is accepted by users and established as a pattern, we\n> could migrate GIT_CONFIG_NOSYSTEM to GIT_CONFIG_SYSTEM=/dev/null\n>\n>\n> Having said all that (meaning: I am not 100% onboard with _NOGLOBAL\n> and think _GLOBAL=/dev/null might be a better design), let's give it\n> a review under the assumption that _NOGLOBAL is the design we would\n> want to choose.\n\nI think doing this via env vars is inherently nasty, but can't think of\na good way to implement it properly without major refactoring.\n\nIMO the \"properly\" would be that we'd just support this sort of thing as\nfirst-class config syntax, as I've suggested before e.g. in the repo\nhooks/config topic.\n\nBut to do that we couldn't \"stream\" the config reading as we do now,\nwe'd need to read system/global/local, and when we saw some new\nmeta-syntax apply those ignores to files we already read, and only then\nstart streaming things to config callbacks.\n\nSee \"config.ignore.*\" in\nhttps://lore.kernel.org/git/87y2ebo16v.fsf@evledraar.gmail.com/\n"},{"id":"421306","messageId":"xmqq35w0qr5f.fsf@gitster.g","threadId":"55455","inReplyTo":"YG+PLROZbSo32F3k@coredump.intra.peff.net","subject":"Re: [PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-08T23:43:08Z","receivedAt":"2021-04-08T23:43:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> (3) when set to any other string, it is taken as a filename that\n>> has the global configuration. Unlike $HOME/.gitconfig or\n>> $XDG_HOME/git/config, it is an error if the named file does not\n>> exist (this is to catch misconfiguration).\n>> \n>> And once this is accepted by users and established as a pattern, we\n>> could migrate GIT_CONFIG_NOSYSTEM to GIT_CONFIG_SYSTEM=/dev/null\n>\n> That seems pretty reasonable. I'm on the fence on your (3). Conceivably\n> somebody could want to override the baked-in defaults without being sure\n> the file is present. But I'm not sure how useful that would be in\n> practice.\n\nI was also on the fence. \n\nIf your plan is to create $HOME/.alternate-config and point at it by\nsetting GIT_CONFIG_GLOBAL=$HOME/.alternate-config, there are two\nplaces you can make typo. You may write a file with a wrong name.\nYou may export a variable with a wrong name.\n\n> Some other things to consider:\n>\n> - does setting GIT_CONFIG_GLOBAL override both the $HOME and\n> $XDG_CONFIG_HOME? If the plan is to override them, that makes sense.\n> But we do usually read from both of them, so conceivably you might\n> want to override just one? That's probably over-engineering, though.\n\nI viewed this to be working at the more conceptual \"here is the file\nto read 'system' (or 'per-user') stuff from\" level, and not at the\nlevel of the individual file, as I consider that it is a mere\nimplementation detail that 'per-user' may read from multiple files.\n\n> - if we have config for \"read from this file instead of the system\n> config\" and \"read from this instead of the user-level config\", then\n> I wonder if people will want \"read this instead of the repo config\".\n> We have resisted having GIT_CONFIG mean that for many years, because\n> I think it gets awkward in some cases (e.g., we'd still want to read\n> it for core.repositoryformatversion, etc). I'm OK with drawing the\n> line there and saying it's not a support feature, but we should be\n> prepared to explain it to users (in the docs or at least in the\n> commit message adding the system/global override variables).\n\nWe may have to bite the bullet and make an official catalog of\nreally \"structurely fundamental\" configuration variables that must\nappear in the per-repository file, and a mechanism to enforce that\nby always reading these variables from \"$GIT_DIR/config\" and always\nignoring appearances of them in any other places.\n\nThat alone would probably be a good thing to do regardless of the\nGIT_CONFIG_NOGLOBAL issue, as I suspect you may be able to wreak\nhavoc by adding random configuration like [extension] in\n$HOME/.gitconfig ;-)\n\nWith that, it would make sense to allow overriding the per-repo\nconfiguration in a similar way, only as a mechanism to override\nconfiguration variables that are about \"preferences\" (as opposed to\nthe structurally fundamental ones).\n"},{"id":"421311","messageId":"xmqqo8eopbyk.fsf@gitster.g","threadId":"55455","inReplyTo":"8735w0icb9.fsf@evledraar.gmail.com","subject":"Re: [PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-08T23:56:35Z","receivedAt":"2021-04-08T23:56:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Thu, Apr 08 2021, Patrick Steinhardt wrote:\n>\n>> +`GIT_CONFIG_NOGLOBAL`::\n>> +\tWhether to skip reading settings from the system-wide `~/.gitconfig`\n>> +\tand `$XDG_CONFIG_HOME/git/config` files. This environment variable can\n>\n> Let's not call ~/.gitconfig system-wide with /etc/gitconfig being,\n> saying \"global\" is consistent with git-config's own\n> --global/--system/--local etc. Still a bit odd, but at least the same\n> nomenclature.\n\nYeah, GLOBAL should be renamed to PER-USER everywhere.\n"},{"id":"421316","messageId":"YG+e/zGgoITH6qZa@coredump.intra.peff.net","threadId":"55455","inReplyTo":"xmqq35w0qr5f.fsf@gitster.g","subject":"Re: [PATCH] config: Introduce GIT_CONFIG_NOGLOBAL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-09T00:25:35Z","receivedAt":"2021-04-09T00:25:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 08, 2021 at 04:43:08PM -0700, Junio C Hamano wrote:\n\n> > Some other things to consider:\n> >\n> > - does setting GIT_CONFIG_GLOBAL override both the $HOME and\n> > $XDG_CONFIG_HOME? If the plan is to override them, that makes sense.\n> > But we do usually read from both of them, so conceivably you might\n> > want to override just one? That's probably over-engineering, though.\n> \n> I viewed this to be working at the more conceptual \"here is the file\n> to read 'system' (or 'per-user') stuff from\" level, and not at the\n> level of the individual file, as I consider that it is a mere\n> implementation detail that 'per-user' may read from multiple files.\n\nYeah. I'm OK with that. I think it's conceptually simpler. I was going\nto also say \"less flexible\", but once you have pointed Git at the file\nof your choice, it is easy to [include] any other one if you want.\n\n> We may have to bite the bullet and make an official catalog of\n> really \"structurely fundamental\" configuration variables that must\n> appear in the per-repository file, and a mechanism to enforce that\n> by always reading these variables from \"$GIT_DIR/config\" and always\n> ignoring appearances of them in any other places.\n> \n> That alone would probably be a good thing to do regardless of the\n> GIT_CONFIG_NOGLOBAL issue, as I suspect you may be able to wreak\n> havoc by adding random configuration like [extension] in\n> $HOME/.gitconfig ;-)\n\nI think we've historically had some problems there, but I think these\ndays it's not too bad. We look directly at $GIT_DIR/config with\nread_repository_format(), loading only stuff that check_repo_format()\ncares about:\n\n - core.repositoryformatversion\n\n - extensions.*\n\n - core.bare\n\n - core.worktree\n\nAnd those _should_ be ignored by the regular config-reading paths. So I\nthink it would be OK to have a $GIT_CONFIG_LOCAL variable that overrides\nthe location in do_git_config_sequence() only, and that would cover only\nthe preferences parts. I do agree it wouldn't hurt to document which\noptions are read in which context.\n\nI do wonder how useful such an option would be, though. I can see why\nyou would want to redirect or disable system-level or user-level config\nonce, and then use it for many invocations. But in a single repo, you\nmight as well just set the repo config, or override it for a single\ninvocation with \"git -c\", etc.\n\nI guess one possible use could be \"I got a repository I do not trust as\na tarball and want to investigate it without paying attention to its\nlocal config\". But that's a pretty narrow case, and you'd probably be\nbetter off just vetting the config yourself anyway (since depending on\nwhat you want to do in the repo, you may need some of that config, like\nremote.*, etc).\n\nI dunno.\n\n-Peff\n"},{"id":"421391","messageId":"da0b8ce6f02d9d182446671cc06b9ee1cda77785.1617975637.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1617975637.git.ps@pks.im","subject":"[PATCH v2 1/3] config: rename `git_etc_config()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-09T13:43:21Z","receivedAt":"2021-04-09T13:43:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `git_etc_gitconfig()` function retrieves the system-level path of\nthe configuration file. We're about to introduce a way to override it\nvia an environment variable, at which point the name of this function\nwould start to become misleading.\n\nRename the function to `git_system_config()` as a preparatory step.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/config.c | 2 +-\n config.c | 6 +++---\n config.h | 3 ++-\n 3 files changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex f71fa39b38..02ed0b3fe7 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -695,7 +695,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \telse if (use_system_config) {\n-\t\tgiven_config_source.file = git_etc_gitconfig();\n+\t\tgiven_config_source.file = git_system_config();\n \t\tgiven_config_source.scope = CONFIG_SCOPE_SYSTEM;\n \t} else if (use_local_config) {\n \t\tgiven_config_source.file = git_pathdup(\"config\");\ndiff --git a/config.c b/config.c\nindex 6428393a41..c552ab4ad9 100644\n--- a/config.c\n+++ b/config.c\n@@ -1844,7 +1844,7 @@ static int git_config_from_blob_ref(config_fn_t fn,\n \treturn git_config_from_blob_oid(fn, name, &oid, data);\n }\n \n-const char *git_etc_gitconfig(void)\n+const char *git_system_config(void)\n {\n \tstatic const char *system_wide;\n \tif (!system_wide)\n@@ -1896,10 +1896,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\trepo_config = NULL;\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n-\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,\n+\tif (git_config_system() && !access_or_die(git_system_config(), R_OK,\n \t\t\t\t\t\t opts->system_gently ?\n \t\t\t\t\t\t ACCESS_EACCES_OK : 0))\n-\t\tret += git_config_from_file(fn, git_etc_gitconfig(),\n+\t\tret += git_config_from_file(fn, git_system_config(),\n \t\t\t\t\t data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\ndiff --git a/config.h b/config.h\nindex 19a9adbaa9..8e8376ae19 100644\n--- a/config.h\n+++ b/config.h\n@@ -318,7 +318,6 @@ int git_config_rename_section(const char *, const char *);\n int git_config_rename_section_in_file(const char *, const char *, const char *);\n int git_config_copy_section(const char *, const char *);\n int git_config_copy_section_in_file(const char *, const char *, const char *);\n-const char *git_etc_gitconfig(void);\n int git_env_bool(const char *, int);\n unsigned long git_env_ulong(const char *, unsigned long);\n int git_config_system(void);\n@@ -327,6 +326,8 @@ int config_error_nonbool(const char *);\n #define config_error_nonbool(s) (config_error_nonbool(s), const_error())\n #endif\n \n+const char *git_system_config(void);\n+\n int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n \n enum config_scope current_config_scope(void);\n-- \n2.31.1\n\n"},{"id":"421392","messageId":"cover.1617975637.git.ps@pks.im","threadId":"55455","inReplyTo":"a23382059bb57022dd1e40d1c2c9a11307b0ff3b.1617891426.git.ps@pks.im","subject":"[PATCH v2 0/3] config: allow overriding global/system config","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-09T13:43:16Z","receivedAt":"2021-04-09T13:43:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the second version of my patch series to provide a way of\noverriding the global system configuration.\n\nInstead of going for GIT_CONFIG_NOGLOBAL, I've adopted Junio's proposal\nof going with GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM, which allow a\nuser to modify the locations of those files. Thanks for the discussion,\nthis solution feels a lot nicer to me!\n\nPatrick\n\nPatrick Steinhardt (3):\n config: rename `git_etc_config()`\n config: unify code paths to get global config paths\n config: allow overriding of global and system configuration\n\n Documentation/git-config.txt | 5 +++\n Documentation/git.txt | 10 +++++\n builtin/config.c | 8 ++--\n config.c | 56 ++++++++++++++++++++++-----\n config.h | 4 +-\n t/t1300-config.sh | 75 ++++++++++++++++++++++++++++++++++++\n 6 files changed, 143 insertions(+), 15 deletions(-)\n\n-- \n2.31.1\n\n"},{"id":"421393","messageId":"dddc85bcf54e9b19f1612cf2a5be928dcb2bad7d.1617975637.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1617975637.git.ps@pks.im","subject":"[PATCH v2 2/3] config: unify code paths to get global config paths","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-09T13:43:25Z","receivedAt":"2021-04-09T13:43:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There's two callsites which assemble global config paths, once in the\nconfig loading code and once in the git-config(1) builtin. We're about\nto implement a way to override global config paths via an environment\nvariable which would require us to adjust both sites.\n\nUnify both code paths into a single `git_global_config()` function which\nreturns both paths for `~/.gitconfig` and the XDG config file. This will\nmake the subsequent patch which introduces the new envvar easier to\nimplement.\n\nNo functional changes are expected from this patch.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/config.c | 6 ++----\n config.c | 20 ++++++++++++++++----\n config.h | 1 +\n 3 files changed, 19 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 02ed0b3fe7..604a0973a5 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -671,9 +671,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (use_global_config) {\n-\t\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n-\t\tchar *xdg_config = xdg_config_home(\"config\");\n+\t\tconst char *user_config, *xdg_config;\n \n+\t\tgit_global_config(&user_config, &xdg_config);\n \t\tif (!user_config)\n \t\t\t/*\n \t\t\t * It is unknown if HOME/.gitconfig exists, so\n@@ -688,10 +688,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tif (access_or_warn(user_config, R_OK, 0) &&\n \t\t xdg_config && !access_or_warn(xdg_config, R_OK, 0)) {\n \t\t\tgiven_config_source.file = xdg_config;\n-\t\t\tfree(user_config);\n \t\t} else {\n \t\t\tgiven_config_source.file = user_config;\n-\t\t\tfree(xdg_config);\n \t\t}\n \t}\n \telse if (use_system_config) {\ndiff --git a/config.c b/config.c\nindex c552ab4ad9..6af0244085 100644\n--- a/config.c\n+++ b/config.c\n@@ -1852,6 +1852,19 @@ const char *git_system_config(void)\n \treturn system_wide;\n }\n \n+void git_global_config(const char **user, const char **xdg)\n+{\n+\tstatic const char *user_config, *xdg_config;\n+\n+\tif (!user_config) {\n+\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n+\t\txdg_config = xdg_config_home(\"config\");\n+\t}\n+\n+\t*user = user_config;\n+\t*xdg = xdg_config;\n+}\n+\n /*\n * Parse environment variable 'k' as a boolean (in various\n * possible spellings); if missing, use the default value 'def'.\n@@ -1883,9 +1896,8 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\t\t\t config_fn_t fn, void *data)\n {\n \tint ret = 0;\n-\tchar *xdg_config = xdg_config_home(\"config\");\n-\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n \tchar *repo_config;\n+\tconst char *user_config, *xdg_config;\n \tenum config_scope prev_parsing_scope = current_parsing_scope;\n \n \tif (opts->commondir)\n@@ -1903,6 +1915,8 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\t\t\t\t data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n+\tgit_global_config(&user_config, &xdg_config);\n+\n \tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n \t\tret += git_config_from_file(fn, xdg_config, data);\n \n@@ -1927,8 +1941,6 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\tdie(_(\"unable to parse command-line config\"));\n \n \tcurrent_parsing_scope = prev_parsing_scope;\n-\tfree(xdg_config);\n-\tfree(user_config);\n \tfree(repo_config);\n \treturn ret;\n }\ndiff --git a/config.h b/config.h\nindex 8e8376ae19..53a782e0f5 100644\n--- a/config.h\n+++ b/config.h\n@@ -327,6 +327,7 @@ int config_error_nonbool(const char *);\n #endif\n \n const char *git_system_config(void);\n+void git_global_config(const char **user, const char **xdg);\n \n int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n \n-- \n2.31.1\n\n"},{"id":"421394","messageId":"272a3b31aa73da8d65b04e647b1b9ca860f4e783.1617975637.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1617975637.git.ps@pks.im","subject":"[PATCH v2 3/3] config: allow overriding of global and system configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-09T13:43:30Z","receivedAt":"2021-04-09T13:43:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In order to have git run in a fully controlled environment without any\nmisconfiguration, it may be desirable for users or scripts to override\nglobal- and system-level configuration files. We already have a way of\ndoing this, which is to unset both HOME and XDG_CONFIG_HOME environment\nvariables and to set `GIT_CONFIG_NOGLOBAL=true`. This is quite kludgy,\nand unsetting the first two variables likely has an impact on other\nexecutables spawned by such a script.\n\nThe obvious way to fix this would be to introduce `GIT_CONFIG_NOSYSTEM`\nas an equivalent to `GIT_CONFIG_NOGLOBAL`. But in the past, it has\nturned out that this design is inflexible: we cannot test system-level\nparsing of the git configuration in our test harness because there is no\nway to change its location, so all tests run with `GIT_CONFIG_NOSYSTEM`\nset.\n\nInstead of doing the same mistake with `GIT_CONFIG_NOGLOBAL`, introduce\ntwo new variables `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM`:\n\n - If unset, git continues to use the usual locations.\n\n - If set to a specific path, we skip reading the normal\n configuration files and instead take the path. This path must\n exist and be readable to ensure that the user didn't typo.\n\n - If set to `/dev/null`, we do not load either global- or\n system-level configuration at all.\n\nThis implements the usecase where we want to execute code in a sanitized\nenvironment without any potential misconfigurations via `/dev/null`, but\nis more flexible and allows for more usecases than simply adding\n`GIT_CONFIG_NOGLOBAL`.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/git-config.txt | 5 +++\n Documentation/git.txt | 10 +++++\n config.c | 34 ++++++++++++++--\n t/t1300-config.sh | 75 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 120 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 4b4cc5c5e8..5cddadafd2 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -340,6 +340,11 @@ 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_GLOBAL::\n+GIT_CONFIG_SYSTEM::\n+\tTake the configuration from the given files instead from global or\n+\tsystem-level configuration. See linkgit:git[1] for details.\n+\n GIT_CONFIG_NOSYSTEM::\n \tWhether to skip reading settings from the system-wide\n \t$(prefix)/etc/gitconfig file. See linkgit:git[1] for details.\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 3a9c44987f..2129629296 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -670,6 +670,16 @@ for further details.\n \tIf this environment variable is set to `0`, git will not prompt\n \ton the terminal (e.g., when asking for HTTP authentication).\n \n+`GIT_CONFIG_GLOBAL`::\n+`GIT_CONFIG_SYSTEM`::\n+\tTake the configuration from the given files instead from global or\n+\tsystem-level configuration files. The files must exist and be readable\n+\tby the current user. If `GIT_CONFIG_SYSTEM` is set, `/etc/gitconfig`\n+\twill not be read. Likewise, if `GIT_CONFIG_GLOBAL` is set, neither\n+\t`$HOME/.gitconfig` nor `$XDG_CONFIG_HOME/git/config` will be read. Can\n+\tbe set to `/dev/null` to skip reading configuration files of the\n+\trespective level.\n+\n `GIT_CONFIG_NOSYSTEM`::\n \tWhether to skip reading settings from the system-wide\n \t`$(prefix)/etc/gitconfig` file. This environment variable can\ndiff --git a/config.c b/config.c\nindex 6af0244085..9dfc4a79f7 100644\n--- a/config.c\n+++ b/config.c\n@@ -1847,8 +1847,22 @@ static int git_config_from_blob_ref(config_fn_t fn,\n const char *git_system_config(void)\n {\n \tstatic const char *system_wide;\n-\tif (!system_wide)\n-\t\tsystem_wide = system_path(ETC_GITCONFIG);\n+\n+\tif (!system_wide) {\n+\t\tsystem_wide = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n+\t\tif (system_wide) {\n+\t\t\t/*\n+\t\t\t * If GIT_CONFIG_SYSTEM is set, it overrides the\n+\t\t\t * /etc/gitconfig. Furthermore, the file must exist in\n+\t\t\t * order to prevent any typos by the user.\n+\t\t\t */\n+\t\t\tif (access(system_wide, R_OK))\n+\t\t\t\tdie(_(\"cannot access '%s'\"), system_wide);\n+\t\t} else {\n+\t\t\tsystem_wide = system_path(ETC_GITCONFIG);\n+\t\t}\n+\t}\n+\n \treturn system_wide;\n }\n \n@@ -1857,8 +1871,20 @@ void git_global_config(const char **user, const char **xdg)\n \tstatic const char *user_config, *xdg_config;\n \n \tif (!user_config) {\n-\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n-\t\txdg_config = xdg_config_home(\"config\");\n+\t\tuser_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n+\t\tif (user_config) {\n+\t\t\t/*\n+\t\t\t * If GIT_CONFIG_GLOBAL is set, then it overrides both\n+\t\t\t * the ~/.gitconfig and the XDG configuration file.\n+\t\t\t * Furthermore, the file must exist in order to prevent\n+\t\t\t * any typos by the user.\n+\t\t\t */\n+\t\t\tif (access(user_config, R_OK))\n+\t\t\t\tdie(_(\"cannot access '%s'\"), user_config);\n+\t\t} else {\n+\t\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n+\t\t\txdg_config = xdg_config_home(\"config\");\n+\t\t}\n \t}\n \n \t*user = user_config;\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex e0dd5d65ce..5498ca32b0 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -2059,6 +2059,81 @@ test_expect_success '--show-scope with --show-origin' '\n \ttest_cmp expect output\n '\n \n+test_expect_success 'override global and system config' '\n+\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n+\n+\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n+\t[home]\n+\t\tconfig = true\n+\tEOF\n+\tmkdir -p \"$HOME\"/.config/git &&\n+\tcat >\"$HOME\"/.config/git/config <<-EOF &&\n+\t[xdg]\n+\t\tconfig = true\n+\tEOF\n+\tcat >.git/config <<-EOF &&\n+\t[local]\n+\t\tconfig = true\n+\tEOF\n+\tcat >custom-global-config <<-EOF &&\n+\t[global]\n+\t\tconfig = true\n+\tEOF\n+\tcat >custom-system-config <<-EOF &&\n+\t[system]\n+\t\tconfig = true\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\tglobal\txdg.config=true\n+\tglobal\thome.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\n+\tcat >expect <<-EOF &&\n+\tsystem\tsystem.config=true\n+\tglobal\tglobal.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \\\n+\t\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tcat >expect <<-EOF &&\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null git config --show-scope --list >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'override global and system config with missing file' '\n+\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist git version &&\n+\ttest_must_fail env GIT_CONFIG_SYSTEM=does-not-exist git version &&\n+\tGIT_CONFIG_NOSYSTEM=true GIT_CONFIG_SYSTEM=does-not-exist git version\n+'\n+\n+test_expect_success 'write to overridden global and system config' '\n+\tcat >expect < The `git_etc_gitconfig()` function retrieves the system-level path of\n> the configuration file. We're about to introduce a way to override it\n> via an environment variable, at which point the name of this function\n> would start to become misleading.\n> \n> Rename the function to `git_system_config()` as a preparatory step.\n\nLooks good. This name has been a minor annoyance for years. :)\n\n-Peff\n"},{"id":"421408","messageId":"YHBw7D/fEUdCckc8@coredump.intra.peff.net","threadId":"55455","inReplyTo":"dddc85bcf54e9b19f1612cf2a5be928dcb2bad7d.1617975637.git.ps@pks.im","subject":"Re: [PATCH v2 2/3] config: unify code paths to get global config paths","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-09T15:21:16Z","receivedAt":"2021-04-09T15:21:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 09, 2021 at 03:43:25PM +0200, Patrick Steinhardt wrote:\n\n> There's two callsites which assemble global config paths, once in the\n> config loading code and once in the git-config(1) builtin. We're about\n> to implement a way to override global config paths via an environment\n> variable which would require us to adjust both sites.\n> \n> Unify both code paths into a single `git_global_config()` function which\n> returns both paths for `~/.gitconfig` and the XDG config file. This will\n> make the subsequent patch which introduces the new envvar easier to\n> implement.\n\nSeems like a good step forward. There is one minor issue with the\nimplementation, though.\n\n> diff --git a/builtin/config.c b/builtin/config.c\n> index 02ed0b3fe7..604a0973a5 100644\n> --- a/builtin/config.c\n> +++ b/builtin/config.c\n> @@ -671,9 +671,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n> \t}\n> \n> \tif (use_global_config) {\n> -\t\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n> -\t\tchar *xdg_config = xdg_config_home(\"config\");\n> +\t\tconst char *user_config, *xdg_config;\n> \n> +\t\tgit_global_config(&user_config, &xdg_config);\n\nThe pointer out-parameters make sense here, since we need to return two\nvalues. I notice they became const, so the function will hold on to\nownership of the memory.\n\n> @@ -688,10 +688,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n> \t\tif (access_or_warn(user_config, R_OK, 0) &&\n> \t\t xdg_config && !access_or_warn(xdg_config, R_OK, 0)) {\n> \t\t\tgiven_config_source.file = xdg_config;\n> -\t\t\tfree(user_config);\n> \t\t} else {\n> \t\t\tgiven_config_source.file = user_config;\n> -\t\t\tfree(xdg_config);\n> \t\t}\n\n...which is why we drop these free() calls. So far so good.\n\n> +void git_global_config(const char **user, const char **xdg)\n> +{\n> +\tstatic const char *user_config, *xdg_config;\n> +\n> +\tif (!user_config) {\n> +\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n> +\t\txdg_config = xdg_config_home(\"config\");\n> +\t}\n> +\n> +\t*user = user_config;\n> +\t*xdg = xdg_config;\n> +}\n\nAnd here in the implementation we hold on to the static values forever.\n\nI think your \"did we initialize already\" check isn't robust, though.\nexpand_user_path() can return NULL, in which case every call would\ntrigger a re-initialization (even leaking xdg_config if it was set in\nthe last round).\n\nSo I think you'd need a separate \"static int initialized\" variable.\n\nThat said, I wonder if we should just pass ownership of the memory to\nthe caller. It is a minor inconvenience that they will have to free()\nthe result, but we're already doing that. And it removes any possibility\nof thread unsafety.\n\nI guess it doesn't match git_system_config() as well, then. But arguably\nit should also just pass ownership (it also has only a handful of\ncallers, and freeing the result would not be a big deal).\n\nI'm OK with either solution, though.\n\n-Peff\n"},{"id":"421410","messageId":"YHB092HN2oVLmqC1@coredump.intra.peff.net","threadId":"55455","inReplyTo":"272a3b31aa73da8d65b04e647b1b9ca860f4e783.1617975637.git.ps@pks.im","subject":"Re: [PATCH v2 3/3] config: allow overriding of global and system configuration","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-09T15:38:31Z","receivedAt":"2021-04-09T15:38:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 09, 2021 at 03:43:30PM +0200, Patrick Steinhardt wrote:\n\n> In order to have git run in a fully controlled environment without any\n> misconfiguration, it may be desirable for users or scripts to override\n> global- and system-level configuration files. We already have a way of\n> doing this, which is to unset both HOME and XDG_CONFIG_HOME environment\n> variables and to set `GIT_CONFIG_NOGLOBAL=true`. This is quite kludgy,\n> and unsetting the first two variables likely has an impact on other\n> executables spawned by such a script.\n> \n> The obvious way to fix this would be to introduce `GIT_CONFIG_NOSYSTEM`\n> as an equivalent to `GIT_CONFIG_NOGLOBAL`. But in the past, it has\n\nI think you have NOSYSTEM and NOGLOBAL mixed up in both paragraphs here?\n\nOtherwise the motivation and description here look very good (and I like\nthe overall direction).\n\n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 3a9c44987f..2129629296 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -670,6 +670,16 @@ for further details.\n> \tIf this environment variable is set to `0`, git will not prompt\n> \ton the terminal (e.g., when asking for HTTP authentication).\n> \n> +`GIT_CONFIG_GLOBAL`::\n> +`GIT_CONFIG_SYSTEM`::\n> +\tTake the configuration from the given files instead from global or\n> +\tsystem-level configuration files. The files must exist and be readable\n> +\tby the current user. If `GIT_CONFIG_SYSTEM` is set, `/etc/gitconfig`\n> +\twill not be read. Likewise, if `GIT_CONFIG_GLOBAL` is set, neither\n> +\t`$HOME/.gitconfig` nor `$XDG_CONFIG_HOME/git/config` will be read. Can\n> +\tbe set to `/dev/null` to skip reading configuration files of the\n> +\trespective level.\n\nMakes sense. The reference to `/etc/gitconfig` here may not be accurate,\ndepending on the build parameters. I notice below in the context that we\nsay:\n\n> `GIT_CONFIG_NOSYSTEM`::\n> \tWhether to skip reading settings from the system-wide\n> \t`$(prefix)/etc/gitconfig` file. This environment variable can\n\nwhich is _also_ not quite right (if $(prefix) is \"/usr\", then the file\nreally is /etc/gitconfig).\n\nI think it might be possible to pull the value of the ETC_GITCONFIG\nMakefile variable into the documentation, so we could probably give the\n\"real\" value. But I wonder if it would suffice to just say:\n\n ...the system config (usually `/etc/gitconfig`) will not be read.\n\nOr is that too confusing (it invites the question \"when is it not\n/etc/gitconfig\")? I guess we could say \"the system config file defined\nat build-time (usually `/etc/gitconfig`)\", which is perhaps more clear.\n\n> @@ -1847,8 +1847,22 @@ static int git_config_from_blob_ref(config_fn_t fn,\n> const char *git_system_config(void)\n> {\n> \tstatic const char *system_wide;\n> -\tif (!system_wide)\n> -\t\tsystem_wide = system_path(ETC_GITCONFIG);\n> +\n> +\tif (!system_wide) {\n> +\t\tsystem_wide = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n\nI wondered, given the \"const char *\" return values in the last patch, if\nyou might pass back the result of getenv() directly. But you duplicate\nit here, which is good, as it avoids portability problems hanging on to\nthe result of getenv().\n\n> +\t\tif (system_wide) {\n> +\t\t\t/*\n> +\t\t\t * If GIT_CONFIG_SYSTEM is set, it overrides the\n> +\t\t\t * /etc/gitconfig. Furthermore, the file must exist in\n> +\t\t\t * order to prevent any typos by the user.\n> +\t\t\t */\n> +\t\t\tif (access(system_wide, R_OK))\n> +\t\t\t\tdie(_(\"cannot access '%s'\"), system_wide);\n> +\t\t} else {\n> +\t\t\tsystem_wide = system_path(ETC_GITCONFIG);\n> +\t\t}\n> +\t}\n\nI was on the fence about whether to enforce the \"this file must exist\"\nproperty, with respect to the overall design. But seeing how we must\nactually add extra code here to handle it makes me want to just treat it\nexactly like the other files.\n\nUsing a separate access() here is also a TOCTOU race, but I'm pretty\nsure the existing config code makes the same mistake (and it's not that\nbig a deal in this context).\n\n> @@ -1857,8 +1871,20 @@ void git_global_config(const char **user, const char **xdg)\n> \tstatic const char *user_config, *xdg_config;\n> \n> \tif (!user_config) {\n> -\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n> -\t\txdg_config = xdg_config_home(\"config\");\n> +\t\tuser_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n> +\t\tif (user_config) {\n> +\t\t\t/*\n> +\t\t\t * If GIT_CONFIG_GLOBAL is set, then it overrides both\n> +\t\t\t * the ~/.gitconfig and the XDG configuration file.\n> +\t\t\t * Furthermore, the file must exist in order to prevent\n> +\t\t\t * any typos by the user.\n> +\t\t\t */\n> +\t\t\tif (access(user_config, R_OK))\n> +\t\t\t\tdie(_(\"cannot access '%s'\"), user_config);\n> +\t\t} else {\n> +\t\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n> +\t\t\txdg_config = xdg_config_home(\"config\");\n> +\t\t}\n> \t}\n\nAnd this looks as I'd expect (but the same comments as above apply, of\ncourse).\n\n> +test_expect_success 'override global and system config' '\n> +\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n> +\n> +\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n> +\t[home]\n> +\t\tconfig = true\n> +\tEOF\n> +\tmkdir -p \"$HOME\"/.config/git &&\n> +\tcat >\"$HOME\"/.config/git/config <<-EOF &&\n> +\t[xdg]\n> +\t\tconfig = true\n> +\tEOF\n> +\tcat >.git/config <<-EOF &&\n> +\t[local]\n> +\t\tconfig = true\n> +\tEOF\n> +\tcat >custom-global-config <<-EOF &&\n> +\t[global]\n> +\t\tconfig = true\n> +\tEOF\n> +\tcat >custom-system-config <<-EOF &&\n> +\t[system]\n> +\t\tconfig = true\n> +\tEOF\n\nMinor style nit, but we usually prefer non-interpolating \"\\EOF\" if we\ndon't intend to interpolate within the here-doc. It does look like\nt1300 has quite a mix of styles, though.\n\n> +\tcat >expect <<-EOF &&\n> +\tglobal\txdg.config=true\n> +\tglobal\thome.config=true\n> +\tlocal\tlocal.config=true\n> +\tEOF\n> +\tgit config --show-scope --list >output &&\n> +\ttest_cmp expect output &&\n> +\n> +\tsane_unset GIT_CONFIG_NOSYSTEM &&\n> +\n> +\tcat >expect <<-EOF &&\n> +\tsystem\tsystem.config=true\n> +\tglobal\tglobal.config=true\n> +\tlocal\tlocal.config=true\n> +\tEOF\n> +\tGIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \\\n> +\t\tgit config --show-scope --list >output &&\n> +\ttest_cmp expect output &&\n> +\n> +\tcat >expect <<-EOF &&\n> +\tlocal\tlocal.config=true\n> +\tEOF\n> +\tGIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null git config --show-scope --list >output &&\n> +\ttest_cmp expect output\n> +'\n\nAnd this test covers all of the new stuff we care about. Good.\n\n> +test_expect_success 'override global and system config with missing file' '\n> +\tsane_unset GIT_CONFIG_NOSYSTEM &&\n> +\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist git version &&\n> +\ttest_must_fail env GIT_CONFIG_SYSTEM=does-not-exist git version &&\n> +\tGIT_CONFIG_NOSYSTEM=true GIT_CONFIG_SYSTEM=does-not-exist git version\n> +'\n\nMakes sense to test given the patch, though if we rip out the \"missing\"\ncheck, then obviously this goes away.\n\n> +test_expect_success 'write to overridden global and system config' '\n\nHmm. I hadn't really considered _writing_ to these files (after all, you\ncan just use \"git config --file\" to do so). I guess it is consistent,\nand would probably be more work (and more error-prone) to try to\ndistinguish reading versus writing in the code.\n\n> +\tcat >expect < +[config]\n> +\tkey = value\n> +EOF\n\nNo \"<<-EOF\" here to fix the indentation?\n\n> +\ttest_must_fail env GIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n> +\ttouch write-to-global &&\n> +\tGIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n> +\ttest_cmp expect write-to-global &&\n\nIn the writing case, the \"must exist\" thing makes it even weirder, since\nwe might be intending to create the file! If we leave in the writing,\nthat makes me even more convinced that we should drop the \"must exist\"\ncheck.\n\n-Peff\n"},{"id":"421411","messageId":"YHB1n+d/60MaQjsH@coredump.intra.peff.net","threadId":"55455","inReplyTo":"cover.1617975637.git.ps@pks.im","subject":"Re: [PATCH v2 0/3] config: allow overriding global/system config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-09T15:41:19Z","receivedAt":"2021-04-09T15:41:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 09, 2021 at 03:43:16PM +0200, Patrick Steinhardt wrote:\n\n> Instead of going for GIT_CONFIG_NOGLOBAL, I've adopted Junio's proposal\n> of going with GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM, which allow a\n> user to modify the locations of those files. Thanks for the discussion,\n> this solution feels a lot nicer to me!\n\nThanks, I like this much better. I left a few comments on the patches\nthemselves. Mostly small suggestions, but design-wise I'm of the opinion\nwe should drop the \"file must exist\" requirement; see the response to\npatch 3.\n\n-Peff\n"},{"id":"421483","messageId":"xmqqo8enm793.fsf@gitster.g","threadId":"55455","inReplyTo":"272a3b31aa73da8d65b04e647b1b9ca860f4e783.1617975637.git.ps@pks.im","subject":"Re: [PATCH v2 3/3] config: allow overriding of global and system configuration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-09T22:18:48Z","receivedAt":"2021-04-09T22:18:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> diff --git a/config.c b/config.c\n> index 6af0244085..9dfc4a79f7 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1847,8 +1847,22 @@ static int git_config_from_blob_ref(config_fn_t fn,\n> const char *git_system_config(void)\n> {\n> \tstatic const char *system_wide;\n> -\tif (!system_wide)\n> -\t\tsystem_wide = system_path(ETC_GITCONFIG);\n> +\n> +\tif (!system_wide) {\n> +\t\tsystem_wide = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n> +\t\tif (system_wide) {\n> +\t\t\t/*\n> +\t\t\t * If GIT_CONFIG_SYSTEM is set, it overrides the\n> +\t\t\t * /etc/gitconfig. Furthermore, the file must exist in\n> +\t\t\t * order to prevent any typos by the user.\n> +\t\t\t */\n> +\t\t\tif (access(system_wide, R_OK))\n> +\t\t\t\tdie(_(\"cannot access '%s'\"), system_wide);\n> +\t\t} else {\n> +\t\t\tsystem_wide = system_path(ETC_GITCONFIG);\n> +\t\t}\n> +\t}\n> +\n> \treturn system_wide;\n> }\n\nI wrote the design to truly special case \"/dev/null\" at the pathname\nstring level, and we do not even try to open/read from a file when\n\"/dev/null\" is given, so that we can even allow \"/dev/null\" to be\nmissing in a funny embedded or containerized environment, but that\ndoes not seem to be what is going on here.\n\nI do not think \"access()\" here is a good idea; the result we get\nhere may not match what happens when we actually try to open the\npath. Just remembering if we got the system_wide value from the\nGIT_CONFIG_SYSTEM, and change what happens when the open fails when\nwe try to use the system-wide configuration depending on that, would\nbe the right approach.\n"},{"id":"421693","messageId":"YHRTUVJZ2m9a0zDa@ncase","threadId":"55455","inReplyTo":"YHB092HN2oVLmqC1@coredump.intra.peff.net","subject":"Re: [PATCH v2 3/3] config: allow overriding of global and system configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-12T14:04:01Z","receivedAt":"2021-04-12T14:04:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Apr 09, 2021 at 11:38:31AM -0400, Jeff King wrote:\n> On Fri, Apr 09, 2021 at 03:43:30PM +0200, Patrick Steinhardt wrote:\n> \n> > In order to have git run in a fully controlled environment without any\n> > misconfiguration, it may be desirable for users or scripts to override\n> > global- and system-level configuration files. We already have a way of\n> > doing this, which is to unset both HOME and XDG_CONFIG_HOME environment\n> > variables and to set `GIT_CONFIG_NOGLOBAL=true`. This is quite kludgy,\n> > and unsetting the first two variables likely has an impact on other\n> > executables spawned by such a script.\n> > \n> > The obvious way to fix this would be to introduce `GIT_CONFIG_NOSYSTEM`\n> > as an equivalent to `GIT_CONFIG_NOGLOBAL`. But in the past, it has\n> \n> I think you have NOSYSTEM and NOGLOBAL mixed up in both paragraphs here?\n> \n> Otherwise the motivation and description here look very good (and I like\n> the overall direction).\n> \n> > diff --git a/Documentation/git.txt b/Documentation/git.txt\n> > index 3a9c44987f..2129629296 100644\n> > --- a/Documentation/git.txt\n> > +++ b/Documentation/git.txt\n> > @@ -670,6 +670,16 @@ for further details.\n> > \tIf this environment variable is set to `0`, git will not prompt\n> > \ton the terminal (e.g., when asking for HTTP authentication).\n> > \n> > +`GIT_CONFIG_GLOBAL`::\n> > +`GIT_CONFIG_SYSTEM`::\n> > +\tTake the configuration from the given files instead from global or\n> > +\tsystem-level configuration files. The files must exist and be readable\n> > +\tby the current user. If `GIT_CONFIG_SYSTEM` is set, `/etc/gitconfig`\n> > +\twill not be read. Likewise, if `GIT_CONFIG_GLOBAL` is set, neither\n> > +\t`$HOME/.gitconfig` nor `$XDG_CONFIG_HOME/git/config` will be read. Can\n> > +\tbe set to `/dev/null` to skip reading configuration files of the\n> > +\trespective level.\n> \n> Makes sense. The reference to `/etc/gitconfig` here may not be accurate,\n> depending on the build parameters. I notice below in the context that we\n> say:\n> \n> > `GIT_CONFIG_NOSYSTEM`::\n> > \tWhether to skip reading settings from the system-wide\n> > \t`$(prefix)/etc/gitconfig` file. This environment variable can\n> \n> which is _also_ not quite right (if $(prefix) is \"/usr\", then the file\n> really is /etc/gitconfig).\n> \n> I think it might be possible to pull the value of the ETC_GITCONFIG\n> Makefile variable into the documentation, so we could probably give the\n> \"real\" value. But I wonder if it would suffice to just say:\n> \n> ...the system config (usually `/etc/gitconfig`) will not be read.\n> \n> Or is that too confusing (it invites the question \"when is it not\n> /etc/gitconfig\")? I guess we could say \"the system config file defined\n> at build-time (usually `/etc/gitconfig`)\", which is perhaps more clear.\n> \n> > @@ -1847,8 +1847,22 @@ static int git_config_from_blob_ref(config_fn_t fn,\n> > const char *git_system_config(void)\n> > {\n> > \tstatic const char *system_wide;\n> > -\tif (!system_wide)\n> > -\t\tsystem_wide = system_path(ETC_GITCONFIG);\n> > +\n> > +\tif (!system_wide) {\n> > +\t\tsystem_wide = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n> \n> I wondered, given the \"const char *\" return values in the last patch, if\n> you might pass back the result of getenv() directly. But you duplicate\n> it here, which is good, as it avoids portability problems hanging on to\n> the result of getenv().\n> \n> > +\t\tif (system_wide) {\n> > +\t\t\t/*\n> > +\t\t\t * If GIT_CONFIG_SYSTEM is set, it overrides the\n> > +\t\t\t * /etc/gitconfig. Furthermore, the file must exist in\n> > +\t\t\t * order to prevent any typos by the user.\n> > +\t\t\t */\n> > +\t\t\tif (access(system_wide, R_OK))\n> > +\t\t\t\tdie(_(\"cannot access '%s'\"), system_wide);\n> > +\t\t} else {\n> > +\t\t\tsystem_wide = system_path(ETC_GITCONFIG);\n> > +\t\t}\n> > +\t}\n> \n> I was on the fence about whether to enforce the \"this file must exist\"\n> property, with respect to the overall design. But seeing how we must\n> actually add extra code here to handle it makes me want to just treat it\n> exactly like the other files.\n> \n> Using a separate access() here is also a TOCTOU race, but I'm pretty\n> sure the existing config code makes the same mistake (and it's not that\n> big a deal in this context).\n> \n> > @@ -1857,8 +1871,20 @@ void git_global_config(const char **user, const char **xdg)\n> > \tstatic const char *user_config, *xdg_config;\n> > \n> > \tif (!user_config) {\n> > -\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n> > -\t\txdg_config = xdg_config_home(\"config\");\n> > +\t\tuser_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n> > +\t\tif (user_config) {\n> > +\t\t\t/*\n> > +\t\t\t * If GIT_CONFIG_GLOBAL is set, then it overrides both\n> > +\t\t\t * the ~/.gitconfig and the XDG configuration file.\n> > +\t\t\t * Furthermore, the file must exist in order to prevent\n> > +\t\t\t * any typos by the user.\n> > +\t\t\t */\n> > +\t\t\tif (access(user_config, R_OK))\n> > +\t\t\t\tdie(_(\"cannot access '%s'\"), user_config);\n> > +\t\t} else {\n> > +\t\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n> > +\t\t\txdg_config = xdg_config_home(\"config\");\n> > +\t\t}\n> > \t}\n> \n> And this looks as I'd expect (but the same comments as above apply, of\n> course).\n> \n> > +test_expect_success 'override global and system config' '\n> > +\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n> > +\n> > +\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n> > +\t[home]\n> > +\t\tconfig = true\n> > +\tEOF\n> > +\tmkdir -p \"$HOME\"/.config/git &&\n> > +\tcat >\"$HOME\"/.config/git/config <<-EOF &&\n> > +\t[xdg]\n> > +\t\tconfig = true\n> > +\tEOF\n> > +\tcat >.git/config <<-EOF &&\n> > +\t[local]\n> > +\t\tconfig = true\n> > +\tEOF\n> > +\tcat >custom-global-config <<-EOF &&\n> > +\t[global]\n> > +\t\tconfig = true\n> > +\tEOF\n> > +\tcat >custom-system-config <<-EOF &&\n> > +\t[system]\n> > +\t\tconfig = true\n> > +\tEOF\n> \n> Minor style nit, but we usually prefer non-interpolating \"\\EOF\" if we\n> don't intend to interpolate within the here-doc. It does look like\n> t1300 has quite a mix of styles, though.\n> \n> > +\tcat >expect <<-EOF &&\n> > +\tglobal\txdg.config=true\n> > +\tglobal\thome.config=true\n> > +\tlocal\tlocal.config=true\n> > +\tEOF\n> > +\tgit config --show-scope --list >output &&\n> > +\ttest_cmp expect output &&\n> > +\n> > +\tsane_unset GIT_CONFIG_NOSYSTEM &&\n> > +\n> > +\tcat >expect <<-EOF &&\n> > +\tsystem\tsystem.config=true\n> > +\tglobal\tglobal.config=true\n> > +\tlocal\tlocal.config=true\n> > +\tEOF\n> > +\tGIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \\\n> > +\t\tgit config --show-scope --list >output &&\n> > +\ttest_cmp expect output &&\n> > +\n> > +\tcat >expect <<-EOF &&\n> > +\tlocal\tlocal.config=true\n> > +\tEOF\n> > +\tGIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null git config --show-scope --list >output &&\n> > +\ttest_cmp expect output\n> > +'\n> \n> And this test covers all of the new stuff we care about. Good.\n> \n> > +test_expect_success 'override global and system config with missing file' '\n> > +\tsane_unset GIT_CONFIG_NOSYSTEM &&\n> > +\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist git version &&\n> > +\ttest_must_fail env GIT_CONFIG_SYSTEM=does-not-exist git version &&\n> > +\tGIT_CONFIG_NOSYSTEM=true GIT_CONFIG_SYSTEM=does-not-exist git version\n> > +'\n> \n> Makes sense to test given the patch, though if we rip out the \"missing\"\n> check, then obviously this goes away.\n> \n> > +test_expect_success 'write to overridden global and system config' '\n> \n> Hmm. I hadn't really considered _writing_ to these files (after all, you\n> can just use \"git config --file\" to do so). I guess it is consistent,\n> and would probably be more work (and more error-prone) to try to\n> distinguish reading versus writing in the code.\n> \n> > +\tcat >expect < > +[config]\n> > +\tkey = value\n> > +EOF\n> \n> No \"<<-EOF\" here to fix the indentation?\n> \n> > +\ttest_must_fail env GIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n> > +\ttouch write-to-global &&\n> > +\tGIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n> > +\ttest_cmp expect write-to-global &&\n> \n> In the writing case, the \"must exist\" thing makes it even weirder, since\n> we might be intending to create the file! If we leave in the writing,\n> that makes me even more convinced that we should drop the \"must exist\"\n> check.\n> \n> -Peff\n\nI do agree that for the writing side it's limiting to require the file\nto exist. The question is really what's gained by it. The worst thing\nthat could happen is that the user writes to a file he didn't intend to\nwrite to -- this can happen regardless of whether or not the file\nexists, and in fact the worse case is where we overwrite values of a\nfile which the user didn't intend to overwrite. The case where we create\na new file by accident doesn't seem to be that interesting to me.\n\nIn any case, the user would probably use `git config -f` anyway. Which\nraises the question whether it's sensible to allow writing to the file\nin the first place. And for the sake of scripts, I lean towards \"yes\":\nthe dev can set up envvars once, and then use the config for both\nreading and writing.\n\nOn the reading side, I can relate with Junio's argument that there's now\ntwo possibilities for typos: once in the envvar, and once in the file\npath. But given that git won't check the envvar's key for typos, we\ncannot really reduce the chance for typos down to zero anyway.\n\nSo I do tend towards just allowing for the file to not exist: when\nreading, we silentely ignore it, and when writing we create it.\n\nPatrick\n"},{"id":"421694","messageId":"34bdbc27d618d7467d2caf6844d8c06bdcb8545b.1618238567.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1618238567.git.ps@pks.im","subject":"[PATCH v3 1/3] config: rename `git_etc_config()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-12T14:46:45Z","receivedAt":"2021-04-12T14:46:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `git_etc_gitconfig()` function retrieves the system-level path of\nthe configuration file. We're about to introduce a way to override it\nvia an environment variable, at which point the name of this function\nwould start to become misleading.\n\nRename the function to `git_system_config()` as a preparatory step.\nWhile at it, the function is also refactored to pass memory ownership to\nthe caller. This is done to better match semantics of\n`git_global_config()`, which is going to be introduced in the next\ncommit.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/config.c | 2 +-\n config.c | 18 ++++++++----------\n config.h | 3 ++-\n 3 files changed, 11 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex f71fa39b38..02ed0b3fe7 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -695,7 +695,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \telse if (use_system_config) {\n-\t\tgiven_config_source.file = git_etc_gitconfig();\n+\t\tgiven_config_source.file = git_system_config();\n \t\tgiven_config_source.scope = CONFIG_SCOPE_SYSTEM;\n \t} else if (use_local_config) {\n \t\tgiven_config_source.file = git_pathdup(\"config\");\ndiff --git a/config.c b/config.c\nindex 6428393a41..8c83669cce 100644\n--- a/config.c\n+++ b/config.c\n@@ -1844,12 +1844,9 @@ static int git_config_from_blob_ref(config_fn_t fn,\n \treturn git_config_from_blob_oid(fn, name, &oid, data);\n }\n \n-const char *git_etc_gitconfig(void)\n+char *git_system_config(void)\n {\n-\tstatic const char *system_wide;\n-\tif (!system_wide)\n-\t\tsystem_wide = system_path(ETC_GITCONFIG);\n-\treturn system_wide;\n+\treturn system_path(ETC_GITCONFIG);\n }\n \n /*\n@@ -1883,6 +1880,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\t\t\t config_fn_t fn, void *data)\n {\n \tint ret = 0;\n+\tchar *system_config = git_system_config();\n \tchar *xdg_config = xdg_config_home(\"config\");\n \tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n \tchar *repo_config;\n@@ -1896,11 +1894,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\trepo_config = NULL;\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n-\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,\n-\t\t\t\t\t\t opts->system_gently ?\n-\t\t\t\t\t\t ACCESS_EACCES_OK : 0))\n-\t\tret += git_config_from_file(fn, git_etc_gitconfig(),\n-\t\t\t\t\t data);\n+\tif (system_config && !access_or_die(system_config, R_OK,\n+\t\t\t\t\t opts->system_gently ?\n+\t\t\t\t\t ACCESS_EACCES_OK : 0))\n+\t\tret += git_config_from_file(fn, system_config, data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n \tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n@@ -1927,6 +1924,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\tdie(_(\"unable to parse command-line config\"));\n \n \tcurrent_parsing_scope = prev_parsing_scope;\n+\tfree(system_config);\n \tfree(xdg_config);\n \tfree(user_config);\n \tfree(repo_config);\ndiff --git a/config.h b/config.h\nindex 19a9adbaa9..2be8fa1880 100644\n--- a/config.h\n+++ b/config.h\n@@ -318,7 +318,6 @@ int git_config_rename_section(const char *, const char *);\n int git_config_rename_section_in_file(const char *, const char *, const char *);\n int git_config_copy_section(const char *, const char *);\n int git_config_copy_section_in_file(const char *, const char *, const char *);\n-const char *git_etc_gitconfig(void);\n int git_env_bool(const char *, int);\n unsigned long git_env_ulong(const char *, unsigned long);\n int git_config_system(void);\n@@ -327,6 +326,8 @@ int config_error_nonbool(const char *);\n #define config_error_nonbool(s) (config_error_nonbool(s), const_error())\n #endif\n \n+char *git_system_config(void);\n+\n int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n \n enum config_scope current_config_scope(void);\n-- \n2.31.1\n\n"},{"id":"421695","messageId":"cover.1618238567.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1617975637.git.ps@pks.im","subject":"[PATCH v3 0/3] config: allow overriding global/system config","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-12T14:46:37Z","receivedAt":"2021-04-12T14:46:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the third version of my patch series to provide a way of\noverriding the global system configuration.\n\nChanges compared to v2:\n\n - `/dev/null` is now truly special-cased to skip reading any file\n such that it's possible to use it e.g. in environments where this\n special file does not exist.\n\n - There is no explicit requirement for the given files to exist\n anymore. On the reading side, it will in most cases just be\n ignored (except for e.g. `git config --global --list` and similar,\n where we'd also fail if the normal global config didn't exist). On\n the writing site, we now create the config file it it's missing.\n\n - Both `git_system_config()` and `git_global_config()` now pass\n memory ownership to the caller.\n\nPatrick\n\nPatrick Steinhardt (3):\n config: rename `git_etc_config()`\n config: unify code paths to get global config paths\n config: allow overriding of global and system configuration\n\n Documentation/git-config.txt | 5 +++\n Documentation/git.txt | 10 +++++\n builtin/config.c | 12 ++++--\n config.c | 48 ++++++++++++++++++------\n config.h | 4 +-\n t/t1300-config.sh | 71 ++++++++++++++++++++++++++++++++++++\n 6 files changed, 133 insertions(+), 17 deletions(-)\n\nRange-diff against v2:\n1: da0b8ce6f0 ! 1: 34bdbc27d6 config: rename `git_etc_config()`\n @@ Commit message\n would start to become misleading.\n \n Rename the function to `git_system_config()` as a preparatory step.\n + While at it, the function is also refactored to pass memory ownership to\n + the caller. This is done to better match semantics of\n + `git_global_config()`, which is going to be introduced in the next\n + commit.\n \n Signed-off-by: Patrick Steinhardt \n \n @@ config.c: static int git_config_from_blob_ref(config_fn_t fn,\n }\n \n -const char *git_etc_gitconfig(void)\n -+const char *git_system_config(void)\n ++char *git_system_config(void)\n {\n - \tstatic const char *system_wide;\n - \tif (!system_wide)\n +-\tstatic const char *system_wide;\n +-\tif (!system_wide)\n +-\t\tsystem_wide = system_path(ETC_GITCONFIG);\n +-\treturn system_wide;\n ++\treturn system_path(ETC_GITCONFIG);\n + }\n + \n + /*\n +@@ config.c: static int do_git_config_sequence(const struct config_options *opts,\n + \t\t\t\t config_fn_t fn, void *data)\n + {\n + \tint ret = 0;\n ++\tchar *system_config = git_system_config();\n + \tchar *xdg_config = xdg_config_home(\"config\");\n + \tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n + \tchar *repo_config;\n @@ config.c: static int do_git_config_sequence(const struct config_options *opts,\n \t\trepo_config = NULL;\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n -\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,\n -+\tif (git_config_system() && !access_or_die(git_system_config(), R_OK,\n - \t\t\t\t\t\t opts->system_gently ?\n - \t\t\t\t\t\t ACCESS_EACCES_OK : 0))\n +-\t\t\t\t\t\t opts->system_gently ?\n +-\t\t\t\t\t\t ACCESS_EACCES_OK : 0))\n -\t\tret += git_config_from_file(fn, git_etc_gitconfig(),\n -+\t\tret += git_config_from_file(fn, git_system_config(),\n - \t\t\t\t\t data);\n +-\t\t\t\t\t data);\n ++\tif (system_config && !access_or_die(system_config, R_OK,\n ++\t\t\t\t\t opts->system_gently ?\n ++\t\t\t\t\t ACCESS_EACCES_OK : 0))\n ++\t\tret += git_config_from_file(fn, system_config, data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n + \tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n +@@ config.c: static int do_git_config_sequence(const struct config_options *opts,\n + \t\tdie(_(\"unable to parse command-line config\"));\n + \n + \tcurrent_parsing_scope = prev_parsing_scope;\n ++\tfree(system_config);\n + \tfree(xdg_config);\n + \tfree(user_config);\n + \tfree(repo_config);\n \n ## config.h ##\n @@ config.h: int git_config_rename_section(const char *, const char *);\n @@ config.h: int config_error_nonbool(const char *);\n #define config_error_nonbool(s) (config_error_nonbool(s), const_error())\n #endif\n \n -+const char *git_system_config(void);\n ++char *git_system_config(void);\n +\n int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n \n2: dddc85bcf5 < -: ---------- config: unify code paths to get global config paths\n-: ---------- > 2: 30f18679bd config: unify code paths to get global config paths\n3: 272a3b31aa ! 3: af663640ae config: allow overriding of global and system configuration\n @@ Commit message\n and unsetting the first two variables likely has an impact on other\n executables spawned by such a script.\n \n - The obvious way to fix this would be to introduce `GIT_CONFIG_NOSYSTEM`\n - as an equivalent to `GIT_CONFIG_NOGLOBAL`. But in the past, it has\n + The obvious way to fix this would be to introduce `GIT_CONFIG_NOGLOBAL`\n + as an equivalent to `GIT_CONFIG_NOSYSTEM`. But in the past, it has\n turned out that this design is inflexible: we cannot test system-level\n parsing of the git configuration in our test harness because there is no\n way to change its location, so all tests run with `GIT_CONFIG_NOSYSTEM`\n @@ Commit message\n - If unset, git continues to use the usual locations.\n \n - If set to a specific path, we skip reading the normal\n - configuration files and instead take the path. This path must\n - exist and be readable to ensure that the user didn't typo.\n + configuration files and instead take the path.\n \n - If set to `/dev/null`, we do not load either global- or\n system-level configuration at all.\n @@ Documentation/git.txt: for further details.\n +`GIT_CONFIG_GLOBAL`::\n +`GIT_CONFIG_SYSTEM`::\n +\tTake the configuration from the given files instead from global or\n -+\tsystem-level configuration files. The files must exist and be readable\n -+\tby the current user. If `GIT_CONFIG_SYSTEM` is set, `/etc/gitconfig`\n ++\tsystem-level configuration files. If `GIT_CONFIG_SYSTEM` is set, the\n ++\tsystem config file defined at build time (usually `/etc/gitconfig`)\n +\twill not be read. Likewise, if `GIT_CONFIG_GLOBAL` is set, neither\n +\t`$HOME/.gitconfig` nor `$XDG_CONFIG_HOME/git/config` will be read. Can\n +\tbe set to `/dev/null` to skip reading configuration files of the\n @@ Documentation/git.txt: for further details.\n \tWhether to skip reading settings from the system-wide\n \t`$(prefix)/etc/gitconfig` file. This environment variable can\n \n + ## builtin/config.c ##\n +@@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)\n + \t\tchar *user_config, *xdg_config;\n + \n + \t\tgit_global_config(&user_config, &xdg_config);\n +-\t\tif (!user_config)\n ++\t\tif (!user_config) {\n ++\t\t\tif (getenv(\"GIT_CONFIG_GLOBAL\"))\n ++\t\t\t\tdie(_(\"GIT_CONFIG_GLOBAL=/dev/null set\"));\n ++\n + \t\t\t/*\n + \t\t\t * It is unknown if HOME/.gitconfig exists, so\n + \t\t\t * we do not know if we should write to XDG\n +@@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)\n + \t\t\t * is set and points at a sane location.\n + \t\t\t */\n + \t\t\tdie(_(\"$HOME not set\"));\n ++\t\t}\n + \n + \t\tgiven_config_source.scope = CONFIG_SCOPE_GLOBAL;\n + \n +\n ## config.c ##\n @@ config.c: static int git_config_from_blob_ref(config_fn_t fn,\n - const char *git_system_config(void)\n + \n + char *git_system_config(void)\n {\n - \tstatic const char *system_wide;\n --\tif (!system_wide)\n --\t\tsystem_wide = system_path(ETC_GITCONFIG);\n -+\n -+\tif (!system_wide) {\n -+\t\tsystem_wide = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n -+\t\tif (system_wide) {\n -+\t\t\t/*\n -+\t\t\t * If GIT_CONFIG_SYSTEM is set, it overrides the\n -+\t\t\t * /etc/gitconfig. Furthermore, the file must exist in\n -+\t\t\t * order to prevent any typos by the user.\n -+\t\t\t */\n -+\t\t\tif (access(system_wide, R_OK))\n -+\t\t\t\tdie(_(\"cannot access '%s'\"), system_wide);\n -+\t\t} else {\n -+\t\t\tsystem_wide = system_path(ETC_GITCONFIG);\n -+\t\t}\n ++\tchar *system_config = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n ++\tif (system_config) {\n ++\t\tif (!strcmp(system_config, \"/dev/null\"))\n ++\t\t\tFREE_AND_NULL(system_config);\n ++\t\treturn system_config;\n +\t}\n -+\n - \treturn system_wide;\n + \treturn system_path(ETC_GITCONFIG);\n }\n \n -@@ config.c: void git_global_config(const char **user, const char **xdg)\n - \tstatic const char *user_config, *xdg_config;\n +-void git_global_config(char **user_config, char **xdg_config)\n ++void git_global_config(char **user_out, char **xdg_out)\n + {\n +-\t*user_config = expand_user_path(\"~/.gitconfig\", 0);\n +-\t*xdg_config = xdg_config_home(\"config\");\n ++\tchar *user_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n ++\tchar *xdg_config = NULL;\n ++\n ++\tif (user_config) {\n ++\t\tif (!strcmp(user_config, \"/dev/null\"))\n ++\t\t\tFREE_AND_NULL(user_config);\n ++\t\txdg_config = NULL;\n ++\t} else {\n ++\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n ++\t\txdg_config = xdg_config_home(\"config\");\n ++\t}\n ++\n ++\t*user_out = user_config;\n ++\t*xdg_out = xdg_config;\n + }\n \n - \tif (!user_config) {\n --\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n --\t\txdg_config = xdg_config_home(\"config\");\n -+\t\tuser_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n -+\t\tif (user_config) {\n -+\t\t\t/*\n -+\t\t\t * If GIT_CONFIG_GLOBAL is set, then it overrides both\n -+\t\t\t * the ~/.gitconfig and the XDG configuration file.\n -+\t\t\t * Furthermore, the file must exist in order to prevent\n -+\t\t\t * any typos by the user.\n -+\t\t\t */\n -+\t\t\tif (access(user_config, R_OK))\n -+\t\t\t\tdie(_(\"cannot access '%s'\"), user_config);\n -+\t\t} else {\n -+\t\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n -+\t\t\txdg_config = xdg_config_home(\"config\");\n -+\t\t}\n - \t}\n - \n - \t*user = user_config;\n + /*\n \n ## t/t1300-config.sh ##\n @@ t/t1300-config.sh: test_expect_success '--show-scope with --show-origin' '\n @@ t/t1300-config.sh: test_expect_success '--show-scope with --show-origin' '\n +\n +test_expect_success 'override global and system config with missing file' '\n +\tsane_unset GIT_CONFIG_NOSYSTEM &&\n -+\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist git version &&\n -+\ttest_must_fail env GIT_CONFIG_SYSTEM=does-not-exist git version &&\n -+\tGIT_CONFIG_NOSYSTEM=true GIT_CONFIG_SYSTEM=does-not-exist git version\n ++\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list >actual &&\n ++\ttest_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list >actual &&\n ++\tGIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version\n +'\n +\n +test_expect_success 'write to overridden global and system config' '\n @@ t/t1300-config.sh: test_expect_success '--show-scope with --show-origin' '\n +\tkey = value\n +EOF\n +\n -+\ttest_must_fail env GIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n -+\ttouch write-to-global &&\n +\tGIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n +\ttest_cmp expect write-to-global &&\n +\n -+\ttest_must_fail env GIT_CONFIG_SYSTEM=write-to-system git config --system config.key value &&\n -+\ttouch write-to-system &&\n +\tGIT_CONFIG_SYSTEM=write-to-system git config --system config.key value &&\n +\ttest_cmp expect write-to-system\n +'\n-- \n2.31.1\n\n"},{"id":"421696","messageId":"aa0f2957e6f1baf7bd600ad43d34664af51c9c25.1618238153.git.ps@pks.im","threadId":"55455","inReplyTo":"a23382059bb57022dd1e40d1c2c9a11307b0ff3b.1617891426.git.ps@pks.im","subject":"[PATCH v3] config: allow overriding of global and system configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-12T14:46:41Z","receivedAt":"2021-04-12T14:46:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In order to have git run in a fully controlled environment without any\nmisconfiguration, it may be desirable for users or scripts to override\nglobal- and system-level configuration files. We already have a way of\ndoing this, which is to unset both HOME and XDG_CONFIG_HOME environment\nvariables and to set `GIT_CONFIG_NOGLOBAL=true`. This is quite kludgy,\nand unsetting the first two variables likely has an impact on other\nexecutables spawned by such a script.\n\nThe obvious way to fix this would be to introduce `GIT_CONFIG_NOGLOBAL`\nas an equivalent to `GIT_CONFIG_NOSYSTEM`. But in the past, it has\nturned out that this design is inflexible: we cannot test system-level\nparsing of the git configuration in our test harness because there is no\nway to change its location, so all tests run with `GIT_CONFIG_NOSYSTEM`\nset.\n\nInstead of doing the same mistake with `GIT_CONFIG_NOGLOBAL`, introduce\ntwo new variables `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM`:\n\n - If unset, git continues to use the usual locations.\n\n - If set to a specific path, we skip reading the normal\n configuration files and instead take the path. This path must\n exist and be readable to ensure that the user didn't typo.\n\n - If set to `/dev/null`, we do not load either global- or\n system-level configuration at all.\n\nThis implements the usecase where we want to execute code in a sanitized\nenvironment without any potential misconfigurations via `/dev/null`, but\nis more flexible and allows for more usecases than simply adding\n`GIT_CONFIG_NOGLOBAL`.\n\nSigned-off-by: Patrick Steinhardt \n---\nRange-diff against v2:\n1: da0b8ce6f0 < -: ---------- config: rename `git_etc_config()`\n2: dddc85bcf5 < -: ---------- config: unify code paths to get global config paths\n3: 272a3b31aa ! 1: aa0f2957e6 config: allow overriding of global and system configuration\n @@ Commit message\n and unsetting the first two variables likely has an impact on other\n executables spawned by such a script.\n \n - The obvious way to fix this would be to introduce `GIT_CONFIG_NOSYSTEM`\n - as an equivalent to `GIT_CONFIG_NOGLOBAL`. But in the past, it has\n + The obvious way to fix this would be to introduce `GIT_CONFIG_NOGLOBAL`\n + as an equivalent to `GIT_CONFIG_NOSYSTEM`. But in the past, it has\n turned out that this design is inflexible: we cannot test system-level\n parsing of the git configuration in our test harness because there is no\n way to change its location, so all tests run with `GIT_CONFIG_NOSYSTEM`\n @@ Documentation/git.txt: for further details.\n +`GIT_CONFIG_GLOBAL`::\n +`GIT_CONFIG_SYSTEM`::\n +\tTake the configuration from the given files instead from global or\n -+\tsystem-level configuration files. The files must exist and be readable\n -+\tby the current user. If `GIT_CONFIG_SYSTEM` is set, `/etc/gitconfig`\n ++\tsystem-level configuration files. If `GIT_CONFIG_SYSTEM` is set, the\n ++\tsystem config file defined at build time (usually `/etc/gitconfig`)\n +\twill not be read. Likewise, if `GIT_CONFIG_GLOBAL` is set, neither\n +\t`$HOME/.gitconfig` nor `$XDG_CONFIG_HOME/git/config` will be read. Can\n +\tbe set to `/dev/null` to skip reading configuration files of the\n @@ Documentation/git.txt: for further details.\n \tWhether to skip reading settings from the system-wide\n \t`$(prefix)/etc/gitconfig` file. This environment variable can\n \n + ## builtin/config.c ##\n +@@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)\n + \t\tchar *user_config, *xdg_config;\n + \n + \t\tgit_global_config(&user_config, &xdg_config);\n +-\t\tif (!user_config)\n ++\t\tif (!user_config) {\n ++\t\t\tif (!strcmp(getenv(\"GIT_CONFIG_GLOBAL\"), \"/dev/null\"))\n ++\t\t\t\tdie(_(\"GIT_CONFIG_GLOBAL=/dev/null set\"));\n ++\n + \t\t\t/*\n + \t\t\t * It is unknown if HOME/.gitconfig exists, so\n + \t\t\t * we do not know if we should write to XDG\n +@@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)\n + \t\t\t * is set and points at a sane location.\n + \t\t\t */\n + \t\t\tdie(_(\"$HOME not set\"));\n ++\t\t}\n + \n + \t\tgiven_config_source.scope = CONFIG_SCOPE_GLOBAL;\n + \n +\n ## config.c ##\n @@ config.c: static int git_config_from_blob_ref(config_fn_t fn,\n - const char *git_system_config(void)\n + \n + char *git_system_config(void)\n {\n - \tstatic const char *system_wide;\n --\tif (!system_wide)\n --\t\tsystem_wide = system_path(ETC_GITCONFIG);\n -+\n -+\tif (!system_wide) {\n -+\t\tsystem_wide = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n -+\t\tif (system_wide) {\n -+\t\t\t/*\n -+\t\t\t * If GIT_CONFIG_SYSTEM is set, it overrides the\n -+\t\t\t * /etc/gitconfig. Furthermore, the file must exist in\n -+\t\t\t * order to prevent any typos by the user.\n -+\t\t\t */\n -+\t\t\tif (access(system_wide, R_OK))\n -+\t\t\t\tdie(_(\"cannot access '%s'\"), system_wide);\n -+\t\t} else {\n -+\t\t\tsystem_wide = system_path(ETC_GITCONFIG);\n -+\t\t}\n ++\tchar *system_config = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n ++\tif (system_config) {\n ++\t\tif (!strcmp(system_config, \"/dev/null\"))\n ++\t\t\tFREE_AND_NULL(system_config);\n ++\t\treturn system_config;\n +\t}\n -+\n - \treturn system_wide;\n + \treturn system_path(ETC_GITCONFIG);\n }\n \n -@@ config.c: void git_global_config(const char **user, const char **xdg)\n - \tstatic const char *user_config, *xdg_config;\n +-void git_global_config(char **user_config, char **xdg_config)\n ++void git_global_config(char **user_out, char **xdg_out)\n + {\n +-\t*user_config = expand_user_path(\"~/.gitconfig\", 0);\n +-\t*xdg_config = xdg_config_home(\"config\");\n ++\tchar *user_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n ++\tchar *xdg_config = NULL;\n ++\n ++\tif (user_config) {\n ++\t\tif (!strcmp(user_config, \"/dev/null\"))\n ++\t\t\tFREE_AND_NULL(user_config);\n ++\t\txdg_config = NULL;\n ++\t} else {\n ++\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n ++\t\txdg_config = xdg_config_home(\"config\");\n ++\t}\n ++\n ++\t*user_out = user_config;\n ++\t*xdg_out = xdg_config;\n + }\n \n - \tif (!user_config) {\n --\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n --\t\txdg_config = xdg_config_home(\"config\");\n -+\t\tuser_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n -+\t\tif (user_config) {\n -+\t\t\t/*\n -+\t\t\t * If GIT_CONFIG_GLOBAL is set, then it overrides both\n -+\t\t\t * the ~/.gitconfig and the XDG configuration file.\n -+\t\t\t * Furthermore, the file must exist in order to prevent\n -+\t\t\t * any typos by the user.\n -+\t\t\t */\n -+\t\t\tif (access(user_config, R_OK))\n -+\t\t\t\tdie(_(\"cannot access '%s'\"), user_config);\n -+\t\t} else {\n -+\t\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n -+\t\t\txdg_config = xdg_config_home(\"config\");\n -+\t\t}\n - \t}\n - \n - \t*user = user_config;\n + /*\n \n ## t/t1300-config.sh ##\n @@ t/t1300-config.sh: test_expect_success '--show-scope with --show-origin' '\n @@ t/t1300-config.sh: test_expect_success '--show-scope with --show-origin' '\n +\n +test_expect_success 'override global and system config with missing file' '\n +\tsane_unset GIT_CONFIG_NOSYSTEM &&\n -+\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist git version &&\n -+\ttest_must_fail env GIT_CONFIG_SYSTEM=does-not-exist git version &&\n -+\tGIT_CONFIG_NOSYSTEM=true GIT_CONFIG_SYSTEM=does-not-exist git version\n ++\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list >actual &&\n ++\ttest_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list >actual &&\n ++\tGIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version\n +'\n +\n +test_expect_success 'write to overridden global and system config' '\n @@ t/t1300-config.sh: test_expect_success '--show-scope with --show-origin' '\n +\tkey = value\n +EOF\n +\n -+\ttest_must_fail env GIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n -+\ttouch write-to-global &&\n +\tGIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n +\ttest_cmp expect write-to-global &&\n +\n -+\ttest_must_fail env GIT_CONFIG_SYSTEM=write-to-system git config --system config.key value &&\n -+\ttouch write-to-system &&\n +\tGIT_CONFIG_SYSTEM=write-to-system git config --system config.key value &&\n +\ttest_cmp expect write-to-system\n +'\n\n Documentation/git-config.txt | 5 +++\n Documentation/git.txt | 10 +++++\n builtin/config.c | 6 ++-\n config.c | 24 ++++++++++--\n t/t1300-config.sh | 71 ++++++++++++++++++++++++++++++++++++\n 5 files changed, 112 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 4b4cc5c5e8..5cddadafd2 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -340,6 +340,11 @@ 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_GLOBAL::\n+GIT_CONFIG_SYSTEM::\n+\tTake the configuration from the given files instead from global or\n+\tsystem-level configuration. See linkgit:git[1] for details.\n+\n GIT_CONFIG_NOSYSTEM::\n \tWhether to skip reading settings from the system-wide\n \t$(prefix)/etc/gitconfig file. See linkgit:git[1] for details.\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 3a9c44987f..380422a6a9 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -670,6 +670,16 @@ for further details.\n \tIf this environment variable is set to `0`, git will not prompt\n \ton the terminal (e.g., when asking for HTTP authentication).\n \n+`GIT_CONFIG_GLOBAL`::\n+`GIT_CONFIG_SYSTEM`::\n+\tTake the configuration from the given files instead from global or\n+\tsystem-level configuration files. If `GIT_CONFIG_SYSTEM` is set, the\n+\tsystem config file defined at build time (usually `/etc/gitconfig`)\n+\twill not be read. Likewise, if `GIT_CONFIG_GLOBAL` is set, neither\n+\t`$HOME/.gitconfig` nor `$XDG_CONFIG_HOME/git/config` will be read. Can\n+\tbe set to `/dev/null` to skip reading configuration files of the\n+\trespective level.\n+\n `GIT_CONFIG_NOSYSTEM`::\n \tWhether to skip reading settings from the system-wide\n \t`$(prefix)/etc/gitconfig` file. This environment variable can\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 865fddd6ce..a577a53af7 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -674,7 +674,10 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tchar *user_config, *xdg_config;\n \n \t\tgit_global_config(&user_config, &xdg_config);\n-\t\tif (!user_config)\n+\t\tif (!user_config) {\n+\t\t\tif (!strcmp(getenv(\"GIT_CONFIG_GLOBAL\"), \"/dev/null\"))\n+\t\t\t\tdie(_(\"GIT_CONFIG_GLOBAL=/dev/null set\"));\n+\n \t\t\t/*\n \t\t\t * It is unknown if HOME/.gitconfig exists, so\n \t\t\t * we do not know if we should write to XDG\n@@ -682,6 +685,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\t * is set and points at a sane location.\n \t\t\t */\n \t\t\tdie(_(\"$HOME not set\"));\n+\t\t}\n \n \t\tgiven_config_source.scope = CONFIG_SCOPE_GLOBAL;\n \ndiff --git a/config.c b/config.c\nindex ebff58aa57..ed46eda997 100644\n--- a/config.c\n+++ b/config.c\n@@ -1846,13 +1846,31 @@ static int git_config_from_blob_ref(config_fn_t fn,\n \n char *git_system_config(void)\n {\n+\tchar *system_config = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n+\tif (system_config) {\n+\t\tif (!strcmp(system_config, \"/dev/null\"))\n+\t\t\tFREE_AND_NULL(system_config);\n+\t\treturn system_config;\n+\t}\n \treturn system_path(ETC_GITCONFIG);\n }\n \n-void git_global_config(char **user_config, char **xdg_config)\n+void git_global_config(char **user_out, char **xdg_out)\n {\n-\t*user_config = expand_user_path(\"~/.gitconfig\", 0);\n-\t*xdg_config = xdg_config_home(\"config\");\n+\tchar *user_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n+\tchar *xdg_config = NULL;\n+\n+\tif (user_config) {\n+\t\tif (!strcmp(user_config, \"/dev/null\"))\n+\t\t\tFREE_AND_NULL(user_config);\n+\t\txdg_config = NULL;\n+\t} else {\n+\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n+\t\txdg_config = xdg_config_home(\"config\");\n+\t}\n+\n+\t*user_out = user_config;\n+\t*xdg_out = xdg_config;\n }\n \n /*\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex e0dd5d65ce..17f1b78c01 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -2059,6 +2059,77 @@ test_expect_success '--show-scope with --show-origin' '\n \ttest_cmp expect output\n '\n \n+test_expect_success 'override global and system config' '\n+\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n+\n+\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n+\t[home]\n+\t\tconfig = true\n+\tEOF\n+\tmkdir -p \"$HOME\"/.config/git &&\n+\tcat >\"$HOME\"/.config/git/config <<-EOF &&\n+\t[xdg]\n+\t\tconfig = true\n+\tEOF\n+\tcat >.git/config <<-EOF &&\n+\t[local]\n+\t\tconfig = true\n+\tEOF\n+\tcat >custom-global-config <<-EOF &&\n+\t[global]\n+\t\tconfig = true\n+\tEOF\n+\tcat >custom-system-config <<-EOF &&\n+\t[system]\n+\t\tconfig = true\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\tglobal\txdg.config=true\n+\tglobal\thome.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\n+\tcat >expect <<-EOF &&\n+\tsystem\tsystem.config=true\n+\tglobal\tglobal.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \\\n+\t\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tcat >expect <<-EOF &&\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null git config --show-scope --list >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'override global and system config with missing file' '\n+\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list >actual &&\n+\ttest_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list >actual &&\n+\tGIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version\n+'\n+\n+test_expect_success 'write to overridden global and system config' '\n+\tcat >expect <\n---\n builtin/config.c | 4 ++--\n config.c | 12 ++++++++++--\n config.h | 1 +\n 3 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 02ed0b3fe7..865fddd6ce 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -671,9 +671,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (use_global_config) {\n-\t\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n-\t\tchar *xdg_config = xdg_config_home(\"config\");\n+\t\tchar *user_config, *xdg_config;\n \n+\t\tgit_global_config(&user_config, &xdg_config);\n \t\tif (!user_config)\n \t\t\t/*\n \t\t\t * It is unknown if HOME/.gitconfig exists, so\ndiff --git a/config.c b/config.c\nindex 8c83669cce..ebff58aa57 100644\n--- a/config.c\n+++ b/config.c\n@@ -1849,6 +1849,12 @@ char *git_system_config(void)\n \treturn system_path(ETC_GITCONFIG);\n }\n \n+void git_global_config(char **user_config, char **xdg_config)\n+{\n+\t*user_config = expand_user_path(\"~/.gitconfig\", 0);\n+\t*xdg_config = xdg_config_home(\"config\");\n+}\n+\n /*\n * Parse environment variable 'k' as a boolean (in various\n * possible spellings); if missing, use the default value 'def'.\n@@ -1881,8 +1887,8 @@ static int do_git_config_sequence(const struct config_options *opts,\n {\n \tint ret = 0;\n \tchar *system_config = git_system_config();\n-\tchar *xdg_config = xdg_config_home(\"config\");\n-\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n+\tchar *xdg_config = NULL;\n+\tchar *user_config = NULL;\n \tchar *repo_config;\n \tenum config_scope prev_parsing_scope = current_parsing_scope;\n \n@@ -1900,6 +1906,8 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\tret += git_config_from_file(fn, system_config, data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n+\tgit_global_config(&user_config, &xdg_config);\n+\n \tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n \t\tret += git_config_from_file(fn, xdg_config, data);\n \ndiff --git a/config.h b/config.h\nindex 2be8fa1880..9038538ffd 100644\n--- a/config.h\n+++ b/config.h\n@@ -327,6 +327,7 @@ int config_error_nonbool(const char *);\n #endif\n \n char *git_system_config(void);\n+void git_global_config(char **user, char **xdg);\n \n int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n \n-- \n2.31.1\n\n"},{"id":"421698","messageId":"af663640ae25a95fa56adf32baf4c2e197f3eea2.1618238567.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1618238567.git.ps@pks.im","subject":"[PATCH v3 3/3] config: allow overriding of global and system configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-12T14:46:54Z","receivedAt":"2021-04-12T14:46:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In order to have git run in a fully controlled environment without any\nmisconfiguration, it may be desirable for users or scripts to override\nglobal- and system-level configuration files. We already have a way of\ndoing this, which is to unset both HOME and XDG_CONFIG_HOME environment\nvariables and to set `GIT_CONFIG_NOGLOBAL=true`. This is quite kludgy,\nand unsetting the first two variables likely has an impact on other\nexecutables spawned by such a script.\n\nThe obvious way to fix this would be to introduce `GIT_CONFIG_NOGLOBAL`\nas an equivalent to `GIT_CONFIG_NOSYSTEM`. But in the past, it has\nturned out that this design is inflexible: we cannot test system-level\nparsing of the git configuration in our test harness because there is no\nway to change its location, so all tests run with `GIT_CONFIG_NOSYSTEM`\nset.\n\nInstead of doing the same mistake with `GIT_CONFIG_NOGLOBAL`, introduce\ntwo new variables `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM`:\n\n - If unset, git continues to use the usual locations.\n\n - If set to a specific path, we skip reading the normal\n configuration files and instead take the path.\n\n - If set to `/dev/null`, we do not load either global- or\n system-level configuration at all.\n\nThis implements the usecase where we want to execute code in a sanitized\nenvironment without any potential misconfigurations via `/dev/null`, but\nis more flexible and allows for more usecases than simply adding\n`GIT_CONFIG_NOGLOBAL`.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/git-config.txt | 5 +++\n Documentation/git.txt | 10 +++++\n builtin/config.c | 6 ++-\n config.c | 24 ++++++++++--\n t/t1300-config.sh | 71 ++++++++++++++++++++++++++++++++++++\n 5 files changed, 112 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 4b4cc5c5e8..5cddadafd2 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -340,6 +340,11 @@ 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_GLOBAL::\n+GIT_CONFIG_SYSTEM::\n+\tTake the configuration from the given files instead from global or\n+\tsystem-level configuration. See linkgit:git[1] for details.\n+\n GIT_CONFIG_NOSYSTEM::\n \tWhether to skip reading settings from the system-wide\n \t$(prefix)/etc/gitconfig file. See linkgit:git[1] for details.\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 3a9c44987f..380422a6a9 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -670,6 +670,16 @@ for further details.\n \tIf this environment variable is set to `0`, git will not prompt\n \ton the terminal (e.g., when asking for HTTP authentication).\n \n+`GIT_CONFIG_GLOBAL`::\n+`GIT_CONFIG_SYSTEM`::\n+\tTake the configuration from the given files instead from global or\n+\tsystem-level configuration files. If `GIT_CONFIG_SYSTEM` is set, the\n+\tsystem config file defined at build time (usually `/etc/gitconfig`)\n+\twill not be read. Likewise, if `GIT_CONFIG_GLOBAL` is set, neither\n+\t`$HOME/.gitconfig` nor `$XDG_CONFIG_HOME/git/config` will be read. Can\n+\tbe set to `/dev/null` to skip reading configuration files of the\n+\trespective level.\n+\n `GIT_CONFIG_NOSYSTEM`::\n \tWhether to skip reading settings from the system-wide\n \t`$(prefix)/etc/gitconfig` file. This environment variable can\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 865fddd6ce..9104816459 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -674,7 +674,10 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tchar *user_config, *xdg_config;\n \n \t\tgit_global_config(&user_config, &xdg_config);\n-\t\tif (!user_config)\n+\t\tif (!user_config) {\n+\t\t\tif (getenv(\"GIT_CONFIG_GLOBAL\"))\n+\t\t\t\tdie(_(\"GIT_CONFIG_GLOBAL=/dev/null set\"));\n+\n \t\t\t/*\n \t\t\t * It is unknown if HOME/.gitconfig exists, so\n \t\t\t * we do not know if we should write to XDG\n@@ -682,6 +685,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\t * is set and points at a sane location.\n \t\t\t */\n \t\t\tdie(_(\"$HOME not set\"));\n+\t\t}\n \n \t\tgiven_config_source.scope = CONFIG_SCOPE_GLOBAL;\n \ndiff --git a/config.c b/config.c\nindex ebff58aa57..ed46eda997 100644\n--- a/config.c\n+++ b/config.c\n@@ -1846,13 +1846,31 @@ static int git_config_from_blob_ref(config_fn_t fn,\n \n char *git_system_config(void)\n {\n+\tchar *system_config = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n+\tif (system_config) {\n+\t\tif (!strcmp(system_config, \"/dev/null\"))\n+\t\t\tFREE_AND_NULL(system_config);\n+\t\treturn system_config;\n+\t}\n \treturn system_path(ETC_GITCONFIG);\n }\n \n-void git_global_config(char **user_config, char **xdg_config)\n+void git_global_config(char **user_out, char **xdg_out)\n {\n-\t*user_config = expand_user_path(\"~/.gitconfig\", 0);\n-\t*xdg_config = xdg_config_home(\"config\");\n+\tchar *user_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n+\tchar *xdg_config = NULL;\n+\n+\tif (user_config) {\n+\t\tif (!strcmp(user_config, \"/dev/null\"))\n+\t\t\tFREE_AND_NULL(user_config);\n+\t\txdg_config = NULL;\n+\t} else {\n+\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n+\t\txdg_config = xdg_config_home(\"config\");\n+\t}\n+\n+\t*user_out = user_config;\n+\t*xdg_out = xdg_config;\n }\n \n /*\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex e0dd5d65ce..17f1b78c01 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -2059,6 +2059,77 @@ test_expect_success '--show-scope with --show-origin' '\n \ttest_cmp expect output\n '\n \n+test_expect_success 'override global and system config' '\n+\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n+\n+\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n+\t[home]\n+\t\tconfig = true\n+\tEOF\n+\tmkdir -p \"$HOME\"/.config/git &&\n+\tcat >\"$HOME\"/.config/git/config <<-EOF &&\n+\t[xdg]\n+\t\tconfig = true\n+\tEOF\n+\tcat >.git/config <<-EOF &&\n+\t[local]\n+\t\tconfig = true\n+\tEOF\n+\tcat >custom-global-config <<-EOF &&\n+\t[global]\n+\t\tconfig = true\n+\tEOF\n+\tcat >custom-system-config <<-EOF &&\n+\t[system]\n+\t\tconfig = true\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\tglobal\txdg.config=true\n+\tglobal\thome.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\n+\tcat >expect <<-EOF &&\n+\tsystem\tsystem.config=true\n+\tglobal\tglobal.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \\\n+\t\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tcat >expect <<-EOF &&\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null git config --show-scope --list >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'override global and system config with missing file' '\n+\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list >actual &&\n+\ttest_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list >actual &&\n+\tGIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version\n+'\n+\n+test_expect_success 'write to overridden global and system config' '\n+\tcat >expect < writes:\n\n> char *git_system_config(void)\n> {\n> +\tchar *system_config = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n> +\tif (system_config) {\n> +\t\tif (!strcmp(system_config, \"/dev/null\"))\n> +\t\t\tFREE_AND_NULL(system_config);\n> +\t\treturn system_config;\n> +\t}\n\nI am not sure if returning NULL from this function will always be\nthe same as returning /dev/null on a system with functioning\n/dev/null. For example, when use_system_config is enabled,\nbuiltin/config.c::cmd_config() assigns the NULL returned by this\nfunction to given_config_source.file and then calls\nconfig.c::config_with_options(), which notices that none of\nuse_stdin, file, or blob member of the config_source exists and\nfalls back to the config_sequence().\n\nSo, for the purpose of special casing \"/dev/null\" textually, the\nabove is not sufficient, I am afraid.\n\nLet's rescind the \"/dev/null gets turned into NULL\" in the above\nchange for now. If we truly want to cater to an installation where\nopen(\"/dev/null\") fails and emulate, that needs to be done at a much\nlower layer, but we do not have to go there for the purpose of this\nseries.\n\nThanks.\n"},{"id":"421830","messageId":"cover.1618297711.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1618238567.git.ps@pks.im","subject":"[PATCH v4 0/3] config: allow overriding global/system config","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-13T07:11:37Z","receivedAt":"2021-04-13T07:11:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the fourth version of my patch series to provide a way of\noverriding the global system configuration.\n\nCompared to v3, I only dropped the special-casing of `/dev/null`. As\nJunio rightly pointed out, the special-casing was incomplete and would\nhave required more work to do the right thing for all cases. It can\nstill be re-added at a later point if the usecase actually comes up.\n\nPatrick\n\nPatrick Steinhardt (3):\n config: rename `git_etc_config()`\n config: unify code paths to get global config paths\n config: allow overriding of global and system configuration\n\n Documentation/git-config.txt | 5 +++\n Documentation/git.txt | 10 +++++\n builtin/config.c | 6 +--\n config.c | 41 +++++++++++++++------\n config.h | 4 +-\n t/t1300-config.sh | 71 ++++++++++++++++++++++++++++++++++++\n 6 files changed, 121 insertions(+), 16 deletions(-)\n\nRange-diff against v3:\n1: 34bdbc27d6 = 1: 34bdbc27d6 config: rename `git_etc_config()`\n2: 30f18679bd = 2: 30f18679bd config: unify code paths to get global config paths\n3: af663640ae ! 3: d27efc0aa8 config: allow overriding of global and system configuration\n @@ Commit message\n - If unset, git continues to use the usual locations.\n \n - If set to a specific path, we skip reading the normal\n - configuration files and instead take the path.\n -\n - - If set to `/dev/null`, we do not load either global- or\n - system-level configuration at all.\n + configuration files and instead take the path. By setting the path\n + to `/dev/null`, no configuration will be loaded for the respective\n + level.\n \n This implements the usecase where we want to execute code in a sanitized\n environment without any potential misconfigurations via `/dev/null`, but\n @@ Documentation/git.txt: for further details.\n \tWhether to skip reading settings from the system-wide\n \t`$(prefix)/etc/gitconfig` file. This environment variable can\n \n - ## builtin/config.c ##\n -@@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)\n - \t\tchar *user_config, *xdg_config;\n - \n - \t\tgit_global_config(&user_config, &xdg_config);\n --\t\tif (!user_config)\n -+\t\tif (!user_config) {\n -+\t\t\tif (getenv(\"GIT_CONFIG_GLOBAL\"))\n -+\t\t\t\tdie(_(\"GIT_CONFIG_GLOBAL=/dev/null set\"));\n -+\n - \t\t\t/*\n - \t\t\t * It is unknown if HOME/.gitconfig exists, so\n - \t\t\t * we do not know if we should write to XDG\n -@@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)\n - \t\t\t * is set and points at a sane location.\n - \t\t\t */\n - \t\t\tdie(_(\"$HOME not set\"));\n -+\t\t}\n - \n - \t\tgiven_config_source.scope = CONFIG_SCOPE_GLOBAL;\n - \n -\n ## config.c ##\n @@ config.c: static int git_config_from_blob_ref(config_fn_t fn,\n \n char *git_system_config(void)\n {\n +\tchar *system_config = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n -+\tif (system_config) {\n -+\t\tif (!strcmp(system_config, \"/dev/null\"))\n -+\t\t\tFREE_AND_NULL(system_config);\n ++\tif (system_config)\n +\t\treturn system_config;\n -+\t}\n \treturn system_path(ETC_GITCONFIG);\n }\n \n @@ config.c: static int git_config_from_blob_ref(config_fn_t fn,\n +\tchar *user_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n +\tchar *xdg_config = NULL;\n +\n -+\tif (user_config) {\n -+\t\tif (!strcmp(user_config, \"/dev/null\"))\n -+\t\t\tFREE_AND_NULL(user_config);\n -+\t\txdg_config = NULL;\n -+\t} else {\n ++\tif (!user_config) {\n +\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n +\t\txdg_config = xdg_config_home(\"config\");\n +\t}\n-- \n2.31.1\n\n"},{"id":"421831","messageId":"34bdbc27d618d7467d2caf6844d8c06bdcb8545b.1618297711.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1618297711.git.ps@pks.im","subject":"[PATCH v4 1/3] config: rename `git_etc_config()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-13T07:11:44Z","receivedAt":"2021-04-13T07:11:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `git_etc_gitconfig()` function retrieves the system-level path of\nthe configuration file. We're about to introduce a way to override it\nvia an environment variable, at which point the name of this function\nwould start to become misleading.\n\nRename the function to `git_system_config()` as a preparatory step.\nWhile at it, the function is also refactored to pass memory ownership to\nthe caller. This is done to better match semantics of\n`git_global_config()`, which is going to be introduced in the next\ncommit.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/config.c | 2 +-\n config.c | 18 ++++++++----------\n config.h | 3 ++-\n 3 files changed, 11 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex f71fa39b38..02ed0b3fe7 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -695,7 +695,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \telse if (use_system_config) {\n-\t\tgiven_config_source.file = git_etc_gitconfig();\n+\t\tgiven_config_source.file = git_system_config();\n \t\tgiven_config_source.scope = CONFIG_SCOPE_SYSTEM;\n \t} else if (use_local_config) {\n \t\tgiven_config_source.file = git_pathdup(\"config\");\ndiff --git a/config.c b/config.c\nindex 6428393a41..8c83669cce 100644\n--- a/config.c\n+++ b/config.c\n@@ -1844,12 +1844,9 @@ static int git_config_from_blob_ref(config_fn_t fn,\n \treturn git_config_from_blob_oid(fn, name, &oid, data);\n }\n \n-const char *git_etc_gitconfig(void)\n+char *git_system_config(void)\n {\n-\tstatic const char *system_wide;\n-\tif (!system_wide)\n-\t\tsystem_wide = system_path(ETC_GITCONFIG);\n-\treturn system_wide;\n+\treturn system_path(ETC_GITCONFIG);\n }\n \n /*\n@@ -1883,6 +1880,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\t\t\t config_fn_t fn, void *data)\n {\n \tint ret = 0;\n+\tchar *system_config = git_system_config();\n \tchar *xdg_config = xdg_config_home(\"config\");\n \tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n \tchar *repo_config;\n@@ -1896,11 +1894,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\trepo_config = NULL;\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n-\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,\n-\t\t\t\t\t\t opts->system_gently ?\n-\t\t\t\t\t\t ACCESS_EACCES_OK : 0))\n-\t\tret += git_config_from_file(fn, git_etc_gitconfig(),\n-\t\t\t\t\t data);\n+\tif (system_config && !access_or_die(system_config, R_OK,\n+\t\t\t\t\t opts->system_gently ?\n+\t\t\t\t\t ACCESS_EACCES_OK : 0))\n+\t\tret += git_config_from_file(fn, system_config, data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n \tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n@@ -1927,6 +1924,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\tdie(_(\"unable to parse command-line config\"));\n \n \tcurrent_parsing_scope = prev_parsing_scope;\n+\tfree(system_config);\n \tfree(xdg_config);\n \tfree(user_config);\n \tfree(repo_config);\ndiff --git a/config.h b/config.h\nindex 19a9adbaa9..2be8fa1880 100644\n--- a/config.h\n+++ b/config.h\n@@ -318,7 +318,6 @@ int git_config_rename_section(const char *, const char *);\n int git_config_rename_section_in_file(const char *, const char *, const char *);\n int git_config_copy_section(const char *, const char *);\n int git_config_copy_section_in_file(const char *, const char *, const char *);\n-const char *git_etc_gitconfig(void);\n int git_env_bool(const char *, int);\n unsigned long git_env_ulong(const char *, unsigned long);\n int git_config_system(void);\n@@ -327,6 +326,8 @@ int config_error_nonbool(const char *);\n #define config_error_nonbool(s) (config_error_nonbool(s), const_error())\n #endif\n \n+char *git_system_config(void);\n+\n int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n \n enum config_scope current_config_scope(void);\n-- \n2.31.1\n\n"},{"id":"421832","messageId":"30f18679bd5b5afb581506b95f76871124be5126.1618297711.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1618297711.git.ps@pks.im","subject":"[PATCH v4 2/3] config: unify code paths to get global config paths","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-13T07:11:48Z","receivedAt":"2021-04-13T07:11:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There's two callsites which assemble global config paths, once in the\nconfig loading code and once in the git-config(1) builtin. We're about\nto implement a way to override global config paths via an environment\nvariable which would require us to adjust both sites.\n\nUnify both code paths into a single `git_global_config()` function which\nreturns both paths for `~/.gitconfig` and the XDG config file. This will\nmake the subsequent patch which introduces the new envvar easier to\nimplement.\n\nNo functional changes are expected from this patch.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/config.c | 4 ++--\n config.c | 12 ++++++++++--\n config.h | 1 +\n 3 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 02ed0b3fe7..865fddd6ce 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -671,9 +671,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (use_global_config) {\n-\t\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n-\t\tchar *xdg_config = xdg_config_home(\"config\");\n+\t\tchar *user_config, *xdg_config;\n \n+\t\tgit_global_config(&user_config, &xdg_config);\n \t\tif (!user_config)\n \t\t\t/*\n \t\t\t * It is unknown if HOME/.gitconfig exists, so\ndiff --git a/config.c b/config.c\nindex 8c83669cce..ebff58aa57 100644\n--- a/config.c\n+++ b/config.c\n@@ -1849,6 +1849,12 @@ char *git_system_config(void)\n \treturn system_path(ETC_GITCONFIG);\n }\n \n+void git_global_config(char **user_config, char **xdg_config)\n+{\n+\t*user_config = expand_user_path(\"~/.gitconfig\", 0);\n+\t*xdg_config = xdg_config_home(\"config\");\n+}\n+\n /*\n * Parse environment variable 'k' as a boolean (in various\n * possible spellings); if missing, use the default value 'def'.\n@@ -1881,8 +1887,8 @@ static int do_git_config_sequence(const struct config_options *opts,\n {\n \tint ret = 0;\n \tchar *system_config = git_system_config();\n-\tchar *xdg_config = xdg_config_home(\"config\");\n-\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n+\tchar *xdg_config = NULL;\n+\tchar *user_config = NULL;\n \tchar *repo_config;\n \tenum config_scope prev_parsing_scope = current_parsing_scope;\n \n@@ -1900,6 +1906,8 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\tret += git_config_from_file(fn, system_config, data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n+\tgit_global_config(&user_config, &xdg_config);\n+\n \tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n \t\tret += git_config_from_file(fn, xdg_config, data);\n \ndiff --git a/config.h b/config.h\nindex 2be8fa1880..9038538ffd 100644\n--- a/config.h\n+++ b/config.h\n@@ -327,6 +327,7 @@ int config_error_nonbool(const char *);\n #endif\n \n char *git_system_config(void);\n+void git_global_config(char **user, char **xdg);\n \n int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n \n-- \n2.31.1\n\n"},{"id":"421833","messageId":"d27efc0aa847f3fb179307b3a265b3759b7d9570.1618297711.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1618297711.git.ps@pks.im","subject":"[PATCH v4 3/3] config: allow overriding of global and system configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-13T07:11:52Z","receivedAt":"2021-04-13T07:11:58Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In order to have git run in a fully controlled environment without any\nmisconfiguration, it may be desirable for users or scripts to override\nglobal- and system-level configuration files. We already have a way of\ndoing this, which is to unset both HOME and XDG_CONFIG_HOME environment\nvariables and to set `GIT_CONFIG_NOGLOBAL=true`. This is quite kludgy,\nand unsetting the first two variables likely has an impact on other\nexecutables spawned by such a script.\n\nThe obvious way to fix this would be to introduce `GIT_CONFIG_NOGLOBAL`\nas an equivalent to `GIT_CONFIG_NOSYSTEM`. But in the past, it has\nturned out that this design is inflexible: we cannot test system-level\nparsing of the git configuration in our test harness because there is no\nway to change its location, so all tests run with `GIT_CONFIG_NOSYSTEM`\nset.\n\nInstead of doing the same mistake with `GIT_CONFIG_NOGLOBAL`, introduce\ntwo new variables `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM`:\n\n - If unset, git continues to use the usual locations.\n\n - If set to a specific path, we skip reading the normal\n configuration files and instead take the path. By setting the path\n to `/dev/null`, no configuration will be loaded for the respective\n level.\n\nThis implements the usecase where we want to execute code in a sanitized\nenvironment without any potential misconfigurations via `/dev/null`, but\nis more flexible and allows for more usecases than simply adding\n`GIT_CONFIG_NOGLOBAL`.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/git-config.txt | 5 +++\n Documentation/git.txt | 10 +++++\n config.c | 17 +++++++--\n t/t1300-config.sh | 71 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 100 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 4b4cc5c5e8..5cddadafd2 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -340,6 +340,11 @@ 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_GLOBAL::\n+GIT_CONFIG_SYSTEM::\n+\tTake the configuration from the given files instead from global or\n+\tsystem-level configuration. See linkgit:git[1] for details.\n+\n GIT_CONFIG_NOSYSTEM::\n \tWhether to skip reading settings from the system-wide\n \t$(prefix)/etc/gitconfig file. See linkgit:git[1] for details.\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 3a9c44987f..380422a6a9 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -670,6 +670,16 @@ for further details.\n \tIf this environment variable is set to `0`, git will not prompt\n \ton the terminal (e.g., when asking for HTTP authentication).\n \n+`GIT_CONFIG_GLOBAL`::\n+`GIT_CONFIG_SYSTEM`::\n+\tTake the configuration from the given files instead from global or\n+\tsystem-level configuration files. If `GIT_CONFIG_SYSTEM` is set, the\n+\tsystem config file defined at build time (usually `/etc/gitconfig`)\n+\twill not be read. Likewise, if `GIT_CONFIG_GLOBAL` is set, neither\n+\t`$HOME/.gitconfig` nor `$XDG_CONFIG_HOME/git/config` will be read. Can\n+\tbe set to `/dev/null` to skip reading configuration files of the\n+\trespective level.\n+\n `GIT_CONFIG_NOSYSTEM`::\n \tWhether to skip reading settings from the system-wide\n \t`$(prefix)/etc/gitconfig` file. This environment variable can\ndiff --git a/config.c b/config.c\nindex ebff58aa57..7caf6c1e53 100644\n--- a/config.c\n+++ b/config.c\n@@ -1846,13 +1846,24 @@ static int git_config_from_blob_ref(config_fn_t fn,\n \n char *git_system_config(void)\n {\n+\tchar *system_config = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n+\tif (system_config)\n+\t\treturn system_config;\n \treturn system_path(ETC_GITCONFIG);\n }\n \n-void git_global_config(char **user_config, char **xdg_config)\n+void git_global_config(char **user_out, char **xdg_out)\n {\n-\t*user_config = expand_user_path(\"~/.gitconfig\", 0);\n-\t*xdg_config = xdg_config_home(\"config\");\n+\tchar *user_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n+\tchar *xdg_config = NULL;\n+\n+\tif (!user_config) {\n+\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n+\t\txdg_config = xdg_config_home(\"config\");\n+\t}\n+\n+\t*user_out = user_config;\n+\t*xdg_out = xdg_config;\n }\n \n /*\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex e0dd5d65ce..17f1b78c01 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -2059,6 +2059,77 @@ test_expect_success '--show-scope with --show-origin' '\n \ttest_cmp expect output\n '\n \n+test_expect_success 'override global and system config' '\n+\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n+\n+\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n+\t[home]\n+\t\tconfig = true\n+\tEOF\n+\tmkdir -p \"$HOME\"/.config/git &&\n+\tcat >\"$HOME\"/.config/git/config <<-EOF &&\n+\t[xdg]\n+\t\tconfig = true\n+\tEOF\n+\tcat >.git/config <<-EOF &&\n+\t[local]\n+\t\tconfig = true\n+\tEOF\n+\tcat >custom-global-config <<-EOF &&\n+\t[global]\n+\t\tconfig = true\n+\tEOF\n+\tcat >custom-system-config <<-EOF &&\n+\t[system]\n+\t\tconfig = true\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\tglobal\txdg.config=true\n+\tglobal\thome.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\n+\tcat >expect <<-EOF &&\n+\tsystem\tsystem.config=true\n+\tglobal\tglobal.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \\\n+\t\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tcat >expect <<-EOF &&\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null git config --show-scope --list >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'override global and system config with missing file' '\n+\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list >actual &&\n+\ttest_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list >actual &&\n+\tGIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version\n+'\n+\n+test_expect_success 'write to overridden global and system config' '\n+\tcat >expect < While at it, the function is also refactored to pass memory ownership to\n> the caller. This is done to better match semantics of\n> `git_global_config()`, which is going to be introduced in the next\n> commit.\n\nI think this turned out nicely. There are only two callers, one of which\nis already handling freeing the global config. In the other:\n\n> diff --git a/builtin/config.c b/builtin/config.c\n> index f71fa39b38..02ed0b3fe7 100644\n> --- a/builtin/config.c\n> +++ b/builtin/config.c\n> @@ -695,7 +695,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n> \t\t}\n> \t}\n> \telse if (use_system_config) {\n> -\t\tgiven_config_source.file = git_etc_gitconfig();\n> +\t\tgiven_config_source.file = git_system_config();\n> \t\tgiven_config_source.scope = CONFIG_SCOPE_SYSTEM;\n> \t} else if (use_local_config) {\n> \t\tgiven_config_source.file = git_pathdup(\"config\");\n\nWe \"leak\" the result, but I think it is actually an improvement. As you\ncan see in the context, we are sometimes allocating the field in other\ncode paths, so this is takes us closer to a world where we can actually\ncall free(given_config_source.file) to fix the leak in all of the other\ncode paths. :)\n\n(I don't think any of that needs to be dealt with in this series, of\ncourse).\n\n-Peff\n"},{"id":"421841","messageId":"YHVJNOjJhysn7LBy@coredump.intra.peff.net","threadId":"55455","inReplyTo":"d27efc0aa847f3fb179307b3a265b3759b7d9570.1618297711.git.ps@pks.im","subject":"Re: [PATCH v4 3/3] config: allow overriding of global and system configuration","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-13T07:33:08Z","receivedAt":"2021-04-13T07:33:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 13, 2021 at 09:11:52AM +0200, Patrick Steinhardt wrote:\n\n> +test_expect_success 'override global and system config with missing file' '\n> +\tsane_unset GIT_CONFIG_NOSYSTEM &&\n> +\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list >actual &&\n> +\ttest_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list >actual &&\n> +\tGIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version\n> +'\n\nI was slightly surprised to see these still marked as test_must_fail.\nBut it's because git-config, when given a _specific_ file to read, will\ncomplain if the file doesn't exist. And that is true independent of your\npatch.\n\nThere is one interesting implication there. Running:\n\n GIT_CONFIG_SYSTEM=/dev/null git config --system --list\n\ndoes _not_ complain, even though:\n\n GIT_CONFIG_NOSYSTEM=1 git config --system --list\n\ndoes. IMHO that is quite sensible, but I wanted to point it out, as\nusing /dev/null is not an exact replacement for GIT_CONFIG_NOSYSTEM (in\nmy opinion it is even better ;) ).\n\n-Peff\n"},{"id":"421842","messageId":"YHVJWKKPkjw0+n/u@coredump.intra.peff.net","threadId":"55455","inReplyTo":"cover.1618297711.git.ps@pks.im","subject":"Re: [PATCH v4 0/3] config: allow overriding global/system config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-13T07:33:44Z","receivedAt":"2021-04-13T07:33:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 13, 2021 at 09:11:37AM +0200, Patrick Steinhardt wrote:\n\n> this is the fourth version of my patch series to provide a way of\n> overriding the global system configuration.\n> \n> Compared to v3, I only dropped the special-casing of `/dev/null`. As\n> Junio rightly pointed out, the special-casing was incomplete and would\n> have required more work to do the right thing for all cases. It can\n> still be re-added at a later point if the usecase actually comes up.\n\nThis version looks good to me. Thanks for working on this!\n\n-Peff\n"},{"id":"421846","messageId":"YHVOUhThkCJiSqmj@ncase","threadId":"55455","inReplyTo":"YHVJNOjJhysn7LBy@coredump.intra.peff.net","subject":"Re: [PATCH v4 3/3] config: allow overriding of global and system configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-13T07:54:58Z","receivedAt":"2021-04-13T07:55:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 13, 2021 at 03:33:08AM -0400, Jeff King wrote:\n> On Tue, Apr 13, 2021 at 09:11:52AM +0200, Patrick Steinhardt wrote:\n> \n> > +test_expect_success 'override global and system config with missing file' '\n> > +\tsane_unset GIT_CONFIG_NOSYSTEM &&\n> > +\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list >actual &&\n> > +\ttest_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list >actual &&\n> > +\tGIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version\n> > +'\n> \n> I was slightly surprised to see these still marked as test_must_fail.\n> But it's because git-config, when given a _specific_ file to read, will\n> complain if the file doesn't exist. And that is true independent of your\n> patch.\n\nYeah, surprising at first but then again it does feel sensible when\nthinking about it: we ask git-config(1) to explitly list the\nglobal/system-level configuration, and when it does not exist it does\nfeel sane to complain.\n\n> There is one interesting implication there. Running:\n> \n> GIT_CONFIG_SYSTEM=/dev/null git config --system --list\n> \n> does _not_ complain, even though:\n> \n> GIT_CONFIG_NOSYSTEM=1 git config --system --list\n> \n> does. IMHO that is quite sensible, but I wanted to point it out, as\n> using /dev/null is not an exact replacement for GIT_CONFIG_NOSYSTEM (in\n> my opinion it is even better ;) ).\n\nWhereas this is the other edge case: we do ask it to explicitly list the\nsystem-level configuration, and it _does_ exist even though it's empty.\nI wouldn't expect to get an error here, and I do prefer this behaviour\nto NOSYSTEM, too.\n\nPatrick\n"},{"id":"421910","messageId":"xmqqblaim5vv.fsf@gitster.g","threadId":"55455","inReplyTo":"cover.1618297711.git.ps@pks.im","subject":"Re: [PATCH v4 0/3] config: allow overriding global/system config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-13T17:49:40Z","receivedAt":"2021-04-13T17:49:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Compared to v3, I only dropped the special-casing of `/dev/null`. As\n> Junio rightly pointed out, the special-casing was incomplete and would\n> have required more work to do the right thing for all cases. It can\n> still be re-added at a later point if the usecase actually comes up.\n>\n> Patrick\n>\n> Patrick Steinhardt (3):\n> config: rename `git_etc_config()`\n> config: unify code paths to get global config paths\n> config: allow overriding of global and system configuration\n>\n> Documentation/git-config.txt | 5 +++\n> Documentation/git.txt | 10 +++++\n> builtin/config.c | 6 +--\n> config.c | 41 +++++++++++++++------\n> config.h | 4 +-\n> t/t1300-config.sh | 71 ++++++++++++++++++++++++++++++++++++\n> 6 files changed, 121 insertions(+), 16 deletions(-)\n\nThis round looks good to me. Sorry for suggesting the \"/dev/null\"\nthing in the first place to lead you into wild goose chase during\nthe last round.\n\nWill queue. Thanks for working on it.\n\nAnd thanks for reviewing, everybody.\n\n>\n> Range-diff against v3:\n> 1: 34bdbc27d6 = 1: 34bdbc27d6 config: rename `git_etc_config()`\n> 2: 30f18679bd = 2: 30f18679bd config: unify code paths to get global config paths\n> 3: af663640ae ! 3: d27efc0aa8 config: allow overriding of global and system configuration\n> @@ Commit message\n> - If unset, git continues to use the usual locations.\n> \n> - If set to a specific path, we skip reading the normal\n> - configuration files and instead take the path.\n> -\n> - - If set to `/dev/null`, we do not load either global- or\n> - system-level configuration at all.\n> + configuration files and instead take the path. By setting the path\n> + to `/dev/null`, no configuration will be loaded for the respective\n> + level.\n> \n> This implements the usecase where we want to execute code in a sanitized\n> environment without any potential misconfigurations via `/dev/null`, but\n> @@ Documentation/git.txt: for further details.\n> \tWhether to skip reading settings from the system-wide\n> \t`$(prefix)/etc/gitconfig` file. This environment variable can\n> \n> - ## builtin/config.c ##\n> -@@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)\n> - \t\tchar *user_config, *xdg_config;\n> - \n> - \t\tgit_global_config(&user_config, &xdg_config);\n> --\t\tif (!user_config)\n> -+\t\tif (!user_config) {\n> -+\t\t\tif (getenv(\"GIT_CONFIG_GLOBAL\"))\n> -+\t\t\t\tdie(_(\"GIT_CONFIG_GLOBAL=/dev/null set\"));\n> -+\n> - \t\t\t/*\n> - \t\t\t * It is unknown if HOME/.gitconfig exists, so\n> - \t\t\t * we do not know if we should write to XDG\n> -@@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)\n> - \t\t\t * is set and points at a sane location.\n> - \t\t\t */\n> - \t\t\tdie(_(\"$HOME not set\"));\n> -+\t\t}\n> - \n> - \t\tgiven_config_source.scope = CONFIG_SCOPE_GLOBAL;\n> - \n> -\n> ## config.c ##\n> @@ config.c: static int git_config_from_blob_ref(config_fn_t fn,\n> \n> char *git_system_config(void)\n> {\n> +\tchar *system_config = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n> -+\tif (system_config) {\n> -+\t\tif (!strcmp(system_config, \"/dev/null\"))\n> -+\t\t\tFREE_AND_NULL(system_config);\n> ++\tif (system_config)\n> +\t\treturn system_config;\n> -+\t}\n> \treturn system_path(ETC_GITCONFIG);\n> }\n> \n> @@ config.c: static int git_config_from_blob_ref(config_fn_t fn,\n> +\tchar *user_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n> +\tchar *xdg_config = NULL;\n> +\n> -+\tif (user_config) {\n> -+\t\tif (!strcmp(user_config, \"/dev/null\"))\n> -+\t\t\tFREE_AND_NULL(user_config);\n> -+\t\txdg_config = NULL;\n> -+\t} else {\n> ++\tif (!user_config) {\n> +\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n> +\t\txdg_config = xdg_config_home(\"config\");\n> +\t}\n"},{"id":"421948","messageId":"YHZ/m7Kcoc89l/Vn@ncase","threadId":"55455","inReplyTo":"xmqqblaim5vv.fsf@gitster.g","subject":"Re: [PATCH v4 0/3] config: allow overriding global/system config","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-14T05:37:31Z","receivedAt":"2021-04-14T05:37:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 13, 2021 at 10:49:40AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > Compared to v3, I only dropped the special-casing of `/dev/null`. As\n> > Junio rightly pointed out, the special-casing was incomplete and would\n> > have required more work to do the right thing for all cases. It can\n> > still be re-added at a later point if the usecase actually comes up.\n> >\n> > Patrick\n> >\n> > Patrick Steinhardt (3):\n> > config: rename `git_etc_config()`\n> > config: unify code paths to get global config paths\n> > config: allow overriding of global and system configuration\n> >\n> > Documentation/git-config.txt | 5 +++\n> > Documentation/git.txt | 10 +++++\n> > builtin/config.c | 6 +--\n> > config.c | 41 +++++++++++++++------\n> > config.h | 4 +-\n> > t/t1300-config.sh | 71 ++++++++++++++++++++++++++++++++++++\n> > 6 files changed, 121 insertions(+), 16 deletions(-)\n> \n> This round looks good to me. Sorry for suggesting the \"/dev/null\"\n> thing in the first place to lead you into wild goose chase during\n> the last round.\n> \n> Will queue. Thanks for working on it.\n> \n> And thanks for reviewing, everybody.\n\nNo worries, and thanks a lot for all the feedback!\n\nPatrick\n"},{"id":"422123","messageId":"20210416211451.GP2947267@szeder.dev","threadId":"55455","inReplyTo":"34bdbc27d618d7467d2caf6844d8c06bdcb8545b.1618297711.git.ps@pks.im","subject":"Re: [PATCH v4 1/3] config: rename `git_etc_config()`","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-04-16T21:14:51Z","receivedAt":"2021-04-16T21:14:56Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Apr 13, 2021 at 09:11:44AM +0200, Patrick Steinhardt wrote:\n> The `git_etc_gitconfig()` function retrieves the system-level path of\n> the configuration file. We're about to introduce a way to override it\n> via an environment variable, at which point the name of this function\n> would start to become misleading.\n> \n> Rename the function to `git_system_config()` as a preparatory step.\n> While at it, the function is also refactored to pass memory ownership to\n> the caller. This is done to better match semantics of\n> `git_global_config()`, which is going to be introduced in the next\n> commit.\n> \n> Signed-off-by: Patrick Steinhardt \n> ---\n> builtin/config.c | 2 +-\n> config.c | 18 ++++++++----------\n> config.h | 3 ++-\n> 3 files changed, 11 insertions(+), 12 deletions(-)\n> \n> diff --git a/builtin/config.c b/builtin/config.c\n> index f71fa39b38..02ed0b3fe7 100644\n> --- a/builtin/config.c\n> +++ b/builtin/config.c\n> @@ -695,7 +695,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n> \t\t}\n> \t}\n> \telse if (use_system_config) {\n> -\t\tgiven_config_source.file = git_etc_gitconfig();\n> +\t\tgiven_config_source.file = git_system_config();\n> \t\tgiven_config_source.scope = CONFIG_SCOPE_SYSTEM;\n> \t} else if (use_local_config) {\n> \t\tgiven_config_source.file = git_pathdup(\"config\");\n> diff --git a/config.c b/config.c\n> index 6428393a41..8c83669cce 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1844,12 +1844,9 @@ static int git_config_from_blob_ref(config_fn_t fn,\n> \treturn git_config_from_blob_oid(fn, name, &oid, data);\n> }\n> \n> -const char *git_etc_gitconfig(void)\n> +char *git_system_config(void)\n> {\n> -\tstatic const char *system_wide;\n> -\tif (!system_wide)\n> -\t\tsystem_wide = system_path(ETC_GITCONFIG);\n> -\treturn system_wide;\n> +\treturn system_path(ETC_GITCONFIG);\n> }\n> \n> /*\n> @@ -1883,6 +1880,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n> \t\t\t\t config_fn_t fn, void *data)\n> {\n> \tint ret = 0;\n> +\tchar *system_config = git_system_config();\n> \tchar *xdg_config = xdg_config_home(\"config\");\n> \tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n> \tchar *repo_config;\n> @@ -1896,11 +1894,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n> \t\trepo_config = NULL;\n> \n> \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n> -\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,\n\nRemoving git_config_system() from the condition breaks\nGIT_CONFIG_NOSYSTEM:\n\n expecting success of 9999.1 'test': \n \tcat /usr/local/etc/gitconfig &&\n \tgit config --list --show-origin --show-scope\n \n + cat /usr/local/etc/gitconfig\n [foo]\n \tbar = baz\n + git config --list --show-origin --show-scope\n system\tfile:/usr/local/etc/gitconfig\tfoo.bar=baz\n local\tfile:.git/config\tcore.repositoryformatversion=0\n local\tfile:.git/config\tcore.filemode=true\n local\tfile:.git/config\tcore.bare=false\n local\tfile:.git/config\tcore.logallrefupdates=true\n ok 1 - test\n\nAnd breaks just about everything the Linux32 job on Travis CI:\n\n https://travis-ci.org/github/git/git/jobs/767207687#L1218\n\n> -\t\t\t\t\t\t opts->system_gently ?\n> -\t\t\t\t\t\t ACCESS_EACCES_OK : 0))\n> -\t\tret += git_config_from_file(fn, git_etc_gitconfig(),\n> -\t\t\t\t\t data);\n> +\tif (system_config && !access_or_die(system_config, R_OK,\n> +\t\t\t\t\t opts->system_gently ?\n> +\t\t\t\t\t ACCESS_EACCES_OK : 0))\n> +\t\tret += git_config_from_file(fn, system_config, data);\n> \n> \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n> \tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n> @@ -1927,6 +1924,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n> \t\tdie(_(\"unable to parse command-line config\"));\n> \n> \tcurrent_parsing_scope = prev_parsing_scope;\n> +\tfree(system_config);\n> \tfree(xdg_config);\n> \tfree(user_config);\n> \tfree(repo_config);\n> diff --git a/config.h b/config.h\n> index 19a9adbaa9..2be8fa1880 100644\n> --- a/config.h\n> +++ b/config.h\n> @@ -318,7 +318,6 @@ int git_config_rename_section(const char *, const char *);\n> int git_config_rename_section_in_file(const char *, const char *, const char *);\n> int git_config_copy_section(const char *, const char *);\n> int git_config_copy_section_in_file(const char *, const char *, const char *);\n> -const char *git_etc_gitconfig(void);\n> int git_env_bool(const char *, int);\n> unsigned long git_env_ulong(const char *, unsigned long);\n> int git_config_system(void);\n> @@ -327,6 +326,8 @@ int config_error_nonbool(const char *);\n> #define config_error_nonbool(s) (config_error_nonbool(s), const_error())\n> #endif\n> \n> +char *git_system_config(void);\n> +\n> int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n> \n> enum config_scope current_config_scope(void);\n> -- \n> 2.31.1\n> \n\n\n"},{"id":"422155","messageId":"YHqf9zjSzpYl7IAW@coredump.intra.peff.net","threadId":"55455","inReplyTo":"20210416211451.GP2947267@szeder.dev","subject":"Re: [PATCH v4 1/3] config: rename `git_etc_config()`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-17T08:44:39Z","receivedAt":"2021-04-17T08:44:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 16, 2021 at 11:14:51PM +0200, SZEDER Gábor wrote:\n\n> > @@ -1883,6 +1880,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n> > \t\t\t\t config_fn_t fn, void *data)\n> > {\n> > \tint ret = 0;\n> > +\tchar *system_config = git_system_config();\n> > \tchar *xdg_config = xdg_config_home(\"config\");\n> > \tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n> > \tchar *repo_config;\n> > @@ -1896,11 +1894,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n> > \t\trepo_config = NULL;\n> > \n> > \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n> > -\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,\n> \n> Removing git_config_system() from the condition breaks\n> GIT_CONFIG_NOSYSTEM:\n\nGood catch. My gut feeling is that the new git_system_config() should\ncheck NOSYSTEM and return NULL if it's set, and then we can get rid of\ngit_config_system() entirely.\n\nThat is slightly different than the old behavior; right now\nGIT_CONFIG_NOSYSTEM only prevents reading during the normal sequence,\nand not reading (or writing!) via \"git config --system\". But I think it\nwould be an improvement to prevent those (the whole point of the feature\nwas to avoid the test suite accidentally accessing the larger\nenvironment).\n\n-Peff\n"},{"id":"422189","messageId":"xmqq7dl0d23g.fsf@gitster.g","threadId":"55455","inReplyTo":"YHqf9zjSzpYl7IAW@coredump.intra.peff.net","subject":"Re: [PATCH v4 1/3] config: rename `git_etc_config()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-17T21:37:39Z","receivedAt":"2021-04-17T21:37:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Apr 16, 2021 at 11:14:51PM +0200, SZEDER Gábor wrote:\n>\n>> > @@ -1883,6 +1880,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n>> > \t\t\t\t config_fn_t fn, void *data)\n>> > {\n>> > \tint ret = 0;\n>> > +\tchar *system_config = git_system_config();\n>> > \tchar *xdg_config = xdg_config_home(\"config\");\n>> > \tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n>> > \tchar *repo_config;\n>> > @@ -1896,11 +1894,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n>> > \t\trepo_config = NULL;\n>> > \n>> > \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n>> > -\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,\n>> \n>> Removing git_config_system() from the condition breaks\n>> GIT_CONFIG_NOSYSTEM:\n>\n> Good catch. My gut feeling is that the new git_system_config() should\n> check NOSYSTEM and return NULL if it's set, and then we can get rid of\n> git_config_system() entirely.\n\nNULL -> /dev/null I guess?\n\n> That is slightly different than the old behavior; right now\n> GIT_CONFIG_NOSYSTEM only prevents reading during the normal sequence,\n> and not reading (or writing!) via \"git config --system\". But I think it\n> would be an improvement to prevent those (the whole point of the feature\n> was to avoid the test suite accidentally accessing the larger\n> environment).\n\nYup.\n"},{"id":"422199","messageId":"YHvGE5/LRPN9obZi@coredump.intra.peff.net","threadId":"55455","inReplyTo":"xmqq7dl0d23g.fsf@gitster.g","subject":"Re: [PATCH v4 1/3] config: rename `git_etc_config()`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-18T05:39:31Z","receivedAt":"2021-04-18T05:39:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 17, 2021 at 02:37:39PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > On Fri, Apr 16, 2021 at 11:14:51PM +0200, SZEDER Gábor wrote:\n> >\n> >> > @@ -1883,6 +1880,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n> >> > \t\t\t\t config_fn_t fn, void *data)\n> >> > {\n> >> > \tint ret = 0;\n> >> > +\tchar *system_config = git_system_config();\n> >> > \tchar *xdg_config = xdg_config_home(\"config\");\n> >> > \tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n> >> > \tchar *repo_config;\n> >> > @@ -1896,11 +1894,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n> >> > \t\trepo_config = NULL;\n> >> > \n> >> > \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n> >> > -\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,\n> >> \n> >> Removing git_config_system() from the condition breaks\n> >> GIT_CONFIG_NOSYSTEM:\n> >\n> > Good catch. My gut feeling is that the new git_system_config() should\n> > check NOSYSTEM and return NULL if it's set, and then we can get rid of\n> > git_config_system() entirely.\n> \n> NULL -> /dev/null I guess?\n\nI was thinking NULL to catch this line from the post-image of Patrick's\nseries:\n\n if (system_config && !access_or_die(system_config, R_OK,\n opts->system_gently ?\n ACCESS_EACCES_OK : 0))\n ret += git_config_from_file(fn, system_config, data);\n\nwhich would see that we have no file at all. But that may be unexpected\nfor other callers (right now git_etc_gitconfig() can never return NULL,\nand I'm not sure what would happen with \"git config --system\"; I suspect\nit would do the regular config sequence instead, which is wrong).\n\nSo yeah, probably returning /dev/null is more sensible (and makes it a\ntrue alias for GIT_CONFIG_SYSTEM=/dev/null).\n\n-Peff\n"},{"id":"422261","messageId":"YH1jnIhuF4UjXVNW@tanuki","threadId":"55455","inReplyTo":"YHvGE5/LRPN9obZi@coredump.intra.peff.net","subject":"Re: [PATCH v4 1/3] config: rename `git_etc_config()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-19T11:03:56Z","receivedAt":"2021-04-19T11:02:43Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Apr 18, 2021 at 01:39:31AM -0400, Jeff King wrote:\n> On Sat, Apr 17, 2021 at 02:37:39PM -0700, Junio C Hamano wrote:\n> \n> > Jeff King writes:\n> > \n> > > On Fri, Apr 16, 2021 at 11:14:51PM +0200, SZEDER Gábor wrote:\n> > >\n> > >> > @@ -1883,6 +1880,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n> > >> > \t\t\t\t config_fn_t fn, void *data)\n> > >> > {\n> > >> > \tint ret = 0;\n> > >> > +\tchar *system_config = git_system_config();\n> > >> > \tchar *xdg_config = xdg_config_home(\"config\");\n> > >> > \tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n> > >> > \tchar *repo_config;\n> > >> > @@ -1896,11 +1894,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n> > >> > \t\trepo_config = NULL;\n> > >> > \n> > >> > \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n> > >> > -\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,\n> > >> \n> > >> Removing git_config_system() from the condition breaks\n> > >> GIT_CONFIG_NOSYSTEM:\n> > >\n> > > Good catch. My gut feeling is that the new git_system_config() should\n> > > check NOSYSTEM and return NULL if it's set, and then we can get rid of\n> > > git_config_system() entirely.\n> > \n> > NULL -> /dev/null I guess?\n> \n> I was thinking NULL to catch this line from the post-image of Patrick's\n> series:\n> \n> if (system_config && !access_or_die(system_config, R_OK,\n> opts->system_gently ?\n> ACCESS_EACCES_OK : 0))\n> ret += git_config_from_file(fn, system_config, data);\n> \n> which would see that we have no file at all. But that may be unexpected\n> for other callers (right now git_etc_gitconfig() can never return NULL,\n> and I'm not sure what would happen with \"git config --system\"; I suspect\n> it would do the regular config sequence instead, which is wrong).\n> \n> So yeah, probably returning /dev/null is more sensible (and makes it a\n> true alias for GIT_CONFIG_SYSTEM=/dev/null).\n> \n> -Peff\n\nIt's only by accident that I dropped the call to `git_config_system()`,\nmust've happened when resolving conflicts somewhere. The issue with just\nreturning `/dev/null` from `git_system_config()` is that git-config(1)\nwould be broken, as you hint at. We do not honor GIT_CONFIG_NOSYSTEM\nthere if the \"--system\" flag was given.\n\nSo yes, we could change it to return `/dev/null`, but that would change\nsemantics of GIT_CONFIG_NOSYSTEM. I'm not sure doing this in the same\nseries is a good idea. Even more so because with returning `/dev/null`,\nthe conversion would be silent: whereas previous versions simply wrote\nto or read from the system-level config, we now pretend to have read or\nwritten something even though we didn't.\n\nPatrick\n"},{"id":"422305","messageId":"1e8899408a249abd3743b2728434352fba701430.1618835148.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1618835148.git.ps@pks.im","subject":"[PATCH v5 1/3] config: rename `git_etc_config()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-19T12:31:08Z","receivedAt":"2021-04-19T12:31:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `git_etc_gitconfig()` function retrieves the system-level path of\nthe configuration file. We're about to introduce a way to override it\nvia an environment variable, at which point the name of this function\nwould start to become misleading.\n\nRename the function to `git_system_config()` as a preparatory step.\nWhile at it, the function is also refactored to pass memory ownership to\nthe caller. This is done to better match semantics of\n`git_global_config()`, which is going to be introduced in the next\ncommit.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/config.c | 2 +-\n config.c | 18 ++++++++----------\n config.h | 3 ++-\n 3 files changed, 11 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex f71fa39b38..02ed0b3fe7 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -695,7 +695,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \telse if (use_system_config) {\n-\t\tgiven_config_source.file = git_etc_gitconfig();\n+\t\tgiven_config_source.file = git_system_config();\n \t\tgiven_config_source.scope = CONFIG_SCOPE_SYSTEM;\n \t} else if (use_local_config) {\n \t\tgiven_config_source.file = git_pathdup(\"config\");\ndiff --git a/config.c b/config.c\nindex 870d9534de..e62960b5a6 100644\n--- a/config.c\n+++ b/config.c\n@@ -1830,12 +1830,9 @@ static int git_config_from_blob_ref(config_fn_t fn,\n \treturn git_config_from_blob_oid(fn, name, &oid, data);\n }\n \n-const char *git_etc_gitconfig(void)\n+char *git_system_config(void)\n {\n-\tstatic const char *system_wide;\n-\tif (!system_wide)\n-\t\tsystem_wide = system_path(ETC_GITCONFIG);\n-\treturn system_wide;\n+\treturn system_path(ETC_GITCONFIG);\n }\n \n /*\n@@ -1869,6 +1866,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\t\t\t config_fn_t fn, void *data)\n {\n \tint ret = 0;\n+\tchar *system_config = git_system_config();\n \tchar *xdg_config = xdg_config_home(\"config\");\n \tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n \tchar *repo_config;\n@@ -1882,11 +1880,10 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\trepo_config = NULL;\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n-\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK,\n-\t\t\t\t\t\t opts->system_gently ?\n-\t\t\t\t\t\t ACCESS_EACCES_OK : 0))\n-\t\tret += git_config_from_file(fn, git_etc_gitconfig(),\n-\t\t\t\t\t data);\n+\tif (git_config_system() && system_config &&\n+\t !access_or_die(system_config, R_OK,\n+\t\t\t opts->system_gently ? ACCESS_EACCES_OK : 0))\n+\t\tret += git_config_from_file(fn, system_config, data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n \tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n@@ -1913,6 +1910,7 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\tdie(_(\"unable to parse command-line config\"));\n \n \tcurrent_parsing_scope = prev_parsing_scope;\n+\tfree(system_config);\n \tfree(xdg_config);\n \tfree(user_config);\n \tfree(repo_config);\ndiff --git a/config.h b/config.h\nindex 19a9adbaa9..2be8fa1880 100644\n--- a/config.h\n+++ b/config.h\n@@ -318,7 +318,6 @@ int git_config_rename_section(const char *, const char *);\n int git_config_rename_section_in_file(const char *, const char *, const char *);\n int git_config_copy_section(const char *, const char *);\n int git_config_copy_section_in_file(const char *, const char *, const char *);\n-const char *git_etc_gitconfig(void);\n int git_env_bool(const char *, int);\n unsigned long git_env_ulong(const char *, unsigned long);\n int git_config_system(void);\n@@ -327,6 +326,8 @@ int config_error_nonbool(const char *);\n #define config_error_nonbool(s) (config_error_nonbool(s), const_error())\n #endif\n \n+char *git_system_config(void);\n+\n int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n \n enum config_scope current_config_scope(void);\n-- \n2.31.1\n\n"},{"id":"422306","messageId":"cover.1618835148.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1618297711.git.ps@pks.im","subject":"[PATCH v5 0/3] config: allow overriding global/system config","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-19T12:31:04Z","receivedAt":"2021-04-19T12:31:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the fifth version of my patch series to provide a way of\noverriding the global system configuration.\n\nChanges to v4:\n\n - Readded the call to `git_config_system()`, which I've previously\n dropped by accident. I didn't move it into the new\n `git_system_config()` function as it would change semantics of\n `git config --system`.\n\n - Added a testcase which verifies that GIT_CONFIG_NOSYSTEM and\n GIT_CONFIG_SYSTEM properly interact with each other: if\n GIT_CONFIG_NOSYSTEM is set, no system-level configuration shall be\n read. This is different than the tests for `git config --system`\n which used to and still does ignore GIT_CONFIG_NOSYSTEM.\n\n - Small fixups for another testcase to drop needless redirects and\n the `sane_unset` of GIT_CONFIG_NOSYSTEM.\n\nPatrick\n\n\nPatrick Steinhardt (3):\n config: rename `git_etc_config()`\n config: unify code paths to get global config paths\n config: allow overriding of global and system configuration\n\n Documentation/git-config.txt | 5 +++\n Documentation/git.txt | 10 +++++\n builtin/config.c | 6 +--\n config.c | 41 ++++++++++++-----\n config.h | 4 +-\n t/t1300-config.sh | 86 ++++++++++++++++++++++++++++++++++++\n 6 files changed, 136 insertions(+), 16 deletions(-)\n\nRange-diff against v4:\n1: 34bdbc27d6 ! 1: 1e8899408a config: rename `git_etc_config()`\n @@ config.c: static int do_git_config_sequence(const struct config_options *opts,\n -\t\t\t\t\t\t ACCESS_EACCES_OK : 0))\n -\t\tret += git_config_from_file(fn, git_etc_gitconfig(),\n -\t\t\t\t\t data);\n -+\tif (system_config && !access_or_die(system_config, R_OK,\n -+\t\t\t\t\t opts->system_gently ?\n -+\t\t\t\t\t ACCESS_EACCES_OK : 0))\n ++\tif (git_config_system() && system_config &&\n ++\t !access_or_die(system_config, R_OK,\n ++\t\t\t opts->system_gently ? ACCESS_EACCES_OK : 0))\n +\t\tret += git_config_from_file(fn, system_config, data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n2: 30f18679bd = 2: 39468f45d2 config: unify code paths to get global config paths\n3: d27efc0aa8 ! 3: 7e7506217e config: allow overriding of global and system configuration\n @@ t/t1300-config.sh: test_expect_success '--show-scope with --show-origin' '\n +'\n +\n +test_expect_success 'override global and system config with missing file' '\n -+\tsane_unset GIT_CONFIG_NOSYSTEM &&\n -+\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list >actual &&\n -+\ttest_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list >actual &&\n ++\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list &&\n ++\ttest_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list &&\n +\tGIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version\n +'\n +\n ++test_expect_success 'system override has no effect with GIT_CONFIG_NOSYSTEM' '\n ++\t# `git config --system` has different semantics compared to other\n ++\t# commands as it ignores GIT_CONFIG_NOSYSTEM. We thus test whether the\n ++\t# variable has an effect via a different proxy.\n ++\tcat >alias-config <<-EOF &&\n ++\t[alias]\n ++\t\thello-world = !echo \"hello world\"\n ++\tEOF\n ++\ttest_must_fail env GIT_CONFIG_NOSYSTEM=true GIT_CONFIG_SYSTEM=alias-config \\\n ++\t\tgit hello-world &&\n ++\tGIT_CONFIG_NOSYSTEM=false GIT_CONFIG_SYSTEM=alias-config \\\n ++\t\tgit hello-world >actual &&\n ++\techo \"hello world\" >expect &&\n ++\ttest_cmp expect actual\n ++'\n ++\n +test_expect_success 'write to overridden global and system config' '\n +\tcat >expect <\n---\n builtin/config.c | 4 ++--\n config.c | 12 ++++++++++--\n config.h | 1 +\n 3 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 02ed0b3fe7..865fddd6ce 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -671,9 +671,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (use_global_config) {\n-\t\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n-\t\tchar *xdg_config = xdg_config_home(\"config\");\n+\t\tchar *user_config, *xdg_config;\n \n+\t\tgit_global_config(&user_config, &xdg_config);\n \t\tif (!user_config)\n \t\t\t/*\n \t\t\t * It is unknown if HOME/.gitconfig exists, so\ndiff --git a/config.c b/config.c\nindex e62960b5a6..161dfaa707 100644\n--- a/config.c\n+++ b/config.c\n@@ -1835,6 +1835,12 @@ char *git_system_config(void)\n \treturn system_path(ETC_GITCONFIG);\n }\n \n+void git_global_config(char **user_config, char **xdg_config)\n+{\n+\t*user_config = expand_user_path(\"~/.gitconfig\", 0);\n+\t*xdg_config = xdg_config_home(\"config\");\n+}\n+\n /*\n * Parse environment variable 'k' as a boolean (in various\n * possible spellings); if missing, use the default value 'def'.\n@@ -1867,8 +1873,8 @@ static int do_git_config_sequence(const struct config_options *opts,\n {\n \tint ret = 0;\n \tchar *system_config = git_system_config();\n-\tchar *xdg_config = xdg_config_home(\"config\");\n-\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n+\tchar *xdg_config = NULL;\n+\tchar *user_config = NULL;\n \tchar *repo_config;\n \tenum config_scope prev_parsing_scope = current_parsing_scope;\n \n@@ -1886,6 +1892,8 @@ static int do_git_config_sequence(const struct config_options *opts,\n \t\tret += git_config_from_file(fn, system_config, data);\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_GLOBAL;\n+\tgit_global_config(&user_config, &xdg_config);\n+\n \tif (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))\n \t\tret += git_config_from_file(fn, xdg_config, data);\n \ndiff --git a/config.h b/config.h\nindex 2be8fa1880..9038538ffd 100644\n--- a/config.h\n+++ b/config.h\n@@ -327,6 +327,7 @@ int config_error_nonbool(const char *);\n #endif\n \n char *git_system_config(void);\n+void git_global_config(char **user, char **xdg);\n \n int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n \n-- \n2.31.1\n\n"},{"id":"422308","messageId":"7e7506217e4aac82e3a03013d24e6f885383ecd3.1618835148.git.ps@pks.im","threadId":"55455","inReplyTo":"cover.1618835148.git.ps@pks.im","subject":"[PATCH v5 3/3] config: allow overriding of global and system configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-19T12:31:16Z","receivedAt":"2021-04-19T12:31:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In order to have git run in a fully controlled environment without any\nmisconfiguration, it may be desirable for users or scripts to override\nglobal- and system-level configuration files. We already have a way of\ndoing this, which is to unset both HOME and XDG_CONFIG_HOME environment\nvariables and to set `GIT_CONFIG_NOGLOBAL=true`. This is quite kludgy,\nand unsetting the first two variables likely has an impact on other\nexecutables spawned by such a script.\n\nThe obvious way to fix this would be to introduce `GIT_CONFIG_NOGLOBAL`\nas an equivalent to `GIT_CONFIG_NOSYSTEM`. But in the past, it has\nturned out that this design is inflexible: we cannot test system-level\nparsing of the git configuration in our test harness because there is no\nway to change its location, so all tests run with `GIT_CONFIG_NOSYSTEM`\nset.\n\nInstead of doing the same mistake with `GIT_CONFIG_NOGLOBAL`, introduce\ntwo new variables `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM`:\n\n - If unset, git continues to use the usual locations.\n\n - If set to a specific path, we skip reading the normal\n configuration files and instead take the path. By setting the path\n to `/dev/null`, no configuration will be loaded for the respective\n level.\n\nThis implements the usecase where we want to execute code in a sanitized\nenvironment without any potential misconfigurations via `/dev/null`, but\nis more flexible and allows for more usecases than simply adding\n`GIT_CONFIG_NOGLOBAL`.\n\nSigned-off-by: Patrick Steinhardt \n---\n Documentation/git-config.txt | 5 +++\n Documentation/git.txt | 10 +++++\n config.c | 17 +++++--\n t/t1300-config.sh | 86 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 115 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 4b4cc5c5e8..5cddadafd2 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -340,6 +340,11 @@ 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_GLOBAL::\n+GIT_CONFIG_SYSTEM::\n+\tTake the configuration from the given files instead from global or\n+\tsystem-level configuration. See linkgit:git[1] for details.\n+\n GIT_CONFIG_NOSYSTEM::\n \tWhether to skip reading settings from the system-wide\n \t$(prefix)/etc/gitconfig file. See linkgit:git[1] for details.\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 3a9c44987f..380422a6a9 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -670,6 +670,16 @@ for further details.\n \tIf this environment variable is set to `0`, git will not prompt\n \ton the terminal (e.g., when asking for HTTP authentication).\n \n+`GIT_CONFIG_GLOBAL`::\n+`GIT_CONFIG_SYSTEM`::\n+\tTake the configuration from the given files instead from global or\n+\tsystem-level configuration files. If `GIT_CONFIG_SYSTEM` is set, the\n+\tsystem config file defined at build time (usually `/etc/gitconfig`)\n+\twill not be read. Likewise, if `GIT_CONFIG_GLOBAL` is set, neither\n+\t`$HOME/.gitconfig` nor `$XDG_CONFIG_HOME/git/config` will be read. Can\n+\tbe set to `/dev/null` to skip reading configuration files of the\n+\trespective level.\n+\n `GIT_CONFIG_NOSYSTEM`::\n \tWhether to skip reading settings from the system-wide\n \t`$(prefix)/etc/gitconfig` file. This environment variable can\ndiff --git a/config.c b/config.c\nindex 161dfaa707..f9c400ad30 100644\n--- a/config.c\n+++ b/config.c\n@@ -1832,13 +1832,24 @@ static int git_config_from_blob_ref(config_fn_t fn,\n \n char *git_system_config(void)\n {\n+\tchar *system_config = xstrdup_or_null(getenv(\"GIT_CONFIG_SYSTEM\"));\n+\tif (system_config)\n+\t\treturn system_config;\n \treturn system_path(ETC_GITCONFIG);\n }\n \n-void git_global_config(char **user_config, char **xdg_config)\n+void git_global_config(char **user_out, char **xdg_out)\n {\n-\t*user_config = expand_user_path(\"~/.gitconfig\", 0);\n-\t*xdg_config = xdg_config_home(\"config\");\n+\tchar *user_config = xstrdup_or_null(getenv(\"GIT_CONFIG_GLOBAL\"));\n+\tchar *xdg_config = NULL;\n+\n+\tif (!user_config) {\n+\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n+\t\txdg_config = xdg_config_home(\"config\");\n+\t}\n+\n+\t*user_out = user_config;\n+\t*xdg_out = xdg_config;\n }\n \n /*\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex e0dd5d65ce..0f92dfe6fb 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -2059,6 +2059,92 @@ test_expect_success '--show-scope with --show-origin' '\n \ttest_cmp expect output\n '\n \n+test_expect_success 'override global and system config' '\n+\ttest_when_finished rm -f \"$HOME\"/.config/git &&\n+\n+\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n+\t[home]\n+\t\tconfig = true\n+\tEOF\n+\tmkdir -p \"$HOME\"/.config/git &&\n+\tcat >\"$HOME\"/.config/git/config <<-EOF &&\n+\t[xdg]\n+\t\tconfig = true\n+\tEOF\n+\tcat >.git/config <<-EOF &&\n+\t[local]\n+\t\tconfig = true\n+\tEOF\n+\tcat >custom-global-config <<-EOF &&\n+\t[global]\n+\t\tconfig = true\n+\tEOF\n+\tcat >custom-system-config <<-EOF &&\n+\t[system]\n+\t\tconfig = true\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\tglobal\txdg.config=true\n+\tglobal\thome.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tsane_unset GIT_CONFIG_NOSYSTEM &&\n+\n+\tcat >expect <<-EOF &&\n+\tsystem\tsystem.config=true\n+\tglobal\tglobal.config=true\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \\\n+\t\tgit config --show-scope --list >output &&\n+\ttest_cmp expect output &&\n+\n+\tcat >expect <<-EOF &&\n+\tlocal\tlocal.config=true\n+\tEOF\n+\tGIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null git config --show-scope --list >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'override global and system config with missing file' '\n+\ttest_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list &&\n+\ttest_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list &&\n+\tGIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version\n+'\n+\n+test_expect_success 'system override has no effect with GIT_CONFIG_NOSYSTEM' '\n+\t# `git config --system` has different semantics compared to other\n+\t# commands as it ignores GIT_CONFIG_NOSYSTEM. We thus test whether the\n+\t# variable has an effect via a different proxy.\n+\tcat >alias-config <<-EOF &&\n+\t[alias]\n+\t\thello-world = !echo \"hello world\"\n+\tEOF\n+\ttest_must_fail env GIT_CONFIG_NOSYSTEM=true GIT_CONFIG_SYSTEM=alias-config \\\n+\t\tgit hello-world &&\n+\tGIT_CONFIG_NOSYSTEM=false GIT_CONFIG_SYSTEM=alias-config \\\n+\t\tgit hello-world >actual &&\n+\techo \"hello world\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'write to overridden global and system config' '\n+\tcat >expect < writes:\n\n> this is the fifth version of my patch series to provide a way of\n> overriding the global system configuration.\n\nHmm, this topic has been in next since April 15th. It is\npreferrable to fix things with incremental updates (the reasoning\nbeing that while in 'seen' the issues are \"mistakes by the author\nalone\", which may not necessarily benefit others to learn from, but\nwhat is in 'next' are supposed to have got enough review exposure,\nso any mistakes found are both of the author and reviewers, i.e.\ntrickier than what is found before it hits 'next' and are more worth\ndocumenting with incremental \"oops that was wrong for such and such\nreason, which even our reviews missed, and here is a fix\").\n\nBut I'll have to go offline for a while soon, so let's make an\nexception, revert the merge of the previous round out of 'next', and\nqueue this in 'seen'.\n\nThanks.\n"},{"id":"422632","messageId":"20210421204637.GA2947267@szeder.dev","threadId":"55455","inReplyTo":"7e7506217e4aac82e3a03013d24e6f885383ecd3.1618835148.git.ps@pks.im","subject":"Re: [PATCH v5 3/3] config: allow overriding of global and system configuration","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-04-21T20:46:37Z","receivedAt":"2021-04-21T20:46:45Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Apr 19, 2021 at 02:31:16PM +0200, Patrick Steinhardt wrote:\n> In order to have git run in a fully controlled environment without any\n> misconfiguration, it may be desirable for users or scripts to override\n> global- and system-level configuration files. We already have a way of\n> doing this, which is to unset both HOME and XDG_CONFIG_HOME environment\n> variables and to set `GIT_CONFIG_NOGLOBAL=true`. This is quite kludgy,\n> and unsetting the first two variables likely has an impact on other\n> executables spawned by such a script.\n> \n> The obvious way to fix this would be to introduce `GIT_CONFIG_NOGLOBAL`\n> as an equivalent to `GIT_CONFIG_NOSYSTEM`. But in the past, it has\n> turned out that this design is inflexible: we cannot test system-level\n> parsing of the git configuration in our test harness because there is no\n> way to change its location, so all tests run with `GIT_CONFIG_NOSYSTEM`\n> set.\n> \n> Instead of doing the same mistake with `GIT_CONFIG_NOGLOBAL`, introduce\n> two new variables `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM`:\n> \n> - If unset, git continues to use the usual locations.\n> \n> - If set to a specific path, we skip reading the normal\n> configuration files and instead take the path. By setting the path\n> to `/dev/null`, no configuration will be loaded for the respective\n> level.\n> \n> This implements the usecase where we want to execute code in a sanitized\n> environment without any potential misconfigurations via `/dev/null`, but\n> is more flexible and allows for more usecases than simply adding\n> `GIT_CONFIG_NOGLOBAL`.\n\nSomething is still not right with this patch series, because:\n\n> +test_expect_success 'write to overridden global and system config' '\n> +\tcat >expect < +[config]\n> +\tkey = value\n> +EOF\n> +\n> +\tGIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n> +\ttest_cmp expect write-to-global &&\n> +\n> +\tGIT_CONFIG_SYSTEM=write-to-system git config --system config.key value &&\n> +\ttest_cmp expect write-to-system\n> +'\n\nThis test fails on Travis CI's Linux32 job:\n\n expecting success of 1300.184 'write to overridden global and system config': \n \tcat >expect < On Mon, Apr 19, 2021 at 02:31:16PM +0200, Patrick Steinhardt wrote:\n> > In order to have git run in a fully controlled environment without any\n> > misconfiguration, it may be desirable for users or scripts to override\n> > global- and system-level configuration files. We already have a way of\n> > doing this, which is to unset both HOME and XDG_CONFIG_HOME environment\n> > variables and to set `GIT_CONFIG_NOGLOBAL=true`. This is quite kludgy,\n> > and unsetting the first two variables likely has an impact on other\n> > executables spawned by such a script.\n> > \n> > The obvious way to fix this would be to introduce `GIT_CONFIG_NOGLOBAL`\n> > as an equivalent to `GIT_CONFIG_NOSYSTEM`. But in the past, it has\n> > turned out that this design is inflexible: we cannot test system-level\n> > parsing of the git configuration in our test harness because there is no\n> > way to change its location, so all tests run with `GIT_CONFIG_NOSYSTEM`\n> > set.\n> > \n> > Instead of doing the same mistake with `GIT_CONFIG_NOGLOBAL`, introduce\n> > two new variables `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM`:\n> > \n> > - If unset, git continues to use the usual locations.\n> > \n> > - If set to a specific path, we skip reading the normal\n> > configuration files and instead take the path. By setting the path\n> > to `/dev/null`, no configuration will be loaded for the respective\n> > level.\n> > \n> > This implements the usecase where we want to execute code in a sanitized\n> > environment without any potential misconfigurations via `/dev/null`, but\n> > is more flexible and allows for more usecases than simply adding\n> > `GIT_CONFIG_NOGLOBAL`.\n> \n> Something is still not right with this patch series, because:\n> \n> > +test_expect_success 'write to overridden global and system config' '\n> > +\tcat >expect < > +[config]\n> > +\tkey = value\n> > +EOF\n> > +\n> > +\tGIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n> > +\ttest_cmp expect write-to-global &&\n> > +\n> > +\tGIT_CONFIG_SYSTEM=write-to-system git config --system config.key value &&\n> > +\ttest_cmp expect write-to-system\n> > +'\n> \n> This test fails on Travis CI's Linux32 job:\n> \n> expecting success of 1300.184 'write to overridden global and system config': \n> \tcat >expect < [config]\n> \tkey = value\n> EOF\n> \tGIT_CONFIG_GLOBAL=write-to-global git config --global config.key value &&\n> \ttest_cmp expect write-to-global &&\n> \tGIT_CONFIG_SYSTEM=write-to-system git config --system config.key value &&\n> \ttest_cmp expect write-to-system\n> + cat\n> + GIT_CONFIG_GLOBAL=write-to-global git config --global config.key value\n> fatal: unable to access '/root/etc/gitconfig': Permission denied\n> error: last command exited with $?=128\n> not ok 184 - write to overridden global and system config\n\n https://travis-ci.org/github/git/git/jobs/767898817#L6931\n\n> Yeah, that job has a weird environment with Docker and 'su'\n> interacting in a way that ultimately builds Git with 'HOME=/root',\n> which in our build system means that 'sysconfdir=/root/etc'. To\n> reproduce at home just run:\n> \n> make prefix=/root && cd t && ./t1300-config.sh -V -x -i\n\nHrm, that's not the only test that fails, but I only ran it with\n'-i'... but in fact most subsequent tests fail with the same error.\n\nI think the culprit is the previous test case which I too eagerly\nsnipped from my previous email, so here it is again (copy-pasted,\nwhitespace-damaged):\n\n> test_expect_success 'override global and system config' '\n> test_when_finished rm -f \"$HOME\"/.config/git &&\n> \n> cat >\"$HOME\"/.gitconfig <<-EOF &&\n> [home]\n> config = true\n> EOF\n> mkdir -p \"$HOME\"/.config/git &&\n> cat >\"$HOME\"/.config/git/config <<-EOF &&\n> [xdg]\n> config = true\n> EOF\n> cat >.git/config <<-EOF &&\n> [local]\n> config = true\n> EOF\n> cat >custom-global-config <<-EOF &&\n> [global]\n> config = true\n> EOF\n> cat >custom-system-config <<-EOF &&\n> [system]\n> config = true\n> EOF\n> \n> cat >expect <<-EOF &&\n> global xdg.config=true\n> global home.config=true\n> local local.config=true\n> EOF\n> git config --show-scope --list >output &&\n> test_cmp expect output &&\n> \n> sane_unset GIT_CONFIG_NOSYSTEM &&\n\nUnsetting GIT_CONFIG_NOSYSTEM like this does affect the environment of\nall subsequent tests and their git commands will then try to look at\nthe system config file.\n\nPutting this 'sane_unset' and the rest of this test case in a subshell\nseems to fix the issue.\n\n> cat >expect <<-EOF &&\n> system system.config=true\n> global global.config=true\n> local local.config=true\n> EOF\n> GIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \\\n> git config --show-scope --list >output &&\n> test_cmp expect output &&\n> \n> cat >expect <<-EOF &&\n> local local.config=true\n> EOF\n> GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null git config --show-scope --list >output &&\n> test_cmp expect output\n> '\n\n\n"},{"id":"422666","messageId":"YIELbYi5Wz6OALqL@ncase","threadId":"55455","inReplyTo":"20210421210614.GB2947267@szeder.dev","subject":"Re: [PATCH v5 3/3] config: allow overriding of global and system configuration","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-22T05:36:45Z","receivedAt":"2021-04-22T05:37:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Apr 21, 2021 at 11:06:14PM +0200, SZEDER Gábor wrote:\n> On Wed, Apr 21, 2021 at 10:46:37PM +0200, SZEDER Gábor wrote:\n> > On Mon, Apr 19, 2021 at 02:31:16PM +0200, Patrick Steinhardt wrote:\n[snip]\n> > test_expect_success 'override global and system config' '\n> > test_when_finished rm -f \"$HOME\"/.config/git &&\n> > \n> > cat >\"$HOME\"/.gitconfig <<-EOF &&\n> > [home]\n> > config = true\n> > EOF\n> > mkdir -p \"$HOME\"/.config/git &&\n> > cat >\"$HOME\"/.config/git/config <<-EOF &&\n> > [xdg]\n> > config = true\n> > EOF\n> > cat >.git/config <<-EOF &&\n> > [local]\n> > config = true\n> > EOF\n> > cat >custom-global-config <<-EOF &&\n> > [global]\n> > config = true\n> > EOF\n> > cat >custom-system-config <<-EOF &&\n> > [system]\n> > config = true\n> > EOF\n> > \n> > cat >expect <<-EOF &&\n> > global xdg.config=true\n> > global home.config=true\n> > local local.config=true\n> > EOF\n> > git config --show-scope --list >output &&\n> > test_cmp expect output &&\n> > \n> > sane_unset GIT_CONFIG_NOSYSTEM &&\n> \n> Unsetting GIT_CONFIG_NOSYSTEM like this does affect the environment of\n> all subsequent tests and their git commands will then try to look at\n> the system config file.\n> \n> Putting this 'sane_unset' and the rest of this test case in a subshell\n> seems to fix the issue.\n\nAh, that makes sense. Thanks for digging into the issue, I'll send a fix\nfor this later today.\n\nPatrick\n"},{"id":"422710","messageId":"5b69d9d34e4bdf1c7eb3b172ad8ef0f78542c789.1619156659.git.ps@pks.im","threadId":"55455","inReplyTo":"20210421204637.GA2947267@szeder.dev","subject":"[PATCH] t1300: fix unset of GIT_CONFIG_NOSYSTEM leaking into subsequent tests","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-04-23T05:47:15Z","receivedAt":"2021-04-23T05:47:27Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In order to test whether the new GIT_CONFIG_SYSTEM environment variable\nbehaves as expected, we unset GIT_CONFIG_NOSYSTEM in one of our tests in\nt1300. But because tests are not executed in a subshell, this unset\nleaks into all subsequent tests and may thus cause them to fail in some\nenvironments. These failures are easily reproducable with `make\nprefix=/root test`.\n\nFix the issue by not using `sane_unset GIT_CONFIG_NOSYSTEM`, but instead\njust manually add it to the environment of the two command invocations\nwhich need it.\n\nReported-by: SZEDER Gábor \nSigned-off-by: Patrick Steinhardt \n---\n\nThis patch applies on top of 47e6f16901 (Sync with master, 2021-04-20),\nwhich is the tip of next at the time of writing.\n\n t/t1300-config.sh | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex 0f92dfe6fb..ec599baeba 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -2092,21 +2092,20 @@ test_expect_success 'override global and system config' '\n \tgit config --show-scope --list >output &&\n \ttest_cmp expect output &&\n \n-\tsane_unset GIT_CONFIG_NOSYSTEM &&\n-\n \tcat >expect <<-EOF &&\n \tsystem\tsystem.config=true\n \tglobal\tglobal.config=true\n \tlocal\tlocal.config=true\n \tEOF\n-\tGIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \\\n+\tGIT_CONFIG_NOSYSTEM=false GIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \\\n \t\tgit config --show-scope --list >output &&\n \ttest_cmp expect output &&\n \n \tcat >expect <<-EOF &&\n \tlocal\tlocal.config=true\n \tEOF\n-\tGIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null git config --show-scope --list >output &&\n+\tGIT_CONFIG_NOSYSTEM=false GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null \\\n+\t\tgit config --show-scope --list >output &&\n \ttest_cmp expect output\n '\n \n-- \n2.31.1\n\n"},{"id":"422738","messageId":"YIKTDjOX3WtBjqAq@coredump.intra.peff.net","threadId":"55455","inReplyTo":"YH1jnIhuF4UjXVNW@tanuki","subject":"Re: [PATCH v4 1/3] config: rename `git_etc_config()`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-23T09:27:42Z","receivedAt":"2021-04-23T09:27:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 19, 2021 at 01:03:56PM +0200, Patrick Steinhardt wrote:\n\n> It's only by accident that I dropped the call to `git_config_system()`,\n> must've happened when resolving conflicts somewhere. The issue with just\n> returning `/dev/null` from `git_system_config()` is that git-config(1)\n> would be broken, as you hint at. We do not honor GIT_CONFIG_NOSYSTEM\n> there if the \"--system\" flag was given.\n> \n> So yes, we could change it to return `/dev/null`, but that would change\n> semantics of GIT_CONFIG_NOSYSTEM. I'm not sure doing this in the same\n> series is a good idea. Even more so because with returning `/dev/null`,\n> the conversion would be silent: whereas previous versions simply wrote\n> to or read from the system-level config, we now pretend to have read or\n> written something even though we didn't.\n\nI'm OK being more cautious here, and leaving NOSYSTEM as it is.\n\nMy original thinking was that returning /dev/null would be an\nimprovement even for the existing call in \"git config --system\". And I\ndo think it would be for reading. But it actually gets pretty weird for\nwriting, because of our atomic-rename strategy.\n\nAnd that is actually true of your new variable too. Doing this:\n\n GIT_CONFIG_SYSTEM=/dev/null git config --system foo.bar\n\nwould return nothing, which makes sense to me. But this:\n\n GIT_CONFIG_SYSTEM=/dev/null git config --system foo.bar value\n\nwould try to write /dev/null.lock, and then rename it into place over\nthe real /dev/null! That may be slightly surprising, but it is not\nreally any different than any other non-regular-file entry.\n\nThe stakes are slightly higher here, as it breaks all sorts of things\nthat can be hard to recover from (even if you know the correct mknod\nincantation, important things like bash seem to get crabby if they can't\nopen /dev/null for writing). And it may be more likely to happen by\nmistake (as opposed to \"git config --file=/dev/null\", which behaves the\nsame). But unless you are root, you are likely to just get an error in\nthe first place, so it seems like an unlikely mistake in practice.\n\nSo I think it may fall into \"if it hurts, don't do it\".\n\n-Peff\n"},{"id":"422739","messageId":"YIKUJMyxL17I+42V@coredump.intra.peff.net","threadId":"55455","inReplyTo":"cover.1618835148.git.ps@pks.im","subject":"Re: [PATCH v5 0/3] config: allow overriding global/system config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-23T09:32:20Z","receivedAt":"2021-04-23T09:32:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 19, 2021 at 02:31:04PM +0200, Patrick Steinhardt wrote:\n\n> this is the fifth version of my patch series to provide a way of\n> overriding the global system configuration.\n> \n> Changes to v4:\n> \n> - Readded the call to `git_config_system()`, which I've previously\n> dropped by accident. I didn't move it into the new\n> `git_system_config()` function as it would change semantics of\n> `git config --system`.\n> \n> - Added a testcase which verifies that GIT_CONFIG_NOSYSTEM and\n> GIT_CONFIG_SYSTEM properly interact with each other: if\n> GIT_CONFIG_NOSYSTEM is set, no system-level configuration shall be\n> read. This is different than the tests for `git config --system`\n> which used to and still does ignore GIT_CONFIG_NOSYSTEM.\n> \n> - Small fixups for another testcase to drop needless redirects and\n> the `sane_unset` of GIT_CONFIG_NOSYSTEM.\n\nWith the extra fixup to avoid unsetting GIT_CONFIG_NOSYSTEM for the\nwhole test suite, this looks good to me.\n\n-Peff\n"}]}