| Message ID | 20200425235716.1822560-1-maxmati4@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | config: use GIT_CONFIG in git config sequence | expand |
Mateusz Nowotyński <maxmati4@gmail.com> writes: > Currently, there is no way to use config file other then ~/.gitconfig. > This can cause a problem, for example, when running tests of software that > depends on git. In such a case user's gitconfig may contain settings that > are incompatible with tests. While I can remotely imagine how an environment variable that overrides everything might be useful at times, we already use GIT_CONFIG environment for a different purpose, so even if such a feature were desirable, the name is already taken, and you'd want to hunt for another one. Also, I do not think I'll take this patch if the justification were solely the above, as it is a solved problem, together with the use of GIT_CONFIG_NOSYSTEM and GIT_ATTR_NOSYSTEM. Tests of a software that depends on git, and perhaps other things, will be affected in things under the testing user's home directory, and not just ~/.gitconfig file. Providing stable environment to run in to your tests is a useful thing to do, but it is not a viable or a particularly smart strategy for doing so to tweak each and every software that your software may depend on, and your software itself, with a custom change like this patch. You can prepare a pretend-home directory for the use of your tests and point the environment variable $HOME to it while running your tests. See how we do this in our test suite for inspiration---it all happens in t/test-lib.sh, I think. Thanks.
This would need an appropriate addition to show-scope functionality in order to be considered complete too, as I believe the current code would give it a scope of "unknown" which is obviously incorrect since we know the scope.
On 26/04/2020 01:16, Junio C Hamano wrote: > Mateusz Nowotyński <maxmati4@gmail.com> writes: > >> Currently, there is no way to use config file other then ~/.gitconfig. >> This can cause a problem, for example, when running tests of software that >> depends on git. In such a case user's gitconfig may contain settings that >> are incompatible with tests. > While I can remotely imagine how an environment variable that > overrides everything might be useful at times, we already use > GIT_CONFIG environment for a different purpose, so even if such a > feature were desirable, the name is already taken, and you'd want to > hunt for another one. Also, I do not think I'll take this patch if > the justification were solely the above, as it is a solved problem, > together with the use of GIT_CONFIG_NOSYSTEM and GIT_ATTR_NOSYSTEM. Mateusz: is there a discoverability/documentation issue for these extra config settings (a better patch maybe there). I know in the past (in another context) I had failed to appreciate that these had been set so couldn't work out why some problems were occurring for me when testing some ideas. > > Tests of a software that depends on git, and perhaps other things, > will be affected in things under the testing user's home directory, > and not just ~/.gitconfig file. Providing stable environment to run > in to your tests is a useful thing to do, but it is not a viable or > a particularly smart strategy for doing so to tweak each and every > software that your software may depend on, and your software itself, > with a custom change like this patch. > > You can prepare a pretend-home directory for the use of your tests > and point the environment variable $HOME to it while running your > tests. See how we do this in our test suite for inspiration---it > all happens in t/test-lib.sh, I think. > > Thanks. > -- Philip
Hi Matt, On 26/04/2020 02:12, Matt Rogers wrote: > This would need an appropriate addition to show-scope functionality in order > to be considered complete too, as I believe the current code would give it a > scope of "unknown" which is obviously incorrect since we know the scope. > Given the extra config environment settings, could/should the --show-scope (or complementary option) also show/clarify these environment variable settings? -- Philip
> Given the extra config environment settings, could/should the > --show-scope (or complementary option) also show/clarify these > environment variable settings? I'm lukewarm either way on this one, I think it would be pretty trivial to write something that did this, so that only leaves the question of 'should' we do this, which I don't really have any particularly strong feelings about. What would this even ultimately look like? perhaps something like this for a command of `git config --show-scope`: system var=option (currently ignored due to GIT_CONFIG_NOSYSTEM) Which kind of begs the question of how many people set that variable and then forget that they set it? Although I can totally see why it would be nice to have some kind of config flag that's a big "Just show me everything that's going on option" which considering these variables would probably be a little bit more than the current next-best of `git config --list --show-scope --show-origin`. Again though, I'm not exactly sure how useful such an option would be.
On Sat, Apr 25, 2020 at 05:16:56PM -0700, Junio C Hamano wrote: > Mateusz Nowotyński <maxmati4@gmail.com> writes: > > > Currently, there is no way to use config file other then ~/.gitconfig. > > This can cause a problem, for example, when running tests of software that > > depends on git. In such a case user's gitconfig may contain settings that > > are incompatible with tests. > > While I can remotely imagine how an environment variable that > overrides everything might be useful at times, we already use > GIT_CONFIG environment for a different purpose, so even if such a > feature were desirable, the name is already taken, and you'd want to > hunt for another one. Also, I do not think I'll take this patch if > the justification were solely the above, as it is a solved problem, > together with the use of GIT_CONFIG_NOSYSTEM and GIT_ATTR_NOSYSTEM. Agreed that we shouldn't reuse GIT_CONFIG but as you said, before huntig for new name we should decide if we want this at all. As far as I know GIT_CONFIG_NOSYSTEM only allows to ignore system wide config file stored in /etc not one in the user home directory (~/.gitconfig). Is there any way to achive it that I'm not aware of (other then overwriting HOME variable)? Or maybe you would like more to add something like GIT_CONFIG_NOUSER to just ignore this file? > > Tests of a software that depends on git, and perhaps other things, > will be affected in things under the testing user's home directory, > and not just ~/.gitconfig file. Providing stable environment to run > in to your tests is a useful thing to do, but it is not a viable or > a particularly smart strategy for doing so to tweak each and every > software that your software may depend on, and your software itself, > with a custom change like this patch. Problem that we are facing is that if developer has git configured to sign commits using, for example, yubikey he has to touch it on each commit that makes tests unusable. > You can prepare a pretend-home directory for the use of your tests > and point the environment variable $HOME to it while running your > tests. See how we do this in our test suite for inspiration---it > all happens in t/test-lib.sh, I think. > This is what we do currently but the problem with this solution is that it breaks other software that also uses HOME as base path for their data. For example asdf version manager. > Thanks. > Regards, Mateusz
Hi Matt, On 26/04/2020 14:30, Matt Rogers wrote: >> Given the extra config environment settings, could/should the >> --show-scope (or complementary option) also show/clarify these >> environment variable settings? > I'm lukewarm either way on this one, I think it would be pretty trivial > to write something that did this, so that only leaves the question of > 'should' we do this, which I don't really have any particularly strong > feelings about. My thought was that in some cases folks will feel they 'know' where their system and global configs are located, but that 'git config' command isn't showing them those contents. > What would this even ultimately look like? perhaps > something like this for a command of `git config --show-scope`: > > system var=option (currently ignored due to GIT_CONFIG_NOSYSTEM) That's one option. The other, non-programmatic way, it to mention the environment variable in the 'git config' man page, at the --show-scope section, telling folk that if those env variables are set there won't be any scope to show! > > Which kind of begs the question of how many people set that variable > and then forget that they set it? The one that caught me was the test suit[1]. I was unable to replicate results when I set up a test. This can be bad on Windows where some settings need to be auto set (or are commonly set) and maybe result in conflicts. Often what doesn't know what's been done on ones behalf. There's plenty of spare configs to go round ;-) > Although I can totally see why it would > be nice to have some kind of config flag that's a big > "Just show me everything that's going on option" which considering these > variables would probably be a little bit more than the current next-best of > `git config --list --show-scope --show-origin`. Again though, I'm not > exactly sure how useful such an option would be. Mateusz's original problem was with discovery of these env variables, so ended up with an XY-problem of proposing a patch to redirect the ~/.gitconfig file, because relevant the env variables weren't (for him, and previously myself) discoverable. If we go with a 'No one reads the manuals' developer view, then the solution has to be more code... hence my wondering if there was an easy win inside the code, or if it needs to be a subjective update to the man pages (never 'easy'). The man page The GIT_CONFIG_NOSYSTEM only gets its 1 mention as a heading, while GIT_CONFIG has two (+ a heading). Maybe we need to spread the love for NOSYSTEM... >
On 2020-04-26 at 19:32:05, Mateusz Nowotyński wrote: > On Sat, Apr 25, 2020 at 05:16:56PM -0700, Junio C Hamano wrote: > > You can prepare a pretend-home directory for the use of your tests > > and point the environment variable $HOME to it while running your > > tests. See how we do this in our test suite for inspiration---it > > all happens in t/test-lib.sh, I think. > > This is what we do currently but the problem with this solution is that > it breaks other software that also uses HOME as base path for their > data. For example asdf version manager. I know nothing about the asdf version manager, but if you're relying on it for programs, those programs should end up in PATH, and when invoked appropriately in those locations, those programs should just work, regardless of what $HOME is set to. If they don't, that would be a defect in asdf, since the Unix expectation is that programs in $PATH should generally function without regard to the setting of $HOME. From my cursory poking around at the repo, it looks like it should do this just fine. So you can set $HOME to a temporary directory and still use asdf as long as your don't reset $PATH. Or, if you want to specifically load asdf programs first, you could do something like this: #!/bin/sh . "$HOME/asdf/asdf.sh" export HOME=$(mktemp -d) # Run tests here. Regardless of your tooling, you definitely want to reset $HOME in almost every nontrivial shell testsuite, since many users have configuration files or data storage that you wouldn't want to use. For example, if you generate a new GnuPG key on every run, the user won't appreciate it if you import it as one of their private keys.
Mateusz Nowotyński <maxmati4@gmail.com> writes: [jc: I do not know why your reply had two blank lines for every line, but I reformatted them to make them repliable] > This is what we do currently but the problem with this solution is that > it breaks other software that also uses HOME as base path for their > data. For example asdf version manager. That sounds like another reason why you would want to redirect HOME to a stable and known directory constructed specifically for the use of your tests. Just like you do not want the behaviour of the tested program getting affected by random settings the users, who are running your tests, might have in their $HOME/.gitconfig, random settings for "asdf version manager" (whatever it is) the users happen to have in their $HOME directory would also have undesirable side effects to your tests, no?
Philip Oakley <philipoakley@iee.email> writes:
> Mateusz's original problem was with discovery of these env variables,...
I somehow doubt it.
Certainly, defeating /etc/gitconfig should be a part of the solution
to the "I want a stable environment to run tests reproducibly,
without getting affected by random settings the testing user may get
affected" problem. It is not enough to defeat $HOME/.gitconfig (and
its xdg variant).
But I didn't get the feeling that Mateusz was even aware of the need
to defeat the system wide configuration; Mateusz only mentioned and
cared about $HOME/.gitconfig (and its xdg variant).
And the thing is that we do have support for an environment variable
to defeat the system-wide configuration (GIT_CONFIG_NOSYSTEM), but
discovering it would not have been sufficient to solve the "stable
environment for reproducible tests" problem, as we do not have an
environment variable to defeat the global (i.e. per user) one.
So, I do not think the problem was with discovery at all. The
problem was two levels deep, (1) not realizing the need to defeat
the system wide settings (for which the environment may have been a
good solution once the need to solve that problem got recognised),
and (2) not realizing the need to redirect $HOME for programs other
than git, hence omitting $HOME/.gitconfig from the sequence is not
a sufficient solution.
Having said all that, I addition of GIT_CONFIG_NOGLOBAL (or
GIT_CONFIG_NOUSER) may not be such a bad thing. We probably will
add GIT_CONFIG_NOLOCAL (or GIT_CONFIG_NOREPOSITORY) to complement
these two if that happens. I dunno.
Junio C Hamano <gitster@pobox.com> writes: > Philip Oakley <philipoakley@iee.email> writes: > >> Mateusz's original problem was with discovery of these env variables,... > > I somehow doubt it. > > Certainly, defeating /etc/gitconfig should be a part of the solution > to the "I want a stable environment to run tests reproducibly, > without getting affected by random settings the testing user may get > affected" problem. It is not enough to defeat $HOME/.gitconfig (and > its xdg variant). > > But I didn't get the feeling that Mateusz was even aware of the need > ... After re-reading the patch that started this thread, I suspect the reason Mateusz did not mention GIT_CONFIG_NOSYSTEM could be because he was aware of the need to defeat /etc/gitconfig, and was already using it. There was no discovery issue---to somebody who would propose the patch under discussion to solve "I want a stable environment for testing", /etc/gitconfig was a solved problem. And unfortunately we do not have GIT_CONFIG_NO_GLOBAL; so there is nothing to discover there, either X-<. If we were to add such an environment, we need to make sure that it is discoverable ;-) I still think setting up a directory that can be used as a stable $HOME replacement and pointing at it during the test, while declining the system-wide one with GIT_CONFIG_NOSYSTEM, is the right approach for "stable test environment" problem.
On Sun, Apr 26, 2020 at 03:32:51PM -0700, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > Philip Oakley <philipoakley@iee.email> writes: > > > >> Mateusz's original problem was with discovery of these env variables,... > > > > I somehow doubt it. > > > > Certainly, defeating /etc/gitconfig should be a part of the solution > > to the "I want a stable environment to run tests reproducibly, > > without getting affected by random settings the testing user may get > > affected" problem. It is not enough to defeat $HOME/.gitconfig (and > > its xdg variant). > > > > But I didn't get the feeling that Mateusz was even aware of the need > > ... > > After re-reading the patch that started this thread, I suspect the > reason Mateusz did not mention GIT_CONFIG_NOSYSTEM could be because > he was aware of the need to defeat /etc/gitconfig, and was already > using it. There was no discovery issue---to somebody who would > propose the patch under discussion to solve "I want a stable > environment for testing", /etc/gitconfig was a solved problem. Yes, this is exactly the case, I'm fully aware of GIT_CONFIG_NOSYSTEM so there is no problem with system wide config file. Anyway if you want to improve its discoverability I can suggest moving description from git-config to git since it's influences all git commands not only git config. > And unfortunately we do not have GIT_CONFIG_NO_GLOBAL; so there is > nothing to discover there, either X-<. If we were to add such an > environment, we need to make sure that it is discoverable ;-) I think it will fully solve my problem so if you are willing to accept it I can do the implementation, it shouldn't be complex anyway. Regarding the discoverability I think the documentation should be put together with GIT_CONFIG_NOSYSTEM since they are very close to each other. > I still think setting up a directory that can be used as a stable > $HOME replacement and pointing at it during the test, while > declining the system-wide one with GIT_CONFIG_NOSYSTEM, is the right > approach for "stable test environment" problem. We have pretty "stable test environment" that is running in CI and it's working really well and stable. Problem starts with developers machines. We would like pose as low requirements on them as possible, ideally it would be just given range of go, ruby and git version that we use. I know that it will be ideal to just overwrite $HOME with some temp directory but reality is even if it's not fully UNIX compliant there are tools that will rely on user $HOME to provide those requirements (go, ruby and git). I think that in such case it's better to add this small feature to git then go and fix every tool in the world. Currently we found issues only with go cache and mentioned earlier asdf version, but I expect more of them as people come and go.
On Sun, Apr 26, 2020 at 08:08:45PM +0000, brian m. carlson wrote: > On 2020-04-26 at 19:32:05, Mateusz Nowotyński wrote: > > On Sat, Apr 25, 2020 at 05:16:56PM -0700, Junio C Hamano wrote: > > > You can prepare a pretend-home directory for the use of your tests > > > and point the environment variable $HOME to it while running your > > > tests. See how we do this in our test suite for inspiration---it > > > all happens in t/test-lib.sh, I think. > > > > This is what we do currently but the problem with this solution is that > > it breaks other software that also uses HOME as base path for their > > data. For example asdf version manager. > > I know nothing about the asdf version manager, but if you're relying on > it for programs, those programs should end up in PATH, and when invoked > appropriately in those locations, those programs should just work, > regardless of what $HOME is set to. If they don't, that would be a > defect in asdf, since the Unix expectation is that programs in $PATH > should generally function without regard to the setting of $HOME. From > my cursory poking around at the repo, it looks like it should do this > just fine. > > So you can set $HOME to a temporary directory and still use asdf as long > as your don't reset $PATH. Or, if you want to specifically load asdf > programs first, you could do something like this: > > #!/bin/sh > > . "$HOME/asdf/asdf.sh" > export HOME=$(mktemp -d) > # Run tests here. > > Regardless of your tooling, you definitely want to reset $HOME in almost > every nontrivial shell testsuite, since many users have configuration > files or data storage that you wouldn't want to use. For example, if > you generate a new GnuPG key on every run, the user won't appreciate it > if you import it as one of their private keys. > -- > brian m. carlson: Houston, Texas, US > OpenPGP: https://keybase.io/bk2204 To be honest I also don't know how it exactly works and I personally don't use it. I think it adds to the PATH just thin wrapper not binary itself. I guess it's done that way because it looks for .tool-versions in current working directory and then fallback to $HOME/.tool-versions. Regarding reseting HOME we are aware that we cannot do actions that have sideeffects outside test directory so we just won't generate/import GnuPG keys. -- Regards, Mateusz
diff --git a/Documentation/git.txt b/Documentation/git.txt index 9d6769e95a..c92411b8d9 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -882,6 +882,10 @@ standard output. adequate and support for it is likely to be removed in the foreseeable future (along with the variable). +GIT_CONFIG:: + Take the configuration from the given file instead of using typical + resolve mechanism. + Discussion[[Discussion]] ------------------------ diff --git a/config.c b/config.c index d17d2bd9dc..811b4e2186 100644 --- a/config.c +++ b/config.c @@ -1754,6 +1754,7 @@ int config_with_options(config_fn_t fn, void *data, const struct config_options *opts) { struct config_include_data inc = CONFIG_INCLUDE_INIT; + const char* env_config = getenv(CONFIG_ENVIRONMENT); if (opts->respect_includes) { inc.fn = fn; @@ -1776,6 +1777,8 @@ int config_with_options(config_fn_t fn, void *data, return git_config_from_file(fn, config_source->file, data); else if (config_source && config_source->blob) return git_config_from_blob_ref(fn, config_source->blob, data); + else if (env_config) + return git_config_from_file(fn, env_config, data); return do_git_config_sequence(opts, fn, data); } diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh index 1f600e2cae..74ed80f5ca 100755 --- a/t/t0007-git-var.sh +++ b/t/t0007-git-var.sh @@ -46,4 +46,24 @@ test_expect_success 'listing and asking for variables are exclusive' ' test_must_fail git var -l GIT_COMMITTER_IDENT ' + +cat > config_file << EOF +[ein] + bahn = strasse +EOF + +test_expect_success 'respect GIT_CONFIG' ' + GIT_CONFIG=$(pwd)/config_file git var -l >actual && + echo strasse >expect && + sed -n s/ein\\.bahn=//p <actual >actual.bahn && + test_cmp expect actual.bahn +' + +test_expect_success 'GIT_CONFIG leads to not existsing file' ' + test_when_finished "sane_unset GIT_CONFIG" && + GIT_CONFIG=$(pwd)/not_existsing_config_file && + export GIT_CONFIG && + test_must_fail git var -l +' + test_done
Currently, there is no way to use config file other then ~/.gitconfig. This can cause a problem, for example, when running tests of software that depends on git. In such a case user's gitconfig may contain settings that are incompatible with tests. This commit expands existing GIT_CONFIG environment variable, that is currently used only by git config, that it is used also in other commands. When it is set it's the only config file that's being used. Signed-off-by: Mateusz Nowotyński <maxmati4@gmail.com> --- Documentation/git.txt | 4 ++++ config.c | 3 +++ t/t0007-git-var.sh | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+)