* Re: [PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig
2012-09-27 14:46 ` [PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig Ramkumar Ramachandra
@ 2012-09-27 16:57 ` Matthieu Moy
2012-09-27 17:50 ` Junio C Hamano
2012-09-27 17:27 ` Junio C Hamano
2012-09-27 17:35 ` Jeff King
2 siblings, 1 reply; 14+ messages in thread
From: Matthieu Moy @ 2012-09-27 16:57 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, David Aguilar, Anurag Priyam, Junio C Hamano
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index eaea079..c8db03f 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -205,6 +205,9 @@ $GIT_DIR/config::
> User-specific configuration file. Also called "global"
> configuration file.
>
> +$GIT_GLOBAL_CONFIG::
> + Overrides the path of the global configuration file.
> +
I'm not particularly in favor of introducing another environment
variable, but if you are to introduce it, why just override the
configuration file, and not $HOME completely (e.g. to override
$HOME/.git-credentials too).
There was a patch proposing that here ($GIT_HOME to override $HOME):
http://thread.gmane.org/gmane.comp.version-control.git/135447/focus=135494
I don't remember exactly what happened to the patch, I can't find an
explicit reason to reject it in the thread, but it seems it didn't make
its way to git.git.
> index cbbdf7d..9b09cee 100644
> --- a/path.c
> +++ b/path.c
> @@ -131,10 +131,15 @@ char *git_path(const char *fmt, ...)
>
> void home_config_paths(char **global, char **xdg, char *file)
> {
> + char *global_config = getenv("GIT_GLOBAL_CONFIG");
> char *xdg_home = getenv("XDG_CONFIG_HOME");
> char *home = getenv("HOME");
> char *to_free = NULL;
>
> + if (global_config) {
> + *global = mkpathdup("%s", global_config);
> + return;
> + }
If you return here, haven't you completely broken the XDG stuff, since
*xdg is set a few lines below in the function?
Also, I guess home_config_paths(..., "ignore") will return the path to
the configuration file instead of the ignore file?
> --- a/t/t1306-xdg-files.sh
> +++ b/t/t1306-xdg-files.sh
> @@ -28,6 +28,14 @@ test_expect_success 'read config: xdg file exists and ~/.gitconfig exists' '
> test_cmp expected actual
> '
>
> +test_expect_success 'read config: $GIT_GLOBAL_CONFIG is set and ~/.gitconfig exists' '
> + >.gitconfig &&
> + echo "[alias]" >.gittestconfig &&
> + echo " myalias = !echo in_gitconfig" >>.gittestconfig &&
> + echo in_gitconfig >expected &&
> + GIT_GLOBAL_CONFIG=~/.gittestconfig git myalias >actual &&
> + test_cmp expected actual
> +'
You should check that "git config --set" works too, as the codepath for
writing to configuration is relatively different from the one to read.
For example, I *think* that "git config --global" will write to
$GIT_GLOBAL_CONFIG and "git config" without --global will ignore it, but
a test would be welcome.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig
2012-09-27 16:57 ` Matthieu Moy
@ 2012-09-27 17:50 ` Junio C Hamano
0 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2012-09-27 17:50 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Ramkumar Ramachandra, Git List, David Aguilar, Anurag Priyam
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
>> index eaea079..c8db03f 100644
>> --- a/Documentation/git-config.txt
>> +++ b/Documentation/git-config.txt
>> @@ -205,6 +205,9 @@ $GIT_DIR/config::
>> User-specific configuration file. Also called "global"
>> configuration file.
>>
>> +$GIT_GLOBAL_CONFIG::
>> + Overrides the path of the global configuration file.
>> +
>
> I'm not particularly in favor of introducing another environment
> variable, but if you are to introduce it, why just override the
> configuration file, and not $HOME completely (e.g. to override
> $HOME/.git-credentials too).
>
> There was a patch proposing that here ($GIT_HOME to override $HOME):
I think both of these are at the entrance of slippery slope to
insanity I'd rather not to venture into.
If somebody hates ~/.dotmanyfiles so much, why not do HOME=~/dots/,
instead of having to set GIT_HOME to move ~/.gitconfig, FROTZ_HOME
to move ~/.frotzconfig, MAIL_HOME to move ~/.mailrc, etc.?
I wouldn't be surprised if some _other_ things break with your HOME
pointing at a directory inside your home directory, but then it may
be better to fix that other thing instead.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig
2012-09-27 14:46 ` [PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig Ramkumar Ramachandra
2012-09-27 16:57 ` Matthieu Moy
@ 2012-09-27 17:27 ` Junio C Hamano
2012-09-27 17:35 ` Jeff King
2 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2012-09-27 17:27 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, David Aguilar, Anurag Priyam
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> Documentation/git-config.txt | 3 +++
> path.c | 5 +++++
> t/t1306-xdg-files.sh | 8 ++++++++
> 3 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index eaea079..c8db03f 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -205,6 +205,9 @@ $GIT_DIR/config::
> User-specific configuration file. Also called "global"
> configuration file.
>
> +$GIT_GLOBAL_CONFIG::
> + Overrides the path of the global configuration file.
> +
Why is this even a good idea?
- Does it make sense not to read $HOME/.gitconfig or XDG stuff
when this is set? Why skip them? If it makes sense to skip
them, why doesn't it also skip $GIT_DIR/config and/or
/etc/gitconfig?
- Why is it not "the third user-specific configuration file" instead?
- Why is it not "a list of paths to read configurations from"?
- Where does it end?
Not overly impressed, I'd have to say.
> diff --git a/path.c b/path.c
> index cbbdf7d..9b09cee 100644
> --- a/path.c
> +++ b/path.c
> @@ -131,10 +131,15 @@ char *git_path(const char *fmt, ...)
>
> void home_config_paths(char **global, char **xdg, char *file)
> {
> + char *global_config = getenv("GIT_GLOBAL_CONFIG");
> char *xdg_home = getenv("XDG_CONFIG_HOME");
> char *home = getenv("HOME");
> char *to_free = NULL;
>
> + if (global_config) {
> + *global = mkpathdup("%s", global_config);
> + return;
> + }
> if (!home) {
> if (global)
> *global = NULL;
> diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
> index 8b14ab1..5b0e08e 100755
> --- a/t/t1306-xdg-files.sh
> +++ b/t/t1306-xdg-files.sh
> @@ -28,6 +28,14 @@ test_expect_success 'read config: xdg file exists and ~/.gitconfig exists' '
> test_cmp expected actual
> '
>
> +test_expect_success 'read config: $GIT_GLOBAL_CONFIG is set and ~/.gitconfig exists' '
> + >.gitconfig &&
> + echo "[alias]" >.gittestconfig &&
> + echo " myalias = !echo in_gitconfig" >>.gittestconfig &&
> + echo in_gitconfig >expected &&
> + GIT_GLOBAL_CONFIG=~/.gittestconfig git myalias >actual &&
How is this tilde expanded and by whom?
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig
2012-09-27 14:46 ` [PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig Ramkumar Ramachandra
2012-09-27 16:57 ` Matthieu Moy
2012-09-27 17:27 ` Junio C Hamano
@ 2012-09-27 17:35 ` Jeff King
2012-09-27 19:57 ` Junio C Hamano
` (2 more replies)
2 siblings, 3 replies; 14+ messages in thread
From: Jeff King @ 2012-09-27 17:35 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, David Aguilar, Anurag Priyam, Junio C Hamano
On Thu, Sep 27, 2012 at 08:16:11PM +0530, Ramkumar Ramachandra wrote:
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index eaea079..c8db03f 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -205,6 +205,9 @@ $GIT_DIR/config::
> User-specific configuration file. Also called "global"
> configuration file.
>
> +$GIT_GLOBAL_CONFIG::
> + Overrides the path of the global configuration file.
> +
Like the other reviews, I am not overly enthused. If we are going to add
a new variable, I think $GIT_HOME makes a lot more sense. But it really
sounds like using $XDG_CONFIG_HOME would be even simpler.
Also, have you considered using a config include? Like:
$ echo '[include]path = ~/my-dotfiles/gitconfig' >~/.gitconfig
It's a one-time setup, and then you get updates inside my-dotfiles
forever. The one-time setup is annoying, but you have to bootstrap
somehow (e.g., you're going to have to copy a .profile or similar to get
the GIT_GLOBAL_CONFIG variable set).
-Peff
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig
2012-09-27 17:35 ` Jeff King
@ 2012-09-27 19:57 ` Junio C Hamano
2012-09-27 20:51 ` Matthieu Moy
2012-09-28 7:05 ` Ramkumar Ramachandra
2 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2012-09-27 19:57 UTC (permalink / raw)
To: Jeff King; +Cc: Ramkumar Ramachandra, Git List, David Aguilar, Anurag Priyam
Jeff King <peff@peff.net> writes:
> Also, have you considered using a config include? Like:
>
> $ echo '[include]path = ~/my-dotfiles/gitconfig' >~/.gitconfig
Very good suggestion.
> ... you have to bootstrap
> somehow (e.g., you're going to have to copy a .profile or similar to get
> the GIT_GLOBAL_CONFIG variable set).
Exactly my thought ;-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig
2012-09-27 17:35 ` Jeff King
2012-09-27 19:57 ` Junio C Hamano
@ 2012-09-27 20:51 ` Matthieu Moy
2012-09-28 7:05 ` Ramkumar Ramachandra
2 siblings, 0 replies; 14+ messages in thread
From: Matthieu Moy @ 2012-09-27 20:51 UTC (permalink / raw)
To: Jeff King
Cc: Ramkumar Ramachandra, Git List, David Aguilar, Anurag Priyam,
Junio C Hamano
Jeff King <peff@peff.net> writes:
> Also, have you considered using a config include? Like:
>
> $ echo '[include]path = ~/my-dotfiles/gitconfig' >~/.gitconfig
>
> It's a one-time setup, and then you get updates inside my-dotfiles
> forever. The one-time setup is annoying, but you have to bootstrap
> somehow (e.g., you're going to have to copy a .profile or similar to get
> the GIT_GLOBAL_CONFIG variable set).
I also strongly prefer symlinks or include over environment variables.
Relying on environment variables makes the setup a bit fragile, partly
because I often mess things up with my shell (e.g. I once had different
shell and environment variables in ~/.xsession and in actual shells,
hence different configuration depending on whether I launch stuff from
my window-manager or from a shell).
I could understand using environment variables for one-shot
configuration change (e.g. HOME=/tmp/ git whatever), but then $HOME is
sufficient in general.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig
2012-09-27 17:35 ` Jeff King
2012-09-27 19:57 ` Junio C Hamano
2012-09-27 20:51 ` Matthieu Moy
@ 2012-09-28 7:05 ` Ramkumar Ramachandra
2 siblings, 0 replies; 14+ messages in thread
From: Ramkumar Ramachandra @ 2012-09-28 7:05 UTC (permalink / raw)
To: Jeff King; +Cc: Git List, David Aguilar, Anurag Priyam, Junio C Hamano
Jeff King wrote:
> Like the other reviews, I am not overly enthused. If we are going to add
> a new variable, I think $GIT_HOME makes a lot more sense. But it really
> sounds like using $XDG_CONFIG_HOME would be even simpler.
>
> Also, have you considered using a config include? Like:
>
> $ echo '[include]path = ~/my-dotfiles/gitconfig' >~/.gitconfig
Sounds like this is the way to go. Or I could pick up Junio's
suggestion to use a Makefile. I'll discontinue working on the patch.
Thanks.
Ram
^ permalink raw reply [flat|nested] 14+ messages in thread