[PATCH] config: use GIT_CONFIG in git config sequence

[PATCH] config: use GIT_CONFIG in git config sequence

[Mateusz Nowotyński]

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(+)

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
-- 
2.24.1

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Junio C Hamano]

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.

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Philip Oakley]

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

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Mateusz Nowotyński]

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

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Mateusz Nowotynski]

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

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Junio C Hamano]

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?

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Matt Rogers]

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.

-- 
Matthew Rogers

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Philip Oakley]

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

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Matt Rogers]

> 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.

-- 
Matthew Rogers

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Philip Oakley]

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...
>
-- 
Philip

[1]
https://lore.kernel.org/git/8850d755-07ce-d8d2-6e5c-88393fce34de@iee.org/

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Junio C Hamano]

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.

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Junio C Hamano]

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.

Re: [PATCH] config: use GIT_CONFIG in git config sequence

[Mateusz Nowotynski]

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.