| Message ID | a23382059bb57022dd1e40d1c2c9a11307b0ff3b.1617891426.git.ps@pks.im (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | config: Introduce GIT_CONFIG_NOGLOBAL | expand |
On Thu, Apr 8, 2021 at 10:18 AM Patrick Steinhardt <ps@pks.im> wrote: > While it's already possible to stop git from reading the system config > via GIT_CONFIG_NOSYSTEM, doing the same for global config files requires > the user to unset both HOME and XDG_CONFIG_HOME. This is an awkward > interface and may even pose a problem e.g. when git hooks rely on these > variables to be present. > > Introduce a new GIT_CONFIG_NOGLOBAL envvar, which is the simple > equivalent to GIT_CONFIG_NOSYSTEM. If set to true, git will skip reading > both `~/.gitconfig` and `$XDG_CONFIG_HOME/git/config`. > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > diff --git a/Documentation/git.txt b/Documentation/git.txt > @@ -670,13 +670,21 @@ for further details. > +`GIT_CONFIG_NOGLOBAL`:: > + Whether to skip reading settings from the system-wide `~/.gitconfig` > + and `$XDG_CONFIG_HOME/git/config` files. This environment variable can > + be used along with `$GIT_CONFIG_NOSYSTEM` to create a predictable > + environment for a picky script, or you can set it temporarily to avoid > + using a buggy global config file while waiting for someone with > + sufficient permissions to fix it. Not necessarily a new problem since you mostly copied the new text from GIT_CONFIG_NOSYSTEM, but this doesn't tell the reader what value to assign to this variable. As currently written, I would end up having to consult the source code to figure out how to use this variable, which makes the documentation less useful than it should be. Perhaps it could be rewritten something like: If set to any value, suppress reading global configuration from `~/.gitconfig` and `$XDG_CONFIG_HOME/git/config` files. This environment variable... The bit about waiting for someone to fix a buggy ~/.gitconfig is somewhat questionable; it might make sense to drop everything after "picky script". > `GIT_CONFIG_NOSYSTEM`:: > Whether to skip reading settings from the system-wide > `$(prefix)/etc/gitconfig` file. This environment variable can > - be used along with `$HOME` and `$XDG_CONFIG_HOME` to create a > - predictable environment for a picky script, or you can set it > - temporarily to avoid using a buggy `/etc/gitconfig` file while > - waiting for someone with sufficient permissions to fix it. > + be used along with `$GIT_CONFIG_NOGLOBAL` to create a predictable > + environment for a picky script, or you can set it temporarily to avoid > + using a buggy `/etc/gitconfig` file while waiting for someone with > + sufficient permissions to fix it. This suffers the same problem of not telling the reader what value to assign. A similar rewrite could improve it, as well.
Patrick Steinhardt <ps@pks.im> writes: > While it's already possible to stop git from reading the system config > via GIT_CONFIG_NOSYSTEM, doing the same for global config files requires > the user to unset both HOME and XDG_CONFIG_HOME. This is an awkward > interface and may even pose a problem e.g. when git hooks rely on these > variables to be present. Yeah, having to unset HOME would affect not just Git. Git may call out something else (e.g. an editor) that may want to know where HOME is, Git may be used in something else (e.g. an IDE) that may want to know where HOME is. Same for XDG_CONFIG_HOME. If it is a valid need to not reading from $HOME/.gitconfig, unsetting HOME is an unacceptable approach to satisfying that need. > Introduce a new GIT_CONFIG_NOGLOBAL envvar, which is the simple > equivalent to GIT_CONFIG_NOSYSTEM. If set to true, git will skip reading > both `~/.gitconfig` and `$XDG_CONFIG_HOME/git/config`. I do not think we'd add an unbounded number of new configuration sources to the existing three-level hierarchy of system-wide (/etc/gitconfig), per-user ($HOME/.gitconfig), and local (.git/config), so it is not too bad, from scalability point of view, to keep adding new GIT_CONFIG_NO$FROTZ variables. Let me go in a couple of different tangents a bit, thinking aloud. Tangent One. I wonder if the presense of includeIf.<cond>.path changes the "we won't have unbounded, so adding another here is OK" reasoning. If somebody wanted to say "Do not use the paths that match this and that pattern", it is likely that we'd end up having to support a single variable that allows more than one "value". In a straw-man syntax "GIT_CONFIG_EXCLUDE='/home/gitster/* /home/peff/*'" might be a way to say "do not use files that are under these two directories. And when that happens, we'd probably notice that it is easier for users to configure, if they can treat 'system' and 'global' as just another special pattern to place on that list. //system and //global might be the syntax we'd choose when time comes, i.e. GIT_CONFIG_EXCLUDE='//system //global' might become a more scalable replacement for GIT_CONFIG_NOSYSTEM=yes GIT_CONFIG_NOHOME=yes Tangent Two. One thing we never managed to properly test in our test suite is the unctioning of GIT_CONFIG_NOSYSTEM. As we do not want to get broken by whatever is in /etc/gitconfig, all our tests run with the environment variable set. For the same reason, in order to avoid getting influenced by whatever the tester has in $HOME/.gitconfig, we export HOME set to the throw-away directory created during the test and control what is in the config file in that directory. In hindsight, it might have been a better design to use GIT_CONFIG_SYSTEM variable that points at a path to be used as a replacement for /etc/gitconfig when it is set; pointing /dev/null with the variable would have been the natural way to say "do not use anything from system configuration". And GIT_CONFIG_GLOBAL can easily follow the same pattern. So, from these two perspective, for the longer term end-user experience, I am not 100% onboard with GIT_CONFIG_NOGLOBAL. An alternative that allows us to move away from GIT_CONFIG_NOSYSTEM in the direction to the tangent #2 would be not to repeat the same mistake by doing GIT_CONFIG_NOGLOBAL, and instead adding GIT_CONFIG_GLOBAL, which is (1) when not set, it does not do anything, (2) when set to "/dev/null" (a literal string; you do not have to spell it "NUL" when on Windows), it acts just like the case where your GIT_CONFIG_NOSYSTEM is set, (3) when set to any other string, it is taken as a filename that has the global configuration. Unlike $HOME/.gitconfig or $XDG_HOME/git/config, it is an error if the named file does not exist (this is to catch misconfiguration). And once this is accepted by users and established as a pattern, we could migrate GIT_CONFIG_NOSYSTEM to GIT_CONFIG_SYSTEM=/dev/null Having said all that (meaning: I am not 100% onboard with _NOGLOBAL and think _GLOBAL=/dev/null might be a better design), let's give it a review under the assumption that _NOGLOBAL is the design we would want to choose. > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt > index 4b4cc5c5e8..88cd064abb 100644 > --- a/Documentation/git-config.txt > +++ b/Documentation/git-config.txt > @@ -340,6 +340,10 @@ GIT_CONFIG:: > Using the "--global" option forces this to ~/.gitconfig. Using the > "--system" option forces this to $(prefix)/etc/gitconfig. > > +GIT_CONFIG_NOGLOBAL:: > + Whether to skip reading settings from the global ~/.gitconfig and > + $XDG_CONFIG_HOME/git/config files. See linkgit:git[1] for details. > + > GIT_CONFIG_NOSYSTEM:: > Whether to skip reading settings from the system-wide > $(prefix)/etc/gitconfig file. See linkgit:git[1] for details. OK. The new one mimics existing _NOSYSTEM and there is nothing surprising in the new description. > diff --git a/Documentation/git.txt b/Documentation/git.txt > index 3a9c44987f..4462bd2da9 100644 > --- a/Documentation/git.txt > +++ b/Documentation/git.txt > @@ -670,13 +670,21 @@ for further details. > If this environment variable is set to `0`, git will not prompt > on the terminal (e.g., when asking for HTTP authentication). > > +`GIT_CONFIG_NOGLOBAL`:: > + Whether to skip reading settings from the system-wide `~/.gitconfig` > + and `$XDG_CONFIG_HOME/git/config` files. This environment variable can > + be used along with `$GIT_CONFIG_NOSYSTEM` to create a predictable > + environment for a picky script, or you can set it temporarily to avoid > + using a buggy global config file while waiting for someone with > + sufficient permissions to fix it. "while waiting for someone with permissions" is a good backstory for inventing _NOSYSTEM because you might not be able to futz with /etc/gitconfig, but does not sound like an appropriate reasoning for _NOGLOBAL that is about $HOME/.gitconfig or $HOME/.config/git/config. > `GIT_CONFIG_NOSYSTEM`:: > Whether to skip reading settings from the system-wide > `$(prefix)/etc/gitconfig` file. This environment variable can > - be used along with `$HOME` and `$XDG_CONFIG_HOME` to create a > - predictable environment for a picky script, or you can set it > - temporarily to avoid using a buggy `/etc/gitconfig` file while > - waiting for someone with sufficient permissions to fix it. > + be used along with `$GIT_CONFIG_NOGLOBAL` to create a predictable > + environment for a picky script, or you can set it temporarily to avoid > + using a buggy `/etc/gitconfig` file while waiting for someone with > + sufficient permissions to fix it. > diff --git a/config.c b/config.c > index 6428393a41..19c1b31c75 100644 > --- a/config.c > +++ b/config.c > @@ -1879,6 +1879,11 @@ int git_config_system(void) > return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0); > } > > +static int git_config_global(void) > +{ > + return !git_env_bool("GIT_CONFIG_NOGLOBAL", 0); > +} > + > static int do_git_config_sequence(const struct config_options *opts, > config_fn_t fn, void *data) > { > @@ -1903,10 +1908,10 @@ static int do_git_config_sequence(const struct config_options *opts, > data); > > current_parsing_scope = CONFIG_SCOPE_GLOBAL; > - if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) > + if (git_config_global() && xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) > ret += git_config_from_file(fn, xdg_config, data); > > - if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) > + if (git_config_global() && user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) > ret += git_config_from_file(fn, user_config, data); > > current_parsing_scope = CONFIG_SCOPE_LOCAL; The implementation itself is quite straightforward for _NOSYSTEM; I see nothing wrong in the code change for the given design. > diff --git a/t/t1300-config.sh b/t/t1300-config.sh > index e0dd5d65ce..0754189974 100755 > --- a/t/t1300-config.sh > +++ b/t/t1300-config.sh > @@ -2059,6 +2059,37 @@ test_expect_success '--show-scope with --show-origin' ' > test_cmp expect output > ' > > +test_expect_success 'GIT_CONFIG_NOGLOBAL' ' > + test_when_finished rm -f "$HOME"/.config/git && > + cat >"$HOME"/.gitconfig <<-EOF && > + [home] > + config = true > + EOF > + mkdir -p "$HOME"/.config/git && > + cat >"$HOME"/.config/git/config <<-EOF && > + [xdg] > + config = true > + EOF > + cat >.git/config <<-EOF && > + [local] > + config = true > + EOF > + > + cat >expect <<-EOF && > + global xdg.config=true > + global home.config=true > + local local.config=true > + EOF > + git config --show-scope --list >output && > + test_cmp expect output && > + > + cat >expect <<-EOF && > + local local.config=true > + EOF > + GIT_CONFIG_NOGLOBAL=true git config --show-scope --list >output && > + test_cmp expect output > +' This test was what initially piqued my curiosity, as the fact that the result filtered with the new mechanism has only 'local' misled me to incorrectly think that we are suppressing both 'system' and 'global' with the single variable. Until I realized that we cannot test the 'system' configuration in our test suite, that is. Thanks.
On Thu, Apr 08, 2021 at 10:25:15AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > While it's already possible to stop git from reading the system config > > via GIT_CONFIG_NOSYSTEM, doing the same for global config files requires > > the user to unset both HOME and XDG_CONFIG_HOME. This is an awkward > > interface and may even pose a problem e.g. when git hooks rely on these > > variables to be present. > > Yeah, having to unset HOME would affect not just Git. Git may call > out something else (e.g. an editor) that may want to know where HOME > is, Git may be used in something else (e.g. an IDE) that may want to > know where HOME is. Same for XDG_CONFIG_HOME. If it is a valid need > to not reading from $HOME/.gitconfig, unsetting HOME is an unacceptable > approach to satisfying that need. We actually used to have GIT_CONFIG_NOGLOBAL, which was used in the test suite. But after switching to setting $HOME, it went away in 8f323c00dd (config: drop support for GIT_CONFIG_NOGLOBAL, 2011-03-15). I agree that it's a little more awkward these days because of $XDG_CONFIG_HOME (and also because it influences other programs besides Git). I'm not particularly opposed to re-adding it, but I do think having an environment variable for "read this file instead", as you suggested below, would be much better. It's a superset of the "no" functionality, and would also let us improve our test coverage (especially if we add a matching one for "system" config). Looking at your suggestion: > So, from these two perspective, for the longer term end-user > experience, I am not 100% onboard with GIT_CONFIG_NOGLOBAL. An > alternative that allows us to move away from GIT_CONFIG_NOSYSTEM in > the direction to the tangent #2 would be not to repeat the same > mistake by doing GIT_CONFIG_NOGLOBAL, and instead adding > GIT_CONFIG_GLOBAL, which is > > (1) when not set, it does not do anything, > > (2) when set to "/dev/null" (a literal string; you do not have to > spell it "NUL" when on Windows), it acts just like the case > where your GIT_CONFIG_NOSYSTEM is set, > > (3) when set to any other string, it is taken as a filename that > has the global configuration. Unlike $HOME/.gitconfig or > $XDG_HOME/git/config, it is an error if the named file does not > exist (this is to catch misconfiguration). > > And once this is accepted by users and established as a pattern, we > could migrate GIT_CONFIG_NOSYSTEM to GIT_CONFIG_SYSTEM=/dev/null That seems pretty reasonable. I'm on the fence on your (3). Conceivably somebody could want to override the baked-in defaults without being sure the file is present. But I'm not sure how useful that would be in practice. Some other things to consider: - does setting GIT_CONFIG_GLOBAL override both the $HOME and $XDG_CONFIG_HOME? If the plan is to override them, that makes sense. But we do usually read from both of them, so conceivably you might want to override just one? That's probably over-engineering, though. - if we have config for "read from this file instead of the system config" and "read from this instead of the user-level config", then I wonder if people will want "read this instead of the repo config". We have resisted having GIT_CONFIG mean that for many years, because I think it gets awkward in some cases (e.g., we'd still want to read it for core.repositoryformatversion, etc). I'm OK with drawing the line there and saying it's not a support feature, but we should be prepared to explain it to users (in the docs or at least in the commit message adding the system/global override variables). -Peff
On Thu, Apr 08 2021, Patrick Steinhardt wrote: > +`GIT_CONFIG_NOGLOBAL`:: > + Whether to skip reading settings from the system-wide `~/.gitconfig` > + and `$XDG_CONFIG_HOME/git/config` files. This environment variable can Let's not call ~/.gitconfig system-wide with /etc/gitconfig being, saying "global" is consistent with git-config's own --global/--system/--local etc. Still a bit odd, but at least the same nomenclature.
On Thu, Apr 08 2021, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: >> +test_expect_success 'GIT_CONFIG_NOGLOBAL' ' >> + test_when_finished rm -f "$HOME"/.config/git && >> + cat >"$HOME"/.gitconfig <<-EOF && >> + [home] >> + config = true >> + EOF >> + mkdir -p "$HOME"/.config/git && >> + cat >"$HOME"/.config/git/config <<-EOF && >> + [xdg] >> + config = true >> + EOF >> + cat >.git/config <<-EOF && >> + [local] >> + config = true >> + EOF >> + >> + cat >expect <<-EOF && >> + global xdg.config=true >> + global home.config=true >> + local local.config=true >> + EOF >> + git config --show-scope --list >output && >> + test_cmp expect output && >> + >> + cat >expect <<-EOF && >> + local local.config=true >> + EOF >> + GIT_CONFIG_NOGLOBAL=true git config --show-scope --list >output && >> + test_cmp expect output >> +' > > This test was what initially piqued my curiosity, as the fact that > the result filtered with the new mechanism has only 'local' misled > me to incorrectly think that we are suppressing both 'system' and > 'global' with the single variable. Until I realized that we cannot > test the 'system' configuration in our test suite, that is. I haven't tested this, but that seems like a thing we want to mock in the test suite by just having git_etc_gitconfig() check a GIT_TEST_* variable. I had a git_env_str() in [1] that we could use for that, or maybe it should be git_env_path() in this case with the same --path handling, or just do what repo_default_branch_name() does. 1. https://lore.kernel.org/git/20201113161320.16458-1-avarab@gmail.com/
On Thu, Apr 08 2021, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > [...] >> Introduce a new GIT_CONFIG_NOGLOBAL envvar, which is the simple >> equivalent to GIT_CONFIG_NOSYSTEM. If set to true, git will skip reading >> both `~/.gitconfig` and `$XDG_CONFIG_HOME/git/config`. > > I do not think we'd add an unbounded number of new configuration > sources to the existing three-level hierarchy of system-wide > (/etc/gitconfig), per-user ($HOME/.gitconfig), and local > (.git/config), so it is not too bad, from scalability point of view, > to keep adding new GIT_CONFIG_NO$FROTZ variables. > > Let me go in a couple of different tangents a bit, thinking aloud. > > Tangent One. I wonder if the presense of includeIf.<cond>.path > changes the "we won't have unbounded, so adding another here is OK" > reasoning. If somebody wanted to say "Do not use the paths that > match this and that pattern", it is likely that we'd end up having > to support a single variable that allows more than one "value". In > a straw-man syntax "GIT_CONFIG_EXCLUDE='/home/gitster/* > /home/peff/*'" might be a way to say "do not use files that are > under these two directories. > > And when that happens, we'd probably notice that it is easier for > users to configure, if they can treat 'system' and 'global' as just > another special pattern to place on that list. //system and //global > might be the syntax we'd choose when time comes, i.e. > > GIT_CONFIG_EXCLUDE='//system //global' > > might become a more scalable replacement for > > GIT_CONFIG_NOSYSTEM=yes GIT_CONFIG_NOHOME=yes > > Tangent Two. One thing we never managed to properly test in our > test suite is the unctioning of GIT_CONFIG_NOSYSTEM. As we do not > want to get broken by whatever is in /etc/gitconfig, all our tests > run with the environment variable set. For the same reason, in > order to avoid getting influenced by whatever the tester has in > $HOME/.gitconfig, we export HOME set to the throw-away directory > created during the test and control what is in the config file in > that directory. In hindsight, it might have been a better design to > use GIT_CONFIG_SYSTEM variable that points at a path to be used as a > replacement for /etc/gitconfig when it is set; pointing /dev/null > with the variable would have been the natural way to say "do not use > anything from system configuration". And GIT_CONFIG_GLOBAL can > easily follow the same pattern. > > So, from these two perspective, for the longer term end-user > experience, I am not 100% onboard with GIT_CONFIG_NOGLOBAL. An > alternative that allows us to move away from GIT_CONFIG_NOSYSTEM in > the direction to the tangent #2 would be not to repeat the same > mistake by doing GIT_CONFIG_NOGLOBAL, and instead adding > GIT_CONFIG_GLOBAL, which is > > (1) when not set, it does not do anything, > > (2) when set to "/dev/null" (a literal string; you do not have to > spell it "NUL" when on Windows), it acts just like the case > where your GIT_CONFIG_NOSYSTEM is set, > > (3) when set to any other string, it is taken as a filename that > has the global configuration. Unlike $HOME/.gitconfig or > $XDG_HOME/git/config, it is an error if the named file does not > exist (this is to catch misconfiguration). > > And once this is accepted by users and established as a pattern, we > could migrate GIT_CONFIG_NOSYSTEM to GIT_CONFIG_SYSTEM=/dev/null > > > Having said all that (meaning: I am not 100% onboard with _NOGLOBAL > and think _GLOBAL=/dev/null might be a better design), let's give it > a review under the assumption that _NOGLOBAL is the design we would > want to choose. I think doing this via env vars is inherently nasty, but can't think of a good way to implement it properly without major refactoring. IMO the "properly" would be that we'd just support this sort of thing as first-class config syntax, as I've suggested before e.g. in the repo hooks/config topic. But to do that we couldn't "stream" the config reading as we do now, we'd need to read system/global/local, and when we saw some new meta-syntax apply those ignores to files we already read, and only then start streaming things to config callbacks. See "config.ignore.*" in https://lore.kernel.org/git/87y2ebo16v.fsf@evledraar.gmail.com/
Jeff King <peff@peff.net> writes: >> (3) when set to any other string, it is taken as a filename that >> has the global configuration. Unlike $HOME/.gitconfig or >> $XDG_HOME/git/config, it is an error if the named file does not >> exist (this is to catch misconfiguration). >> >> And once this is accepted by users and established as a pattern, we >> could migrate GIT_CONFIG_NOSYSTEM to GIT_CONFIG_SYSTEM=/dev/null > > That seems pretty reasonable. I'm on the fence on your (3). Conceivably > somebody could want to override the baked-in defaults without being sure > the file is present. But I'm not sure how useful that would be in > practice. I was also on the fence. If your plan is to create $HOME/.alternate-config and point at it by setting GIT_CONFIG_GLOBAL=$HOME/.alternate-config, there are two places you can make typo. You may write a file with a wrong name. You may export a variable with a wrong name. > Some other things to consider: > > - does setting GIT_CONFIG_GLOBAL override both the $HOME and > $XDG_CONFIG_HOME? If the plan is to override them, that makes sense. > But we do usually read from both of them, so conceivably you might > want to override just one? That's probably over-engineering, though. I viewed this to be working at the more conceptual "here is the file to read 'system' (or 'per-user') stuff from" level, and not at the level of the individual file, as I consider that it is a mere implementation detail that 'per-user' may read from multiple files. > - if we have config for "read from this file instead of the system > config" and "read from this instead of the user-level config", then > I wonder if people will want "read this instead of the repo config". > We have resisted having GIT_CONFIG mean that for many years, because > I think it gets awkward in some cases (e.g., we'd still want to read > it for core.repositoryformatversion, etc). I'm OK with drawing the > line there and saying it's not a support feature, but we should be > prepared to explain it to users (in the docs or at least in the > commit message adding the system/global override variables). We may have to bite the bullet and make an official catalog of really "structurely fundamental" configuration variables that must appear in the per-repository file, and a mechanism to enforce that by always reading these variables from "$GIT_DIR/config" and always ignoring appearances of them in any other places. That alone would probably be a good thing to do regardless of the GIT_CONFIG_NOGLOBAL issue, as I suspect you may be able to wreak havoc by adding random configuration like [extension] in $HOME/.gitconfig ;-) With that, it would make sense to allow overriding the per-repo configuration in a similar way, only as a mechanism to override configuration variables that are about "preferences" (as opposed to the structurally fundamental ones).
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Thu, Apr 08 2021, Patrick Steinhardt wrote: > >> +`GIT_CONFIG_NOGLOBAL`:: >> + Whether to skip reading settings from the system-wide `~/.gitconfig` >> + and `$XDG_CONFIG_HOME/git/config` files. This environment variable can > > Let's not call ~/.gitconfig system-wide with /etc/gitconfig being, > saying "global" is consistent with git-config's own > --global/--system/--local etc. Still a bit odd, but at least the same > nomenclature. Yeah, GLOBAL should be renamed to PER-USER everywhere.
On Thu, Apr 08, 2021 at 04:43:08PM -0700, Junio C Hamano wrote: > > Some other things to consider: > > > > - does setting GIT_CONFIG_GLOBAL override both the $HOME and > > $XDG_CONFIG_HOME? If the plan is to override them, that makes sense. > > But we do usually read from both of them, so conceivably you might > > want to override just one? That's probably over-engineering, though. > > I viewed this to be working at the more conceptual "here is the file > to read 'system' (or 'per-user') stuff from" level, and not at the > level of the individual file, as I consider that it is a mere > implementation detail that 'per-user' may read from multiple files. Yeah. I'm OK with that. I think it's conceptually simpler. I was going to also say "less flexible", but once you have pointed Git at the file of your choice, it is easy to [include] any other one if you want. > We may have to bite the bullet and make an official catalog of > really "structurely fundamental" configuration variables that must > appear in the per-repository file, and a mechanism to enforce that > by always reading these variables from "$GIT_DIR/config" and always > ignoring appearances of them in any other places. > > That alone would probably be a good thing to do regardless of the > GIT_CONFIG_NOGLOBAL issue, as I suspect you may be able to wreak > havoc by adding random configuration like [extension] in > $HOME/.gitconfig ;-) I think we've historically had some problems there, but I think these days it's not too bad. We look directly at $GIT_DIR/config with read_repository_format(), loading only stuff that check_repo_format() cares about: - core.repositoryformatversion - extensions.* - core.bare - core.worktree And those _should_ be ignored by the regular config-reading paths. So I think it would be OK to have a $GIT_CONFIG_LOCAL variable that overrides the location in do_git_config_sequence() only, and that would cover only the preferences parts. I do agree it wouldn't hurt to document which options are read in which context. I do wonder how useful such an option would be, though. I can see why you would want to redirect or disable system-level or user-level config once, and then use it for many invocations. But in a single repo, you might as well just set the repo config, or override it for a single invocation with "git -c", etc. I guess one possible use could be "I got a repository I do not trust as a tarball and want to investigate it without paying attention to its local config". But that's a pretty narrow case, and you'd probably be better off just vetting the config yourself anyway (since depending on what you want to do in the repo, you may need some of that config, like remote.*, etc). I dunno. -Peff
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 4b4cc5c5e8..88cd064abb 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -340,6 +340,10 @@ GIT_CONFIG:: Using the "--global" option forces this to ~/.gitconfig. Using the "--system" option forces this to $(prefix)/etc/gitconfig. +GIT_CONFIG_NOGLOBAL:: + Whether to skip reading settings from the global ~/.gitconfig and + $XDG_CONFIG_HOME/git/config files. See linkgit:git[1] for details. + GIT_CONFIG_NOSYSTEM:: Whether to skip reading settings from the system-wide $(prefix)/etc/gitconfig file. See linkgit:git[1] for details. diff --git a/Documentation/git.txt b/Documentation/git.txt index 3a9c44987f..4462bd2da9 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -670,13 +670,21 @@ for further details. If this environment variable is set to `0`, git will not prompt on the terminal (e.g., when asking for HTTP authentication). +`GIT_CONFIG_NOGLOBAL`:: + Whether to skip reading settings from the system-wide `~/.gitconfig` + and `$XDG_CONFIG_HOME/git/config` files. This environment variable can + be used along with `$GIT_CONFIG_NOSYSTEM` to create a predictable + environment for a picky script, or you can set it temporarily to avoid + using a buggy global config file while waiting for someone with + sufficient permissions to fix it. + `GIT_CONFIG_NOSYSTEM`:: Whether to skip reading settings from the system-wide `$(prefix)/etc/gitconfig` file. This environment variable can - be used along with `$HOME` and `$XDG_CONFIG_HOME` to create a - predictable environment for a picky script, or you can set it - temporarily to avoid using a buggy `/etc/gitconfig` file while - waiting for someone with sufficient permissions to fix it. + be used along with `$GIT_CONFIG_NOGLOBAL` to create a predictable + environment for a picky script, or you can set it temporarily to avoid + using a buggy `/etc/gitconfig` file while waiting for someone with + sufficient permissions to fix it. `GIT_FLUSH`:: If this environment variable is set to "1", then commands such diff --git a/config.c b/config.c index 6428393a41..19c1b31c75 100644 --- a/config.c +++ b/config.c @@ -1879,6 +1879,11 @@ int git_config_system(void) return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0); } +static int git_config_global(void) +{ + return !git_env_bool("GIT_CONFIG_NOGLOBAL", 0); +} + static int do_git_config_sequence(const struct config_options *opts, config_fn_t fn, void *data) { @@ -1903,10 +1908,10 @@ static int do_git_config_sequence(const struct config_options *opts, data); current_parsing_scope = CONFIG_SCOPE_GLOBAL; - if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) + if (git_config_global() && xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) ret += git_config_from_file(fn, xdg_config, data); - if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) + if (git_config_global() && user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) ret += git_config_from_file(fn, user_config, data); current_parsing_scope = CONFIG_SCOPE_LOCAL; diff --git a/t/t1300-config.sh b/t/t1300-config.sh index e0dd5d65ce..0754189974 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -2059,6 +2059,37 @@ test_expect_success '--show-scope with --show-origin' ' test_cmp expect output ' +test_expect_success 'GIT_CONFIG_NOGLOBAL' ' + test_when_finished rm -f "$HOME"/.config/git && + cat >"$HOME"/.gitconfig <<-EOF && + [home] + config = true + EOF + mkdir -p "$HOME"/.config/git && + cat >"$HOME"/.config/git/config <<-EOF && + [xdg] + config = true + EOF + cat >.git/config <<-EOF && + [local] + config = true + EOF + + cat >expect <<-EOF && + global xdg.config=true + global home.config=true + local local.config=true + EOF + git config --show-scope --list >output && + test_cmp expect output && + + cat >expect <<-EOF && + local local.config=true + EOF + GIT_CONFIG_NOGLOBAL=true git config --show-scope --list >output && + test_cmp expect output +' + for opt in --local --worktree do test_expect_success "$opt requires a repo" '
While it's already possible to stop git from reading the system config via GIT_CONFIG_NOSYSTEM, doing the same for global config files requires the user to unset both HOME and XDG_CONFIG_HOME. This is an awkward interface and may even pose a problem e.g. when git hooks rely on these variables to be present. Introduce a new GIT_CONFIG_NOGLOBAL envvar, which is the simple equivalent to GIT_CONFIG_NOSYSTEM. If set to true, git will skip reading both `~/.gitconfig` and `$XDG_CONFIG_HOME/git/config`. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- Documentation/git-config.txt | 4 ++++ Documentation/git.txt | 16 ++++++++++++---- config.c | 9 +++++++-- t/t1300-config.sh | 31 +++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 6 deletions(-)