[PATCHv2] Possibility to read both from ~/.gitconfig and from $XDG_CONFIG_HOME/git/config

[PATCHv2] Possibility to read both from ~/.gitconfig and from $XDG_CONFIG_HOME/git/config

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

Git will read both in $XDG_CONFIG_HOME/git/config and in ~/.gitconfig in this order:
.git/config > ~/.gitconfig > $XDG_CONFIG_HOME/git/config > /etc/gitconfig
If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/config will be used.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Lucien KONG <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Franck JONAS <Franck.Jonas@ensimag.imag.fr>
---
 Documentation/git-config.txt    |    9 +++-
 builtin/config.c                |   48 +++++++++++++++++----
 config.c                        |   21 ++++++++--
 t/t1306-read-xdg-config-file.sh |   87 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 149 insertions(+), 16 deletions(-)
 create mode 100755 t/t1306-read-xdg-config-file.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index d9463cb..30418fa 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -99,8 +99,8 @@ OPTIONS
 	For writing options: write to global ~/.gitconfig file rather than
 	the repository .git/config.
 +
-For reading options: read only from global ~/.gitconfig rather than
-from all available files.
+For reading options: read only from global ~/.gitconfig then from
+$XDG_CONFIG_HOME/git/config rather than from all available files.
 +
 See also <<FILES>>.
 
@@ -194,7 +194,7 @@ See also <<FILES>>.
 FILES
 -----
 
-If not set explicitly with '--file', there are three files where
+If not set explicitly with '--file', there are four files where
 'git config' will search for configuration options:
 
 $GIT_DIR/config::
@@ -204,6 +204,9 @@ $GIT_DIR/config::
 	User-specific configuration file. Also called "global"
 	configuration file.
 
+$XDG_CONFIG_HOME/git/config::
+	Second user-specific configuration file. ~/.gitconfig has priority.
+
 $(prefix)/etc/gitconfig::
 	System-wide configuration file.
 
diff --git a/builtin/config.c b/builtin/config.c
index 33c8820..38dba4f 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -161,7 +161,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = -1;
-	char *global = NULL, *repo_config = NULL;
+	char *gitconfig_global = NULL, *xdg_global = NULL, *repo_config = NULL;
 	const char *system_wide = NULL, *local;
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 	config_fn_t fn;
@@ -171,8 +171,15 @@ static int get_value(const char *key_, const char *regex_)
 	if (!local) {
 		const char *home = getenv("HOME");
 		local = repo_config = git_pathdup("config");
-		if (home)
-			global = xstrdup(mkpath("%s/.gitconfig", home));
+		if (home) {
+			const char *xdg_config_home = getenv("XDG_CONFIG_HOME");
+			if (xdg_config_home)
+				xdg_global = xstrdup(mkpath("%s/git/config", xdg_config_home));
+			else
+				xdg_global = xstrdup(mkpath("%s/.config/git/config", home));
+
+			gitconfig_global = xstrdup(mkpath("%s/.gitconfig", home));
+		}
 		if (git_config_system())
 			system_wide = git_etc_gitconfig();
 	}
@@ -229,15 +236,19 @@ static int get_value(const char *key_, const char *regex_)
 
 	if (do_all && system_wide)
 		git_config_from_file(fn, system_wide, data);
-	if (do_all && global)
-		git_config_from_file(fn, global, data);
+	if (do_all && xdg_global)
+		git_config_from_file(fn, xdg_global, data);
+	if (do_all && gitconfig_global)
+		git_config_from_file(fn, gitconfig_global, data);
 	if (do_all)
 		git_config_from_file(fn, local, data);
 	git_config_from_parameters(fn, data);
 	if (!do_all && !seen)
 		git_config_from_file(fn, local, data);
-	if (!do_all && !seen && global)
-		git_config_from_file(fn, global, data);
+	if (!do_all && !seen && gitconfig_global)
+		git_config_from_file(fn, gitconfig_global, data);
+	if (!do_all && !seen && xdg_global)
+		git_config_from_file(fn, xdg_global, data);
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
@@ -254,7 +265,8 @@ static int get_value(const char *key_, const char *regex_)
 
 free_strings:
 	free(repo_config);
-	free(global);
+	free(gitconfig_global);
+	free(xdg_global);
 	return ret;
 }
 
@@ -381,7 +393,25 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	if (use_global_config) {
 		char *home = getenv("HOME");
 		if (home) {
-			char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
+			char *user_config;
+			const char *gitconfig_path = mkpath("%s/.gitconfig", home);
+			const char *xdg_config_path = NULL;
+			const char *xdg_config_home = NULL;
+
+			xdg_config_home = getenv("XDG_CONFIG_HOME");
+			if (xdg_config_home)
+				xdg_config_path = mkpath("%s/git/config", xdg_config_home);
+			else
+				xdg_config_path = mkpath("%s/.config/git/config", home);
+
+			if (access(gitconfig_path, R_OK) && !access(xdg_config_path, R_OK) &&
+			    (actions == ACTION_LIST ||
+			     actions == ACTION_GET_COLOR ||
+			     actions == ACTION_GET_COLORBOOL))
+				user_config = xstrdup(xdg_config_path);
+			else
+				user_config = xstrdup(gitconfig_path);
+
 			given_config_file = user_config;
 		} else {
 			die("$HOME not set");
diff --git a/config.c b/config.c
index 71ef171..53557dc 100644
--- a/config.c
+++ b/config.c
@@ -939,10 +939,23 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 
 	home = getenv("HOME");
 	if (home) {
-		char buf[PATH_MAX];
-		char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
-		if (!access(user_config, R_OK)) {
-			ret += git_config_from_file(fn, user_config, data);
+		const char *gitconfig_path = xstrdup(mkpath("%s/.gitconfig", home));
+		const char *xdg_config_path = NULL;
+		const char *xdg_config_home = NULL;
+
+		xdg_config_home = getenv("XDG_CONFIG_HOME");
+		if (xdg_config_home)
+			xdg_config_path = xstrdup(mkpath("%s/git/config", xdg_config_home));
+		else
+			xdg_config_path = xstrdup(mkpath("%s/.config/git/config", home));
+
+		if (!access(xdg_config_path, R_OK)) {
+			ret += git_config_from_file(fn, xdg_config_path, data);
+			found += 1;
+		}
+
+		if (!access(gitconfig_path, R_OK)) {
+			ret += git_config_from_file(fn, gitconfig_path, data);
 			found += 1;
 		}
 	}
diff --git a/t/t1306-read-xdg-config-file.sh b/t/t1306-read-xdg-config-file.sh
new file mode 100755
index 0000000..4cab38b
--- /dev/null
+++ b/t/t1306-read-xdg-config-file.sh
@@ -0,0 +1,87 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Valentin Duperray, Lucien Kong, Franck Jonas,
+#		     Thomas Nguy, Khoi Nguyen
+#		     Grenoble INP Ensimag
+#
+
+test_description='possibility to read $XDG_CONFIG_HOME/git/config file'
+
+. ./test-lib.sh
+
+test_expect_success 'read automatically: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p .config/git &&
+	echo "[advice]" >.config/git/config &&
+	echo "	statushints = false" >>.config/git/config &&
+	cat >expect <<-\EOF &&
+	# On branch master
+	#
+	# Initial commit
+	#
+	# Untracked files:
+	#	.config/
+	#	expect
+	#	output
+	nothing added to commit but untracked files present
+	EOF
+	git status >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'read automatically: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[advice]" >.gitconfig &&
+	echo "	statushints = true" >>.gitconfig &&
+	cat >expect <<-\EOF &&
+	# On branch master
+	#
+	# Initial commit
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	.config/
+	#	.gitconfig
+	#	expect
+	#	output
+	nothing added to commit but untracked files present (use "git add" to track)
+	EOF
+	git status >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo "[user]" >.config/git/config &&
+	echo "	name = read_config" >>.config/git/config &&
+	echo read_config >expect &&
+	git config --get user.name >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo read_gitconfig >expect &&
+	git config --get user.name >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo user.name=read_config >expect &&
+	git config --global --list >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo user.name=read_gitconfig >expect &&
+	git config --global --list >output &&
+	test_cmp expect output
+'
+
+test_done
-- 
1.7.8

Re: [PATCHv2] Possibility to read both from ~/.gitconfig and from $XDG_CONFIG_HOME/git/config

[Junio C Hamano]

Huynh Khoi Nguyen NGUYEN  <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
writes:

> From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>
>
> Git will read both in $XDG_CONFIG_HOME/git/config and in ~/.gitconfig in this order:
> .git/config > ~/.gitconfig > $XDG_CONFIG_HOME/git/config > /etc/gitconfig
> If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/config will be used.

Is it just me who finds the above three lines extremely unreadable?

Also can you give this patch a bit more sensible title?
"Possibility to" does not tell us much---anything is possible if you
change code after all.

I see the patch does not touch the writing codepath, which is
probably a good thing, but the log message should explicitly state
that.

> @@ -194,7 +194,7 @@ See also <<FILES>>.
>  FILES
>  -----
>  
> -If not set explicitly with '--file', there are three files where
> +If not set explicitly with '--file', there are four files where
>  'git config' will search for configuration options:
>  
>  $GIT_DIR/config::
> @@ -204,6 +204,9 @@ $GIT_DIR/config::
>  	User-specific configuration file. Also called "global"
>  	configuration file.
>  
> +$XDG_CONFIG_HOME/git/config::
> +	Second user-specific configuration file. ~/.gitconfig has priority.
> +

I am not sure in what way $HOME/.gitconfig has "priority".

Your proposed log message says that You read from $HOME/.gitconfig
and then from $XDG_CONFIG_HOME/git/config, which means that any
single-valued variable set in $HOME/.gitconfig will be overwritten
by whatever is in $XDG_CONFIG_HOME/git/config, no?  That sounds like
you are giving priority to the latter to me.

And for multi-valued variables, settings from both files are read,
so there isn't much inherent priority between the two, except for
variables for which the definition order matters, of course.

If you read only from $HOME/.gitconfig if exists, and read from
$XDG_CONFIG_HOME/git/config only when $HOME/.gitconfig does not,
then you are giving $HOME/.gitconfig a priority, but that is not
what the patch is doing as far as I can tell.

> diff --git a/builtin/config.c b/builtin/config.c
> index 33c8820..38dba4f 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -161,7 +161,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
>  static int get_value(const char *key_, const char *regex_)
>  {
>  	int ret = -1;
> -	char *global = NULL, *repo_config = NULL;
> +	char *gitconfig_global = NULL, *xdg_global = NULL, *repo_config = NULL;
>  	const char *system_wide = NULL, *local;
>  	struct config_include_data inc = CONFIG_INCLUDE_INIT;
>  	config_fn_t fn;
> @@ -171,8 +171,15 @@ static int get_value(const char *key_, const char *regex_)
>  	if (!local) {
>  		const char *home = getenv("HOME");
>  		local = repo_config = git_pathdup("config");
> -		if (home)
> -			global = xstrdup(mkpath("%s/.gitconfig", home));
> +		if (home) {
> +			const char *xdg_config_home = getenv("XDG_CONFIG_HOME");
> +			if (xdg_config_home)

This is logically wrong; even when you fail to read $HOME, you may
be able to read $XDG_CONFIG_HOME, no?  It shouldn't be nested inside
"if (home)" at all, methinks.

It would be more like

	global = xdg_global = NULL;
        if (HOME exists?)
        	global = $HOME/.gitconfig
	if (XDG_CONFIG_HOME exists?)
        	xdg_global = $XDG_CONFIG_HOME/git/config
	else if (HOME exists?)
        	xdg_global = $HOME/.config/git/config

no?

> @@ -381,7 +393,25 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>  	if (use_global_config) {
>  		char *home = getenv("HOME");
>  		if (home) {
> -			char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
> +			char *user_config;
> +			const char *gitconfig_path = mkpath("%s/.gitconfig", home);
> +			const char *xdg_config_path = NULL;
> +			const char *xdg_config_home = NULL;
> +
> +			xdg_config_home = getenv("XDG_CONFIG_HOME");
> +			if (xdg_config_home)
> +				xdg_config_path = mkpath("%s/git/config", xdg_config_home);
> +			else
> +				xdg_config_path = mkpath("%s/.config/git/config", home);
> +
> +			if (access(gitconfig_path, R_OK) && !access(xdg_config_path, R_OK) &&
> +			    (actions == ACTION_LIST ||
> +			     actions == ACTION_GET_COLOR ||
> +			     actions == ACTION_GET_COLORBOOL))
> +				user_config = xstrdup(xdg_config_path);
> +			else
> +				user_config = xstrdup(gitconfig_path);
> +
>  			given_config_file = user_config;
>  		} else {
>  			die("$HOME not set");

Exactly the same comment applies here.

You seem to always write to $HOME/.gitconfig, so missing $HOME may
be an error if the action is to store, but if you are reading and if
$XDG_CONFIG_HOME is set, you do not have to have $HOME set, no?
Even when there is $HOME, if there is no $HOME/.gitconfig file, you
wouldn't want to give an error, so missing $HOME environment should
be treated pretty much the same way as missing $HOME/.gitconfig file
for the purpose of reading, no?

> diff --git a/config.c b/config.c
> index 71ef171..53557dc 100644
> --- a/config.c
> +++ b/config.c
> @@ -939,10 +939,23 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  
>  	home = getenv("HOME");
>  	if (home) {
> -		char buf[PATH_MAX];
> -		char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
> -		if (!access(user_config, R_OK)) {
> -			ret += git_config_from_file(fn, user_config, data);
> +		const char *gitconfig_path = xstrdup(mkpath("%s/.gitconfig", home));
> +		const char *xdg_config_path = NULL;
> +		const char *xdg_config_home = NULL;
> +
> +		xdg_config_home = getenv("XDG_CONFIG_HOME");
> +		if (xdg_config_home)
> +			xdg_config_path = xstrdup(mkpath("%s/git/config", xdg_config_home));
> +		else
> +			xdg_config_path = xstrdup(mkpath("%s/.config/git/config", home));

Exactly the same comment applies here, too.

The original that read from $HOME/.gitconfig was simple enough so
having three copies of getenv("HOME") was perfectly fine, but as you
are introduce this much complexity to to decide which two files to
read from, the code added this patch needs to be refactored and
three copies of the same logic need to be consolidated, I would have
to say.

Re: [PATCHv2] Possibility to read both from ~/.gitconfig and from $XDG_CONFIG_HOME/git/config

[Ramsay Jones]

Junio C Hamano wrote:
> Huynh Khoi Nguyen NGUYEN  <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
[...]

>> diff --git a/config.c b/config.c
>> index 71ef171..53557dc 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -939,10 +939,23 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>>  
>>  	home = getenv("HOME");
>>  	if (home) {
>> -		char buf[PATH_MAX];
>> -		char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
>> -		if (!access(user_config, R_OK)) {
>> -			ret += git_config_from_file(fn, user_config, data);
>> +		const char *gitconfig_path = xstrdup(mkpath("%s/.gitconfig", home));
>> +		const char *xdg_config_path = NULL;
>> +		const char *xdg_config_home = NULL;
>> +
>> +		xdg_config_home = getenv("XDG_CONFIG_HOME");
>> +		if (xdg_config_home)
>> +			xdg_config_path = xstrdup(mkpath("%s/git/config", xdg_config_home));
>> +		else
>> +			xdg_config_path = xstrdup(mkpath("%s/.config/git/config", home));
> 
> Exactly the same comment applies here, too.

I have not tried this patch (or the v3 version, which I haven't read yet), but
is it likely that this has re-introduced the bug addressed by commit 05bab3ea
("config.c: Fix a static buffer overwrite bug by avoiding mkpath()", 19-11-2011)?.

I don't know the answer, but I suspect that it may have done just that. (indeed, it
may well have made the bug more likely to appear).

> 
> The original that read from $HOME/.gitconfig was simple enough so
> having three copies of getenv("HOME") was perfectly fine, but as you
> are introduce this much complexity to to decide which two files to
> read from, the code added this patch needs to be refactored and
> three copies of the same logic need to be consolidated, I would have
> to say.

I agree. Also, using mksnpath() in the refactored code (rather than
mkpath()) would be a good idea. :-P

ATB,
Ramsay Jones

[PATCHv3] Read from XDG configuration file, not write

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

Git will be able to read in $XDG_CONFIG_HOME/git/config, a new
configuration file following XDG specification. In the order of
reading, this file is between global configuration file and system
wide configuration file. Git will not be able to write in this new
configuration file. If $XDG_CONFIG_HOME is either not set or empty,
$HOME/.config/git/config will be used as new configuration file.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Lucien KONG <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Franck JONAS <Franck.Jonas@ensimag.imag.fr>
---
 Documentation/git-config.txt    |   12 ++++-
 builtin/config.c                |   37 +++++++++++-----
 cache.h                         |    1 +
 config.c                        |    5 ++
 path.c                          |   12 +++++
 t/t1306-read-xdg-config-file.sh |   87 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 139 insertions(+), 15 deletions(-)
 create mode 100755 t/t1306-read-xdg-config-file.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index d9463cb..004015a 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -99,8 +99,8 @@ OPTIONS
 	For writing options: write to global ~/.gitconfig file rather than
 	the repository .git/config.
 +
-For reading options: read only from global ~/.gitconfig rather than
-from all available files.
+For reading options: read only from global ~/.gitconfig then from
+$XDG_CONFIG_HOME/git/config rather than from all available files.
 +
 See also <<FILES>>.
 
@@ -194,7 +194,7 @@ See also <<FILES>>.
 FILES
 -----
 
-If not set explicitly with '--file', there are three files where
+If not set explicitly with '--file', there are four files where
 'git config' will search for configuration options:
 
 $GIT_DIR/config::
@@ -204,6 +204,12 @@ $GIT_DIR/config::
 	User-specific configuration file. Also called "global"
 	configuration file.
 
+$XDG_CONFIG_HOME/git/config::
+	Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
+	or empty, $HOME/.config/git/config will be used. Any single-valued
+	variable set in this file will be overwritten by whatever is in
+	~/.gitconfig.
+
 $(prefix)/etc/gitconfig::
 	System-wide configuration file.
 
diff --git a/builtin/config.c b/builtin/config.c
index 33c8820..3fb23a1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -161,7 +161,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = -1;
-	char *global = NULL, *repo_config = NULL;
+	char *gitconfig_global = NULL, *xdg_global = NULL, *repo_config = NULL;
 	const char *system_wide = NULL, *local;
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 	config_fn_t fn;
@@ -172,7 +172,9 @@ static int get_value(const char *key_, const char *regex_)
 		const char *home = getenv("HOME");
 		local = repo_config = git_pathdup("config");
 		if (home)
-			global = xstrdup(mkpath("%s/.gitconfig", home));
+			gitconfig_global = xstrdup(mkpath("%s/.gitconfig", home));
+		if (xdg_git_path("config"))
+			xdg_global = xdg_git_path("config");
 		if (git_config_system())
 			system_wide = git_etc_gitconfig();
 	}
@@ -229,15 +231,19 @@ static int get_value(const char *key_, const char *regex_)
 
 	if (do_all && system_wide)
 		git_config_from_file(fn, system_wide, data);
-	if (do_all && global)
-		git_config_from_file(fn, global, data);
+	if (do_all && xdg_global)
+		git_config_from_file(fn, xdg_global, data);
+	if (do_all && gitconfig_global)
+		git_config_from_file(fn, gitconfig_global, data);
 	if (do_all)
 		git_config_from_file(fn, local, data);
 	git_config_from_parameters(fn, data);
 	if (!do_all && !seen)
 		git_config_from_file(fn, local, data);
-	if (!do_all && !seen && global)
-		git_config_from_file(fn, global, data);
+	if (!do_all && !seen && gitconfig_global)
+		git_config_from_file(fn, gitconfig_global, data);
+	if (!do_all && !seen && xdg_global)
+		git_config_from_file(fn, xdg_global, data);
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
@@ -254,7 +260,8 @@ static int get_value(const char *key_, const char *regex_)
 
 free_strings:
 	free(repo_config);
-	free(global);
+	free(gitconfig_global);
+	free(xdg_global);
 	return ret;
 }
 
@@ -380,12 +387,18 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 	if (use_global_config) {
 		char *home = getenv("HOME");
-		if (home) {
-			char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
-			given_config_file = user_config;
-		} else {
+		const char *gitconfig_path = mkpath("%s/.gitconfig", home);
+		const char *xdg_config_path = xdg_git_path("config");
+
+		if (access(gitconfig_path, R_OK) && !access(xdg_config_path, R_OK) &&
+		    (actions == ACTION_LIST ||
+		     actions == ACTION_GET_COLOR ||
+		     actions == ACTION_GET_COLORBOOL))
+			given_config_file = xdg_config_path;
+		else if (home)
+			given_config_file = xstrdup(gitconfig_path);
+		else
 			die("$HOME not set");
-		}
 	}
 	else if (use_system_config)
 		given_config_file = git_etc_gitconfig();
diff --git a/cache.h b/cache.h
index 06413e1..b86d083 100644
--- a/cache.h
+++ b/cache.h
@@ -708,6 +708,7 @@ int set_shared_perm(const char *path, int mode);
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
+extern char *xdg_git_path(char *file);
 extern char *expand_user_path(const char *path);
 const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
diff --git a/config.c b/config.c
index 71ef171..e08632f 100644
--- a/config.c
+++ b/config.c
@@ -937,6 +937,11 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		found += 1;
 	}
 
+	if (!access(xdg_git_path("config"), R_OK)) {
+		ret += git_config_from_file(fn, xdg_git_path("config"), data);
+		found += 1;
+	}
+
 	home = getenv("HOME");
 	if (home) {
 		char buf[PATH_MAX];
diff --git a/path.c b/path.c
index 6f2aa69..7dbedb2 100644
--- a/path.c
+++ b/path.c
@@ -122,6 +122,18 @@ char *git_path(const char *fmt, ...)
 	return cleanup_path(pathname);
 }
 
+char *xdg_git_path(char *file)
+{
+	const char *home = getenv("HOME");
+	const char *xdg_config_home = getenv("XDG_CONFIG_HOME");
+
+	if (xdg_config_home)
+		return xstrdup(mkpath("%s/git/%s", xdg_config_home, file));
+	if (home)
+		return xstrdup(mkpath("%s/.config/git/%s", home, file));
+	return NULL;
+}
+
 char *git_path_submodule(const char *path, const char *fmt, ...)
 {
 	char *pathname = get_pathname();
diff --git a/t/t1306-read-xdg-config-file.sh b/t/t1306-read-xdg-config-file.sh
new file mode 100755
index 0000000..4cab38b
--- /dev/null
+++ b/t/t1306-read-xdg-config-file.sh
@@ -0,0 +1,87 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Valentin Duperray, Lucien Kong, Franck Jonas,
+#		     Thomas Nguy, Khoi Nguyen
+#		     Grenoble INP Ensimag
+#
+
+test_description='possibility to read $XDG_CONFIG_HOME/git/config file'
+
+. ./test-lib.sh
+
+test_expect_success 'read automatically: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p .config/git &&
+	echo "[advice]" >.config/git/config &&
+	echo "	statushints = false" >>.config/git/config &&
+	cat >expect <<-\EOF &&
+	# On branch master
+	#
+	# Initial commit
+	#
+	# Untracked files:
+	#	.config/
+	#	expect
+	#	output
+	nothing added to commit but untracked files present
+	EOF
+	git status >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'read automatically: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[advice]" >.gitconfig &&
+	echo "	statushints = true" >>.gitconfig &&
+	cat >expect <<-\EOF &&
+	# On branch master
+	#
+	# Initial commit
+	#
+	# Untracked files:
+	#   (use "git add <file>..." to include in what will be committed)
+	#
+	#	.config/
+	#	.gitconfig
+	#	expect
+	#	output
+	nothing added to commit but untracked files present (use "git add" to track)
+	EOF
+	git status >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo "[user]" >.config/git/config &&
+	echo "	name = read_config" >>.config/git/config &&
+	echo read_config >expect &&
+	git config --get user.name >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo read_gitconfig >expect &&
+	git config --get user.name >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo user.name=read_config >expect &&
+	git config --global --list >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo user.name=read_gitconfig >expect &&
+	git config --global --list >output &&
+	test_cmp expect output
+'
+
+test_done
-- 
1.7.8

Re: [PATCHv3] Read from XDG configuration file, not write

[Junio C Hamano]

Huynh Khoi Nguyen NGUYEN  <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
writes:

> From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>
>
> Git will be able to read in $XDG_CONFIG_HOME/git/config, a new

Getting nicer.

> @@ -172,7 +172,9 @@ static int get_value(const char *key_, const char *regex_)
>  		const char *home = getenv("HOME");
>  		local = repo_config = git_pathdup("config");
>  		if (home)
> -			global = xstrdup(mkpath("%s/.gitconfig", home));
> +			gitconfig_global = xstrdup(mkpath("%s/.gitconfig", home));
> +		if (xdg_git_path("config"))
> +			xdg_global = xdg_git_path("config");
>  		if (git_config_system())
>  			system_wide = git_etc_gitconfig();
>  	}

Hrm, xdg_git_path() returns allocated memory, and each call site
leaks its return value, no?

I didn't mean a micro-helper function like xdg_git_path() when I
suggested refactoring.  I meant a helper that figures out all the
necessary bits in one go.  For example, can't the above call site
look more like this?

        static int get_value(const char *key_, const char *regex_)
        {
                int ret = -1;
                char *global = NULL, *xdg = NULL, *repo_config = NULL;
                const char *system_wide = NULL, *local;
                struct config_include_data inc = CONFIG_INCLUDE_INIT;
                config_fn_t fn;
                void *data;

                local = given_config_file;
                if (!local) {
                        local = repo_config = git_pathdup("config");
                        if (git_config_system())
                                system_wide = git_etc_gitconfig();
			home_config_paths(&global, &xdg);
                }
		...

And then the config.c::home_config_paths() may look like:

	void home_config_paths(char **global, char **xdg)
	{
		char *xdg_home = getenv("XDG_CONFIG_HOME");
                char *home = getenv("HOME");
                char *to_free = NULL;
		
                if (!home) {
			*global = NULL;
		} else {
			if (!xdg_home) {
	                        to_free = strdup(mkpath("%s/.config", home));
				xdg_home = to_free;
			}
			*global = xstrdup(mkpath("%s/.gitconfig", home));
		}
                
                if (!xdg_home)
                	*xdg = NULL;
		else
			*xdg = xstrdup(mkpath("%s/git/config", xdg_home));
		free(to_free);
	}


> diff --git a/t/t1306-read-xdg-config-file.sh b/t/t1306-read-xdg-config-file.sh
> new file mode 100755
> index 0000000..4cab38b
> --- /dev/null
> +++ b/t/t1306-read-xdg-config-file.sh
> @@ -0,0 +1,87 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2012 Valentin Duperray, Lucien Kong, Franck Jonas,
> +#		     Thomas Nguy, Khoi Nguyen
> +#		     Grenoble INP Ensimag
> +#
> +
> +test_description='possibility to read $XDG_CONFIG_HOME/git/config file'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'read automatically: xdg file exists and ~/.gitconfig doesn'\''t' '
> +	mkdir -p .config/git &&
> +	echo "[advice]" >.config/git/config &&
> +	echo "	statushints = false" >>.config/git/config &&
> +	cat >expect <<-\EOF &&
> +	# On branch master
> +	#
> +	# Initial commit

Use of "status" output as test vector is not such a good idea for
multiple reasons.

 - You are aware that the "status hints" feature is currently in
   flux (somebody nearby is working on a patch);

 - Even without another topic that currently works on it, "git
   status" output is for human consumption and subject to
   improvement and also l10n.

 - "status" output lists tracked and untracked paths, which makes it
   harder to later introduce new file to the test directory that are
   needed for later tests, or rename ones that were used in earlier
   versions of the test, without having to update all of these test
   vectors.

> +	#
> +	# Untracked files:
> +	#	.config/
> +	#	expect
> +	#	output

This is a good example that shows why using "status" output as a
test vector is not a good idea.

Notice you listed "expect" and "output" here.  As my next suggestion
is to rename "output" to "actual" (we typically "test_cmp expect actual"),
use of "git status" output as a test vector would mean you would
have to update them whenever such a change is needed.

> +	nothing added to commit but untracked files present
> +	EOF
> +	git status >output &&
> +	test_cmp expect output
> +'

[PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

Git will be able to read in $XDG_CONFIG_HOME/git/config, a new
configuration file following XDG specification. In the order of
reading, this file is between global configuration file and system
wide configuration file. Git will not be able to write in this new
configuration file. If core.excludesfile is not define, Git will read
the global exclude files in $XDG_CONFIG_HOME/git/ignore. Same goes for
core.attributesfile in $XDG_CONFIG_HOME/git/attributes. If
$XDG_CONFIG_HOME is either not set or empty, $HOME/.config will be
used.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Lucien KONG <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Franck JONAS <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/git-config.txt    |   12 +++-
 attr.c                          |   10 +++
 builtin/config.c                |   28 ++++++---
 cache.h                         |    1 +
 config.c                        |   21 ++++---
 dir.c                           |    4 +
 path.c                          |   26 ++++++++
 t/t1306-read-xdg-config-file.sh |  133 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 214 insertions(+), 21 deletions(-)
 create mode 100755 t/t1306-read-xdg-config-file.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index d9463cb..7e344a2 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -99,8 +99,8 @@ OPTIONS
 	For writing options: write to global ~/.gitconfig file rather than
 	the repository .git/config.
 +
-For reading options: read only from global ~/.gitconfig rather than
-from all available files.
+For reading options: read only from global ~/.gitconfig and from
+$XDG_CONFIG_HOME/git/config rather than from all available files.
 +
 See also <<FILES>>.
 
@@ -194,7 +194,7 @@ See also <<FILES>>.
 FILES
 -----
 
-If not set explicitly with '--file', there are three files where
+If not set explicitly with '--file', there are four files where
 'git config' will search for configuration options:
 
 $GIT_DIR/config::
@@ -204,6 +204,12 @@ $GIT_DIR/config::
 	User-specific configuration file. Also called "global"
 	configuration file.
 
+$XDG_CONFIG_HOME/git/config::
+	Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
+	or empty, $HOME/.config/git/config will be used. Any single-valued
+	variable set in this file will be overwritten by whatever is in
+	~/.gitconfig.
+
 $(prefix)/etc/gitconfig::
 	System-wide configuration file.
 
diff --git a/attr.c b/attr.c
index 303751f..441387f 100644
--- a/attr.c
+++ b/attr.c
@@ -497,6 +497,9 @@ static int git_attr_system(void)
 static void bootstrap_attr_stack(void)
 {
 	struct attr_stack *elem;
+	char *xdg_attributes_file;
+
+	home_config_paths(NULL, &xdg_attributes_file, "attributes");
 
 	if (attr_stack)
 		return;
@@ -522,6 +525,13 @@ static void bootstrap_attr_stack(void)
 			elem->prev = attr_stack;
 			attr_stack = elem;
 		}
+	} else if (!access(xdg_attributes_file, R_OK)) {
+		elem = read_attr_from_file(xdg_attributes_file, 1);
+		if (elem) {
+			elem->origin = NULL;
+			elem->prev = attr_stack;
+			attr_stack = elem;
+		}
 	}
 
 	if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
diff --git a/builtin/config.c b/builtin/config.c
index 33c8820..da54fd1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -161,7 +161,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = -1;
-	char *global = NULL, *repo_config = NULL;
+	char *global = NULL, *xdg = NULL, *repo_config = NULL;
 	const char *system_wide = NULL, *local;
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 	config_fn_t fn;
@@ -169,12 +169,10 @@ static int get_value(const char *key_, const char *regex_)
 
 	local = given_config_file;
 	if (!local) {
-		const char *home = getenv("HOME");
 		local = repo_config = git_pathdup("config");
-		if (home)
-			global = xstrdup(mkpath("%s/.gitconfig", home));
 		if (git_config_system())
 			system_wide = git_etc_gitconfig();
+		home_config_paths(&global, &xdg, "config");
 	}
 
 	if (use_key_regexp) {
@@ -229,6 +227,8 @@ static int get_value(const char *key_, const char *regex_)
 
 	if (do_all && system_wide)
 		git_config_from_file(fn, system_wide, data);
+	if (do_all && xdg)
+		git_config_from_file(fn, xdg, data);
 	if (do_all && global)
 		git_config_from_file(fn, global, data);
 	if (do_all)
@@ -238,6 +238,8 @@ static int get_value(const char *key_, const char *regex_)
 		git_config_from_file(fn, local, data);
 	if (!do_all && !seen && global)
 		git_config_from_file(fn, global, data);
+	if (!do_all && !seen && xdg)
+		git_config_from_file(fn, xdg, data);
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
@@ -255,6 +257,7 @@ static int get_value(const char *key_, const char *regex_)
 free_strings:
 	free(repo_config);
 	free(global);
+	free(xdg);
 	return ret;
 }
 
@@ -379,13 +382,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 
 	if (use_global_config) {
-		char *home = getenv("HOME");
-		if (home) {
-			char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
+		char *user_config = NULL;
+		char *xdg_config = NULL;
+
+		home_config_paths(&user_config, &xdg_config, "config");
+
+		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&
+		    (actions == ACTION_LIST ||
+		     actions == ACTION_GET_COLOR ||
+		     actions == ACTION_GET_COLORBOOL))
+			given_config_file = xdg_config;
+		else if (user_config)
 			given_config_file = user_config;
-		} else {
+		else
 			die("$HOME not set");
-		}
 	}
 	else if (use_system_config)
 		given_config_file = git_etc_gitconfig();
diff --git a/cache.h b/cache.h
index 06413e1..0632503 100644
--- a/cache.h
+++ b/cache.h
@@ -708,6 +708,7 @@ int set_shared_perm(const char *path, int mode);
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
+extern void home_config_paths(char **global, char **xdg, char *file);
 extern char *expand_user_path(const char *path);
 const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
diff --git a/config.c b/config.c
index 71ef171..d1393b8 100644
--- a/config.c
+++ b/config.c
@@ -929,7 +929,10 @@ int git_config_system(void)
 int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 {
 	int ret = 0, found = 0;
-	const char *home = NULL;
+	char *xdg_config = NULL;
+	char *user_config = NULL;
+
+	home_config_paths(&user_config, &xdg_config, "config");
 
 	if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {
 		ret += git_config_from_file(fn, git_etc_gitconfig(),
@@ -937,14 +940,14 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		found += 1;
 	}
 
-	home = getenv("HOME");
-	if (home) {
-		char buf[PATH_MAX];
-		char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
-		if (!access(user_config, R_OK)) {
-			ret += git_config_from_file(fn, user_config, data);
-			found += 1;
-		}
+	if (!access(xdg_config, R_OK)) {
+		ret += git_config_from_file(fn, xdg_config, data);
+		found += 1;
+	}
+
+	if (!access(user_config, R_OK)) {
+		ret += git_config_from_file(fn, user_config, data);
+		found += 1;
 	}
 
 	if (repo_config && !access(repo_config, R_OK)) {
diff --git a/dir.c b/dir.c
index ed1510f..e0c3589 100644
--- a/dir.c
+++ b/dir.c
@@ -1234,13 +1234,17 @@ int remove_dir_recursively(struct strbuf *path, int flag)
 void setup_standard_excludes(struct dir_struct *dir)
 {
 	const char *path;
+	char *xdg_path;
 
 	dir->exclude_per_dir = ".gitignore";
 	path = git_path("info/exclude");
+	home_config_paths(NULL, &xdg_path, "ignore");
 	if (!access(path, R_OK))
 		add_excludes_from_file(dir, path);
 	if (excludes_file && !access(excludes_file, R_OK))
 		add_excludes_from_file(dir, excludes_file);
+	else if (!access(xdg_path, R_OK))
+		add_excludes_from_file(dir, xdg_path);
 }
 
 int remove_path(const char *name)
diff --git a/path.c b/path.c
index 6f2aa69..53f3f53 100644
--- a/path.c
+++ b/path.c
@@ -122,6 +122,32 @@ char *git_path(const char *fmt, ...)
 	return cleanup_path(pathname);
 }
 
+void home_config_paths(char **global, char **xdg, char *file)
+{
+	char *xdg_home = getenv("XDG_CONFIG_HOME");
+	char *home = getenv("HOME");
+	char *to_free = NULL;
+
+	if (!home) {
+		if (global)
+			*global = NULL;
+	} else {
+		if (!xdg_home) {
+			to_free = strdup(mkpath("%s/.config", home));
+			xdg_home = to_free;
+		}
+		if (global)
+			*global = xstrdup(mkpath("%s/.gitconfig", home));
+	}
+
+	if (!xdg_home)
+		*xdg = NULL;
+	else
+		*xdg = xstrdup(mkpath("%s/git/%s", xdg_home, file));
+
+	free(to_free);
+}
+
 char *git_path_submodule(const char *path, const char *fmt, ...)
 {
 	char *pathname = get_pathname();
diff --git a/t/t1306-read-xdg-config-file.sh b/t/t1306-read-xdg-config-file.sh
new file mode 100755
index 0000000..a8775c4
--- /dev/null
+++ b/t/t1306-read-xdg-config-file.sh
@@ -0,0 +1,133 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Valentin Duperray, Lucien Kong, Franck Jonas,
+#		     Thomas Nguy, Khoi Nguyen
+#		     Grenoble INP Ensimag
+#
+
+test_description='Compatibility with $XDG_CONFIG_HOME/git/config /ignore and /attributes files'
+
+. ./test-lib.sh
+
+test_expect_success 'read config: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p .config/git &&
+	echo "[alias]" >.config/git/config &&
+	echo "	myalias = !echo in_config" >>.config/git/config &&
+	echo in_config >expected &&
+	git myalias >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read config: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[alias]" >.gitconfig &&
+	echo "	myalias = !echo in_gitconfig" >>.gitconfig &&
+	echo in_gitconfig >expected &&
+	git myalias >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo "[user]" >.config/git/config &&
+	echo "	name = read_config" >>.config/git/config &&
+	echo read_config >expected &&
+	git config --get user.name >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo read_gitconfig >expected &&
+	git config --get user.name >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo user.name=read_config >expected &&
+	git config --global --list >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo user.name=read_gitconfig >expected &&
+	git config --global --list >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'Exclusion of a file in the XDG ignore file' '
+	git init git &&
+	cd git &&
+	echo foo >to_be_excluded &&
+	git add to_be_excluded &&
+	git rm --cached to_be_excluded &&
+	mkdir -p ../.config/git/ &&
+	echo to_be_excluded >../.config/git/ignore &&
+	test_must_fail git add to_be_excluded
+'
+
+
+test_expect_success 'Exclusion in both XDG and local ignore files' '
+	echo to_be_excluded >.gitignore &&
+	test_must_fail git add to_be_excluded
+'
+
+
+test_expect_success 'Exclusion in a non-XDG global ignore file' '
+	echo >../.config/git/ignore &&
+	rm .gitignore &&
+	echo to_be_excluded >../my_gitignore &&
+	git config core.excludesfile ../my_gitignore &&
+	test_when_finished "cd .. && rm -rf git && rm -r .config" &&
+	test_must_fail git add to_be_excluded
+'
+
+
+test_expect_success 'Checking attributes in the XDG attributes file' '
+	git init git &&
+	cd git &&
+	echo foo >f &&
+	git check-attr -a f >actual &&
+	test_line_count -eq 0 actual &&
+	cd .. &&
+	mkdir -p .config/git/ &&
+	echo "f attr_f" >.config/git/attributes &&
+	cd git &&
+	echo "f: attr_f: set" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'Checking attributes in both XDG and local ignore files' '
+	echo  "f -attr_f" >.gitattributes &&
+	echo "f: attr_f: unset" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'Checking attributes in a non-XDG global attributes file' '
+	rm .gitattributes &&
+	echo "f attr_f=test" >../my_gitattributes &&
+	git config core.attributesfile ../my_gitattributes &&
+	test_when_finished "cd .. && rm -rf git && rm -r .config" &&
+	echo "f: attr_f: test" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+
+test_done
-- 
1.7.8

Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files

[Matthieu Moy]

Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
writes:

> Git will not be able to write in this new configuration file.

The use of the future is ambiguous, and since another patch will allow
writing, I'd rather rephrase it as e.g.

Git currently does not write to this new configuration file.

> If core.excludesfile is not define, Git will read the global exclude

s/define/defined/

> files in $XDG_CONFIG_HOME/git/ignore. Same goes for
> core.attributesfile in $XDG_CONFIG_HOME/git/attributes. If
> $XDG_CONFIG_HOME is either not set or empty, $HOME/.config will be
> used.

I'd prefer having two separate patches for the config file and for the
two others. If ignore and attributes are simple enough, they may go to
the same patch, but ideally, there would be two separate patches again.

> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt

No doc for core.excludesfile and core.attributesfile change :-(.

> --- a/attr.c
> +++ b/attr.c
> @@ -497,6 +497,9 @@ static int git_attr_system(void)
>  static void bootstrap_attr_stack(void)
>  {
>  	struct attr_stack *elem;
> +	char *xdg_attributes_file;
> +
> +	home_config_paths(NULL, &xdg_attributes_file, "attributes");
>  
>  	if (attr_stack)
>  		return;
> @@ -522,6 +525,13 @@ static void bootstrap_attr_stack(void)
>  			elem->prev = attr_stack;
>  			attr_stack = elem;
>  		}
> +	} else if (!access(xdg_attributes_file, R_OK)) {
> +		elem = read_attr_from_file(xdg_attributes_file, 1);
> +		if (elem) {
> +			elem->origin = NULL;
> +			elem->prev = attr_stack;
> +			attr_stack = elem;
> +		}
>  	}
>  
>  	if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {

The logic seems overly complex, and you duplicate the if() uselessly.

Why not just set the variable git_attributes_file before entering the
if? Something like this:

diff --git a/attr.c b/attr.c
index 303751f..71dc472 100644
--- a/attr.c
+++ b/attr.c
@@ -515,6 +515,9 @@ static void bootstrap_attr_stack(void)
 		}
 	}
 
+	if (!git_attributes_file)
+		git_attributes_file = "foo";
+
 	if (git_attributes_file) {
 		elem = read_attr_from_file(git_attributes_file, 1);
 		if (elem) {

(obviously replacing "foo" by the actual code involving
home_config_paths(..., "attributes")).

Doing so, you may even get rid of the "if (git_attributes_file)" on the
next line.

Then, the logic is really "core.excludesfile defaults to the XDG file",
not "if such variable is set then do that else do something else".

> --- a/dir.c
> +++ b/dir.c
> @@ -1234,13 +1234,17 @@ int remove_dir_recursively(struct strbuf *path, int flag)
>  void setup_standard_excludes(struct dir_struct *dir)
>  {
>  	const char *path;
> +	char *xdg_path;
>  
>  	dir->exclude_per_dir = ".gitignore";
>  	path = git_path("info/exclude");
> +	home_config_paths(NULL, &xdg_path, "ignore");
>  	if (!access(path, R_OK))
>  		add_excludes_from_file(dir, path);
>  	if (excludes_file && !access(excludes_file, R_OK))
>  		add_excludes_from_file(dir, excludes_file);
> +	else if (!access(xdg_path, R_OK))
> +		add_excludes_from_file(dir, xdg_path);
>  }

Same remark here. Look at the patch I sent earlier to give a default
value:

http://thread.gmane.org/gmane.comp.version-control.git/133343/focus=133415

For example, you version reads from XDG file if core.excludesfile is
set, but the file it points to doesn't exist. I don't think this is
expected.

> +	echo foo >to_be_excluded &&
> +	git add to_be_excluded &&
> +	git rm --cached to_be_excluded &&

Err, why add and remove it? You just need to create it, right?

> +	cd .. &&
> +	mkdir -p .config/git/ &&

I don't like these relative references to $HOME. If you mean $HOME, why
not say

mkdir -p $HOME/.config/git/
echo "f attr_f" >$HOME/.config/git/

?

(you don't even need to "cd")

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files

[nguyenhu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=";"; format=flowed	DelSp=Yes, Size: 4453 bytes --]



Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :

> I'd prefer having two separate patches for the config file and for the
> two others. If ignore and attributes are simple enough, they may go to
> the same patch, but ideally, there would be two separate patches again.

We will separate this patch in three different patches.

> No doc for core.excludesfile and core.attributesfile change :-(.

It will be done for the next patch ;)

>> --- a/attr.c
>> +++ b/attr.c
>> @@ -497,6 +497,9 @@ static int git_attr_system(void)
>>  static void bootstrap_attr_stack(void)
>>  {
>>  	struct attr_stack *elem;
>> +	char *xdg_attributes_file;
>> +
>> +	home_config_paths(NULL, &xdg_attributes_file, "attributes");
>>
>>  	if (attr_stack)
>>  		return;
>> @@ -522,6 +525,13 @@ static void bootstrap_attr_stack(void)
>>  			elem->prev = attr_stack;
>>  			attr_stack = elem;
>>  		}
>> +	} else if (!access(xdg_attributes_file, R_OK)) {
>> +		elem = read_attr_from_file(xdg_attributes_file, 1);
>> +		if (elem) {
>> +			elem->origin = NULL;
>> +			elem->prev = attr_stack;
>> +			attr_stack = elem;
>> +		}
>>  	}
>>
>>  	if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
>
> The logic seems overly complex, and you duplicate the if() uselessly.
>
> Why not just set the variable git_attributes_file before entering the
> if? Something like this:
>
> diff --git a/attr.c b/attr.c
> index 303751f..71dc472 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -515,6 +515,9 @@ static void bootstrap_attr_stack(void)
>  		}
>  	}
>
> +	if (!git_attributes_file)
> +		git_attributes_file = "foo";
> +
>  	if (git_attributes_file) {
>  		elem = read_attr_from_file(git_attributes_file, 1);
>  		if (elem) {
>
> (obviously replacing "foo" by the actual code involving
> home_config_paths(..., "attributes")).
>
> Doing so, you may even get rid of the "if (git_attributes_file)" on the
> next line.

We first thought to use an "else if" in order not to pointlessly check the
existence of the xdg_attributes_file (or double checking git_attributes_file)
if git_attributes_file exists.
BTW after checking the code more closely, we do not need to verify the  
existence of the xdg_attributes_file so it is indeed more clean to use  
your version, Thank.

>> --- a/dir.c
>> +++ b/dir.c
>> @@ -1234,13 +1234,17 @@ int remove_dir_recursively(struct strbuf  
>> *path, int flag)
>>  void setup_standard_excludes(struct dir_struct *dir)
>>  {
>>  	const char *path;
>> +	char *xdg_path;
>>
>>  	dir->exclude_per_dir = ".gitignore";
>>  	path = git_path("info/exclude");
>> +	home_config_paths(NULL, &xdg_path, "ignore");
>>  	if (!access(path, R_OK))
>>  		add_excludes_from_file(dir, path);
>>  	if (excludes_file && !access(excludes_file, R_OK))
>>  		add_excludes_from_file(dir, excludes_file);
>> +	else if (!access(xdg_path, R_OK))
>> +		add_excludes_from_file(dir, xdg_path);
>>  }
> Same remark here. Look at the patch I sent earlier to give a default
> value:
>
> http://thread.gmane.org/gmane.comp.version-control.git/133343/focus=133415
>
> For example, you version reads from XDG file if core.excludesfile is
> set, but the file it points to doesn't exist. I don't think this is
> expected.

Actually, it's the opposite. Our version only read from XDG file if  
core.excludesfile is not set.
After checking your patch, it may be more logical to inialize the default
value of excludes_file to the xdg_path as done for the attributes file.

>> +	echo foo >to_be_excluded &&
>> +	git add to_be_excluded &&
>> +	git rm --cached to_be_excluded &&
>
> Err, why add and remove it? You just need to create it, right?

It was to check if to_be_excluded is indeed not ignored at the beginning
of the test before ignoring it but that's seem a bit over-testing, I'll
remove it.

>> +	cd .. &&
>> +	mkdir -p .config/git/ &&
>
> I don't like these relative references to $HOME. If you mean $HOME, why
> not say
>
> mkdir -p $HOME/.config/git/
> echo "f attr_f" >$HOME/.config/git/
>
> ?

It will be fixed but BTW where the tests are executed,
$HOME has a weird behaviour:

echo $HOME and echo "$HOME"
      both returns /.../t/trash directory.t1306-read-xdg-config-file
but   echo foo >$HOME   writes in ../t/trash
while echo foo >"$HOME" writes in t/trash directory.t1306-read-xdg-config-file
so "$HOME" is needed for the tests to work.

Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files

[Matthieu Moy]

nguyenhu@minatec.inpg.fr writes:

>>> --- a/dir.c
>>> +++ b/dir.c
>>> @@ -1234,13 +1234,17 @@ int remove_dir_recursively(struct strbuf  
>>> *path, int flag)
>>>  void setup_standard_excludes(struct dir_struct *dir)
>>>  {
>>>  	const char *path;
>>> +	char *xdg_path;
>>>
>>>  	dir->exclude_per_dir = ".gitignore";
>>>  	path = git_path("info/exclude");
>>> +	home_config_paths(NULL, &xdg_path, "ignore");
>>>  	if (!access(path, R_OK))
>>>  		add_excludes_from_file(dir, path);
>>>  	if (excludes_file && !access(excludes_file, R_OK))
>>>  		add_excludes_from_file(dir, excludes_file);
>>> +	else if (!access(xdg_path, R_OK))
>>> +		add_excludes_from_file(dir, xdg_path);
>>>  }
>> Same remark here. Look at the patch I sent earlier to give a default
>> value:
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/133343/focus=133415
>>
>> For example, you version reads from XDG file if core.excludesfile is
>> set, but the file it points to doesn't exist. I don't think this is
>> expected.
>
> Actually, it's the opposite. Our version only read from XDG file if  
> core.excludesfile is not set.

It's what you want to do, but not what I read from the code. Your "else
if" above is reachable if "excludes_file && !access(excludes_file, R_OK)"
is false, which includes the case when excludes_file is set but does not
exist.

Anyway, this just shows that the logic is too complex, we shouldn't need
this discussion with simple enough code.

> echo $HOME and echo "$HOME"
>       both returns /.../t/trash directory.t1306-read-xdg-config-file
> but   echo foo >$HOME   writes in ../t/trash
> while echo foo >"$HOME" writes in t/trash directory.t1306-read-xdg-config-file
> so "$HOME" is needed for the tests to work.

In general, $HOME means "evaluate $HOME and do the whitespace splitting
after", and "$HOME" means "evaluate $HOME and don't do whitespace
splitting". Since "trash directory" contains a space, you need the
quoting.

There are places where quoting is not needed, though, and I think
redirect is part of them. But I wouldn't be surprised if the different
levels of quoting when ran in test-lib broke that.

My advice: always quote variables, unless you have a very good reason
not to do so.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

[PATCHv5 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

Git will be able to read from $XDG_CONFIG_HOME/git/config, a new
configuration file following XDG specification. In the order of
reading, this file is between global configuration file and system
wide configuration file. Git currently does not write to this new
configuration file. If $XDG_CONFIG_HOME is either not set or empty,
$HOME/.config will be used.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <Valentin.DUPERRAY@ensimag.imag.fr>
Signed-off-by: Franck JONAS <Franck.JONAS@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <Thomas.NGUY@ensimag.imag.fr>
Signed-off-by: Lucien KONG <Lucien.KONG@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/git-config.txt |   12 ++++++--
 builtin/config.c             |   28 ++++++++++++------
 cache.h                      |    1 +
 config.c                     |   21 ++++++++------
 path.c                       |   26 +++++++++++++++++
 t/t1306-xdg-files.sh         |   64 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 131 insertions(+), 21 deletions(-)
 create mode 100755 t/t1306-xdg-files.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index d9463cb..7e344a2 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -99,8 +99,8 @@ OPTIONS
 	For writing options: write to global ~/.gitconfig file rather than
 	the repository .git/config.
 +
-For reading options: read only from global ~/.gitconfig rather than
-from all available files.
+For reading options: read only from global ~/.gitconfig and from
+$XDG_CONFIG_HOME/git/config rather than from all available files.
 +
 See also <<FILES>>.
 
@@ -194,7 +194,7 @@ See also <<FILES>>.
 FILES
 -----
 
-If not set explicitly with '--file', there are three files where
+If not set explicitly with '--file', there are four files where
 'git config' will search for configuration options:
 
 $GIT_DIR/config::
@@ -204,6 +204,12 @@ $GIT_DIR/config::
 	User-specific configuration file. Also called "global"
 	configuration file.
 
+$XDG_CONFIG_HOME/git/config::
+	Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
+	or empty, $HOME/.config/git/config will be used. Any single-valued
+	variable set in this file will be overwritten by whatever is in
+	~/.gitconfig.
+
 $(prefix)/etc/gitconfig::
 	System-wide configuration file.
 
diff --git a/builtin/config.c b/builtin/config.c
index 33c8820..da54fd1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -161,7 +161,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = -1;
-	char *global = NULL, *repo_config = NULL;
+	char *global = NULL, *xdg = NULL, *repo_config = NULL;
 	const char *system_wide = NULL, *local;
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 	config_fn_t fn;
@@ -169,12 +169,10 @@ static int get_value(const char *key_, const char *regex_)
 
 	local = given_config_file;
 	if (!local) {
-		const char *home = getenv("HOME");
 		local = repo_config = git_pathdup("config");
-		if (home)
-			global = xstrdup(mkpath("%s/.gitconfig", home));
 		if (git_config_system())
 			system_wide = git_etc_gitconfig();
+		home_config_paths(&global, &xdg, "config");
 	}
 
 	if (use_key_regexp) {
@@ -229,6 +227,8 @@ static int get_value(const char *key_, const char *regex_)
 
 	if (do_all && system_wide)
 		git_config_from_file(fn, system_wide, data);
+	if (do_all && xdg)
+		git_config_from_file(fn, xdg, data);
 	if (do_all && global)
 		git_config_from_file(fn, global, data);
 	if (do_all)
@@ -238,6 +238,8 @@ static int get_value(const char *key_, const char *regex_)
 		git_config_from_file(fn, local, data);
 	if (!do_all && !seen && global)
 		git_config_from_file(fn, global, data);
+	if (!do_all && !seen && xdg)
+		git_config_from_file(fn, xdg, data);
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
@@ -255,6 +257,7 @@ static int get_value(const char *key_, const char *regex_)
 free_strings:
 	free(repo_config);
 	free(global);
+	free(xdg);
 	return ret;
 }
 
@@ -379,13 +382,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 
 	if (use_global_config) {
-		char *home = getenv("HOME");
-		if (home) {
-			char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
+		char *user_config = NULL;
+		char *xdg_config = NULL;
+
+		home_config_paths(&user_config, &xdg_config, "config");
+
+		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&
+		    (actions == ACTION_LIST ||
+		     actions == ACTION_GET_COLOR ||
+		     actions == ACTION_GET_COLORBOOL))
+			given_config_file = xdg_config;
+		else if (user_config)
 			given_config_file = user_config;
-		} else {
+		else
 			die("$HOME not set");
-		}
 	}
 	else if (use_system_config)
 		given_config_file = git_etc_gitconfig();
diff --git a/cache.h b/cache.h
index 06413e1..0632503 100644
--- a/cache.h
+++ b/cache.h
@@ -708,6 +708,7 @@ int set_shared_perm(const char *path, int mode);
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
+extern void home_config_paths(char **global, char **xdg, char *file);
 extern char *expand_user_path(const char *path);
 const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
diff --git a/config.c b/config.c
index 71ef171..d1393b8 100644
--- a/config.c
+++ b/config.c
@@ -929,7 +929,10 @@ int git_config_system(void)
 int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 {
 	int ret = 0, found = 0;
-	const char *home = NULL;
+	char *xdg_config = NULL;
+	char *user_config = NULL;
+
+	home_config_paths(&user_config, &xdg_config, "config");
 
 	if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {
 		ret += git_config_from_file(fn, git_etc_gitconfig(),
@@ -937,14 +940,14 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		found += 1;
 	}
 
-	home = getenv("HOME");
-	if (home) {
-		char buf[PATH_MAX];
-		char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
-		if (!access(user_config, R_OK)) {
-			ret += git_config_from_file(fn, user_config, data);
-			found += 1;
-		}
+	if (!access(xdg_config, R_OK)) {
+		ret += git_config_from_file(fn, xdg_config, data);
+		found += 1;
+	}
+
+	if (!access(user_config, R_OK)) {
+		ret += git_config_from_file(fn, user_config, data);
+		found += 1;
 	}
 
 	if (repo_config && !access(repo_config, R_OK)) {
diff --git a/path.c b/path.c
index 6f2aa69..53f3f53 100644
--- a/path.c
+++ b/path.c
@@ -122,6 +122,32 @@ char *git_path(const char *fmt, ...)
 	return cleanup_path(pathname);
 }
 
+void home_config_paths(char **global, char **xdg, char *file)
+{
+	char *xdg_home = getenv("XDG_CONFIG_HOME");
+	char *home = getenv("HOME");
+	char *to_free = NULL;
+
+	if (!home) {
+		if (global)
+			*global = NULL;
+	} else {
+		if (!xdg_home) {
+			to_free = strdup(mkpath("%s/.config", home));
+			xdg_home = to_free;
+		}
+		if (global)
+			*global = xstrdup(mkpath("%s/.gitconfig", home));
+	}
+
+	if (!xdg_home)
+		*xdg = NULL;
+	else
+		*xdg = xstrdup(mkpath("%s/git/%s", xdg_home, file));
+
+	free(to_free);
+}
+
 char *git_path_submodule(const char *path, const char *fmt, ...)
 {
 	char *pathname = get_pathname();
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
new file mode 100755
index 0000000..77823fc
--- /dev/null
+++ b/t/t1306-xdg-files.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Valentin Duperray, Lucien Kong, Franck Jonas,
+#		     Thomas Nguy, Khoi Nguyen
+#		     Grenoble INP Ensimag
+#
+
+test_description='Compatibility with $XDG_CONFIG_HOME/git/ files'
+
+. ./test-lib.sh
+
+test_expect_success 'read config: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p .config/git &&
+	echo "[alias]" >.config/git/config &&
+	echo "	myalias = !echo in_config" >>.config/git/config &&
+	echo in_config >expected &&
+	git myalias >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'read config: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[alias]" >.gitconfig &&
+	echo "	myalias = !echo in_gitconfig" >>.gitconfig &&
+	echo in_gitconfig >expected &&
+	git myalias >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo "[user]" >.config/git/config &&
+	echo "	name = read_config" >>.config/git/config &&
+	echo read_config >expected &&
+	git config --get user.name >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo read_gitconfig >expected &&
+	git config --get user.name >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo user.name=read_config >expected &&
+	git config --global --list >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo user.name=read_gitconfig >expected &&
+	git config --global --list >actual &&
+	test_cmp expected actual
+'
+
+test_done
-- 
1.7.8

[PATCHv5 2/4] Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

If core.excludesfile is not defined, its default value will be
$XDG_CONFIG_HOME/git/ignore in order to follow XDG specification. If
$XDG_CONFIG_HOME is either not set or emty, $HOME/.config will be
used.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <Valentin.DUPERRAY@ensimag.imag.fr>
Signed-off-by: Franck JONAS <Franck.JONAS@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <Thomas.NGUY@ensimag.imag.fr>
Signed-off-by: Lucien KONG <Lucien.KONG@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/config.txt    |    4 +++-
 Documentation/gitignore.txt |    4 +++-
 dir.c                       |    7 ++++++-
 t/t1306-xdg-files.sh        |   23 +++++++++++++++++++++++
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 915cb5a..bfaef2c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -483,7 +483,9 @@ core.excludesfile::
 	'.git/info/exclude', git looks into this file for patterns
 	of files which are not meant to be tracked.  "`~/`" is expanded
 	to the value of `$HOME` and "`~user/`" to the specified user's
-	home directory.  See linkgit:gitignore[5].
+	home directory. Its default value is $XDG_CONFIG_HOME/git/ignore.
+	If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config will
+	be used. See linkgit:gitignore[5].
 
 core.askpass::
 	Some commands (e.g. svn and http interfaces) that interactively
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 2e7328b..05ee33f 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -50,7 +50,9 @@ the repository but are specific to one user's workflow) should go into
 the `$GIT_DIR/info/exclude` file.  Patterns which a user wants git to
 ignore in all situations (e.g., backup or temporary files generated by
 the user's editor of choice) generally go into a file specified by
-`core.excludesfile` in the user's `~/.gitconfig`.
+`core.excludesfile` in the user's `~/.gitconfig`. Its default value is
+$XDG_CONFIG_HOME/git/ignore. If $XDG_CONFIG_HOME is either not set or empty,
+$HOME/.config will be used.
 
 The underlying git plumbing tools, such as
 'git ls-files' and 'git read-tree', read
diff --git a/dir.c b/dir.c
index ed1510f..8c6f47f 100644
--- a/dir.c
+++ b/dir.c
@@ -1234,12 +1234,17 @@ int remove_dir_recursively(struct strbuf *path, int flag)
 void setup_standard_excludes(struct dir_struct *dir)
 {
 	const char *path;
+	char *xdg_path;
 
 	dir->exclude_per_dir = ".gitignore";
 	path = git_path("info/exclude");
+	if (!excludes_file) {
+		home_config_paths(NULL, &xdg_path, "ignore");
+		excludes_file = xdg_path;
+	}
 	if (!access(path, R_OK))
 		add_excludes_from_file(dir, path);
-	if (excludes_file && !access(excludes_file, R_OK))
+	if (!access(excludes_file, R_OK))
 		add_excludes_from_file(dir, excludes_file);
 }
 
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 77823fc..1a1f07e 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -61,4 +61,27 @@ test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists'
 	test_cmp expected actual
 '
 
+test_expect_success 'Exclusion of a file in the XDG ignore file' '
+	git init git &&
+	cd git &&
+	echo foo >to_be_excluded &&
+	mkdir -p "$HOME"/.config/git/ &&
+	echo to_be_excluded >"$HOME"/.config/git/ignore &&
+	test_must_fail git add to_be_excluded
+'
+
+test_expect_success 'Exclusion in both XDG and local ignore files' '
+	echo to_be_excluded >.gitignore &&
+	test_must_fail git add to_be_excluded
+'
+
+test_expect_success 'Exclusion in a non-XDG global ignore file' '
+	echo >"$HOME"/.config/git/ignore &&
+	rm .gitignore &&
+	echo to_be_excluded >"$HOME"/my_gitignore &&
+	git config core.excludesfile "$HOME"/my_gitignore &&
+	test_when_finished "cd \"$HOME\" && rm -rf git && rm -r .config" &&
+	test_must_fail git add to_be_excluded
+'
+
 test_done
-- 
1.7.8

Re: [PATCHv5 2/4] Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore

[Matthieu Moy]

Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
writes:

> If core.excludesfile is not defined, its default value will be
> $XDG_CONFIG_HOME/git/ignore in order to follow XDG specification. If
> $XDG_CONFIG_HOME is either not set or emty, $HOME/.config will be
> used.

It's not clear whether $HOME/.config will be used to replace
$XDG_CONFIG_HOME or $XDG_CONFIG_HOME/git/ignore. I'd say explicitely
$HOME/.config/git/ignore to avoid this.

> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -483,7 +483,9 @@ core.excludesfile::
>  	'.git/info/exclude', git looks into this file for patterns
>  	of files which are not meant to be tracked.  "`~/`" is expanded
>  	to the value of `$HOME` and "`~user/`" to the specified user's
> -	home directory.  See linkgit:gitignore[5].
> +	home directory. Its default value is $XDG_CONFIG_HOME/git/ignore.
> +	If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config will
> +	be used. See linkgit:gitignore[5].

Likewise.

> --- a/Documentation/gitignore.txt
> +++ b/Documentation/gitignore.txt
> @@ -50,7 +50,9 @@ the repository but are specific to one user's workflow) should go into
>  the `$GIT_DIR/info/exclude` file.  Patterns which a user wants git to
>  ignore in all situations (e.g., backup or temporary files generated by
>  the user's editor of choice) generally go into a file specified by
> -`core.excludesfile` in the user's `~/.gitconfig`.
> +`core.excludesfile` in the user's `~/.gitconfig`. Its default value is
> +$XDG_CONFIG_HOME/git/ignore. If $XDG_CONFIG_HOME is either not set or empty,
> +$HOME/.config will be used.

Likewise.

Otherwise, sounds good.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCHv5 2/4] Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore

[nguyenhu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=";"; format=flowed	DelSp=Yes, Size: 912 bytes --]


Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :

> Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
> writes:
>
>> If core.excludesfile is not defined, its default value will be
>> $XDG_CONFIG_HOME/git/ignore in order to follow XDG specification. If
>> $XDG_CONFIG_HOME is either not set or emty, $HOME/.config will be
>> used.
>
> It's not clear whether $HOME/.config will be used to replace
> $XDG_CONFIG_HOME or $XDG_CONFIG_HOME/git/ignore. I'd say explicitely
> $HOME/.config/git/ignore to avoid this.
>
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>
> Likewise.
>
>> --- a/Documentation/gitignore.txt
>> +++ b/Documentation/gitignore.txt
>
> Likewise.
>
> Otherwise, sounds good.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

We will also do the same changes with core.attributesfile, in our next  
v6 version.

[PATCHv5 3/4] Let core.attributesfile default to $XDG_CONFIG_HOME/git/attributes

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

If core.attributesfile is not defined, its default value will be
$XDG_CONFIG_HOME/git/attributes in order to follow XDG
specification. If $XDG_CONFIG_HOME is either not set or emty,
$HOME/.config will be used.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <Valentin.DUPERRAY@ensimag.imag.fr>
Signed-off-by: Franck JONAS <Franck.JONAS@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <Thomas.NGUY@ensimag.imag.fr>
Signed-off-by: Lucien KONG <Lucien.KONG@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/config.txt        |    4 +++-
 Documentation/gitattributes.txt |    2 ++
 attr.c                          |   17 ++++++++++-------
 t/t1306-xdg-files.sh            |   30 ++++++++++++++++++++++++++++++
 4 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bfaef2c..9dc274c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -500,7 +500,9 @@ core.attributesfile::
 	In addition to '.gitattributes' (per-directory) and
 	'.git/info/attributes', git looks into this file for attributes
 	(see linkgit:gitattributes[5]). Path expansions are made the same
-	way as for `core.excludesfile`.
+	way as for `core.excludesfile`. Its default value is
+	$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
+	set or empty, $HOME/.config will be used.
 
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 80120ea..3339eef 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -75,6 +75,8 @@ repositories (i.e., attributes of interest to all users) should go into
 `.gitattributes` files. Attributes that should affect all repositories
 for a single user should be placed in a file specified by the
 `core.attributesfile` configuration option (see linkgit:git-config[1]).
+Its default value is $XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME
+is either not set or empty, $HOME/.config will be used.
 Attributes for all users on a system should be placed in the
 `$(prefix)/etc/gitattributes` file.
 
diff --git a/attr.c b/attr.c
index 303751f..aef93d8 100644
--- a/attr.c
+++ b/attr.c
@@ -497,6 +497,7 @@ static int git_attr_system(void)
 static void bootstrap_attr_stack(void)
 {
 	struct attr_stack *elem;
+	char *xdg_attributes_file;
 
 	if (attr_stack)
 		return;
@@ -515,13 +516,15 @@ static void bootstrap_attr_stack(void)
 		}
 	}
 
-	if (git_attributes_file) {
-		elem = read_attr_from_file(git_attributes_file, 1);
-		if (elem) {
-			elem->origin = NULL;
-			elem->prev = attr_stack;
-			attr_stack = elem;
-		}
+	if (!git_attributes_file) {
+		home_config_paths(NULL, &xdg_attributes_file, "attributes");
+		git_attributes_file = xdg_attributes_file;
+	}
+	elem = read_attr_from_file(git_attributes_file, 1);
+	if (elem) {
+		elem->origin = NULL;
+		elem->prev = attr_stack;
+		attr_stack = elem;
 	}
 
 	if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 1a1f07e..80621ed 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -84,4 +84,34 @@ test_expect_success 'Exclusion in a non-XDG global ignore file' '
 	test_must_fail git add to_be_excluded
 '
 
+test_expect_success 'Checking attributes in the XDG attributes file' '
+	git init git &&
+	cd git &&
+	echo foo >f &&
+	git check-attr -a f >actual &&
+	test_line_count -eq 0 actual &&
+	mkdir -p "$HOME"/.config/git/ &&
+	echo "f attr_f" >"$HOME"/.config/git/attributes &&
+	echo "f: attr_f: set" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'Checking attributes in both XDG and local ignore files' '
+	echo "f -attr_f" >.gitattributes &&
+	echo "f: attr_f: unset" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'Checking attributes in a non-XDG global attributes file' '
+	rm .gitattributes &&
+	echo "f attr_f=test" >"$HOME"/my_gitattributes &&
+	git config core.attributesfile "$HOME"/my_gitattributes &&
+	test_when_finished "cd \"$HOME\" && rm -rf git && rm -r .config" &&
+	echo "f: attr_f: test" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.7.8

[PATCHv5 4/4] Write to $XDG_CONFIG_HOME/git/config file

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

Git will be able to write to $XDG_CONFIG_HOME/git/config, a new
configuration file following XDG specification, if:
- this file already exists, and
- $HOME/.gitconfig file doesn't, and
- global option is used.
Otherwise Git writes to $HOME/.gitconfig with global option, as usual.
If you don't create $XDG_CONFIG_HOME/git/config, there is ABSOLUTELY
NO change. Users can use this new file ONLY if they want.
If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config will be
used.
Advice for users who often come back to an old version of GIT:
you shouldn't create this file.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <Valentin.DUPERRAY@ensimag.imag.fr>
Signed-off-by: Franck JONAS <Franck.JONAS@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <Thomas.NGUY@ensimag.imag.fr>
Signed-off-by: Lucien KONG <Lucien.KONG@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/git-config.txt |    3 ++-
 builtin/config.c             |    5 +----
 t/t1306-xdg-files.sh         |   18 ++++++++++++++++++
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 7e344a2..a2b4786 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -97,7 +97,8 @@ OPTIONS
 
 --global::
 	For writing options: write to global ~/.gitconfig file rather than
-	the repository .git/config.
+	the repository .git/config, write to $XDG_CONFIG_HOME/git/config file
+	if this file exists and ~/.gitconfig file doesn't.
 +
 For reading options: read only from global ~/.gitconfig and from
 $XDG_CONFIG_HOME/git/config rather than from all available files.
diff --git a/builtin/config.c b/builtin/config.c
index da54fd1..e8e1c0a 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -387,10 +387,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 		home_config_paths(&user_config, &xdg_config, "config");
 
-		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&
-		    (actions == ACTION_LIST ||
-		     actions == ACTION_GET_COLOR ||
-		     actions == ACTION_GET_COLORBOOL))
+		if (access(user_config, R_OK) && !access(xdg_config, R_OK))
 			given_config_file = xdg_config;
 		else if (user_config)
 			given_config_file = user_config;
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 80621ed..1ff709d 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -114,4 +114,22 @@ test_expect_success 'Checking attributes in a non-XDG global attributes file' '
 	test_cmp expected actual
 '
 
+test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p .config/git &&
+	>.config/git/config &&
+	rm .gitconfig &&
+	git config --global user.name "write_config" &&
+	echo "[user]" >expected &&
+	echo "	name = write_config" >>expected &&
+	test_cmp expected .config/git/config
+'
+
+test_expect_success 'write: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	git config --global user.name "write_gitconfig" &&
+	echo "[user]" >expected &&
+	echo "	name = write_gitconfig" >>expected &&
+	test_cmp expected .gitconfig
+'
+
 test_done
-- 
1.7.8

Re: [PATCHv5 4/4] Write to $XDG_CONFIG_HOME/git/config file

[Matthieu Moy]

Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
writes:

> +test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'\''t' '
> +	mkdir -p .config/git &&
> +	>.config/git/config &&
> +	rm .gitconfig &&
> +	git config --global user.name "write_config" &&
> +	echo "[user]" >expected &&
> +	echo "	name = write_config" >>expected &&
> +	test_cmp expected .config/git/config
> +'

It was suggested that you also write to the XDG file if the XDG
_directory_ exists. After thinking about it, I tend to disagree with
this, because creating ~/.config/git/ignore would let Git write to the
XDG configuration file as a side effect, which I find strange.

Anyway, you may add the case where ~/.config/git/ exists but not the
config file in it to your tests, to materialize the decision you have
taken on that point.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCHv5 4/4] Write to $XDG_CONFIG_HOME/git/config file

[nguyenhu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=";"; format=flowed	DelSp=Yes, Size: 1068 bytes --]


Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :

> Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
> writes:
>
>> +test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'\''t' '
>> +	mkdir -p .config/git &&
>> +	>.config/git/config &&
>> +	rm .gitconfig &&
>> +	git config --global user.name "write_config" &&
>> +	echo "[user]" >expected &&
>> +	echo "	name = write_config" >>expected &&
>> +	test_cmp expected .config/git/config
>> +'
>
> It was suggested that you also write to the XDG file if the XDG
> _directory_ exists. After thinking about it, I tend to disagree with
> this, because creating ~/.config/git/ignore would let Git write to the
> XDG configuration file as a side effect, which I find strange.
>
> Anyway, you may add the case where ~/.config/git/ exists but not the
> config file in it to your tests, to materialize the decision you have
> taken on that point.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

I agree. This case will be added in our next v6 version.

[PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

Git will be able to read from $XDG_CONFIG_HOME/git/config, a new
configuration file following XDG specification. In the order of
reading, this file is between global configuration file and system
wide configuration file. Git currently does not write to this new
configuration file. If $XDG_CONFIG_HOME is either not set or empty,
$HOME/.config/git/config will be used.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/git-config.txt |   12 +++++--
 builtin/config.c             |   28 +++++++++++-----
 cache.h                      |    3 ++
 config.c                     |   23 ++++++++-----
 path.c                       |   40 ++++++++++++++++++++++++
 t/t1306-xdg-files.sh         |   70 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 155 insertions(+), 21 deletions(-)
 create mode 100755 t/t1306-xdg-files.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index d9463cb..7e344a2 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -99,8 +99,8 @@ OPTIONS
 	For writing options: write to global ~/.gitconfig file rather than
 	the repository .git/config.
 +
-For reading options: read only from global ~/.gitconfig rather than
-from all available files.
+For reading options: read only from global ~/.gitconfig and from
+$XDG_CONFIG_HOME/git/config rather than from all available files.
 +
 See also <<FILES>>.
 
@@ -194,7 +194,7 @@ See also <<FILES>>.
 FILES
 -----
 
-If not set explicitly with '--file', there are three files where
+If not set explicitly with '--file', there are four files where
 'git config' will search for configuration options:
 
 $GIT_DIR/config::
@@ -204,6 +204,12 @@ $GIT_DIR/config::
 	User-specific configuration file. Also called "global"
 	configuration file.
 
+$XDG_CONFIG_HOME/git/config::
+	Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
+	or empty, $HOME/.config/git/config will be used. Any single-valued
+	variable set in this file will be overwritten by whatever is in
+	~/.gitconfig.
+
 $(prefix)/etc/gitconfig::
 	System-wide configuration file.
 
diff --git a/builtin/config.c b/builtin/config.c
index 33c8820..da54fd1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -161,7 +161,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = -1;
-	char *global = NULL, *repo_config = NULL;
+	char *global = NULL, *xdg = NULL, *repo_config = NULL;
 	const char *system_wide = NULL, *local;
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 	config_fn_t fn;
@@ -169,12 +169,10 @@ static int get_value(const char *key_, const char *regex_)
 
 	local = given_config_file;
 	if (!local) {
-		const char *home = getenv("HOME");
 		local = repo_config = git_pathdup("config");
-		if (home)
-			global = xstrdup(mkpath("%s/.gitconfig", home));
 		if (git_config_system())
 			system_wide = git_etc_gitconfig();
+		home_config_paths(&global, &xdg, "config");
 	}
 
 	if (use_key_regexp) {
@@ -229,6 +227,8 @@ static int get_value(const char *key_, const char *regex_)
 
 	if (do_all && system_wide)
 		git_config_from_file(fn, system_wide, data);
+	if (do_all && xdg)
+		git_config_from_file(fn, xdg, data);
 	if (do_all && global)
 		git_config_from_file(fn, global, data);
 	if (do_all)
@@ -238,6 +238,8 @@ static int get_value(const char *key_, const char *regex_)
 		git_config_from_file(fn, local, data);
 	if (!do_all && !seen && global)
 		git_config_from_file(fn, global, data);
+	if (!do_all && !seen && xdg)
+		git_config_from_file(fn, xdg, data);
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
@@ -255,6 +257,7 @@ static int get_value(const char *key_, const char *regex_)
 free_strings:
 	free(repo_config);
 	free(global);
+	free(xdg);
 	return ret;
 }
 
@@ -379,13 +382,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 
 	if (use_global_config) {
-		char *home = getenv("HOME");
-		if (home) {
-			char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
+		char *user_config = NULL;
+		char *xdg_config = NULL;
+
+		home_config_paths(&user_config, &xdg_config, "config");
+
+		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&
+		    (actions == ACTION_LIST ||
+		     actions == ACTION_GET_COLOR ||
+		     actions == ACTION_GET_COLORBOOL))
+			given_config_file = xdg_config;
+		else if (user_config)
 			given_config_file = user_config;
-		} else {
+		else
 			die("$HOME not set");
-		}
 	}
 	else if (use_system_config)
 		given_config_file = git_etc_gitconfig();
diff --git a/cache.h b/cache.h
index 06413e1..fbba2d6 100644
--- a/cache.h
+++ b/cache.h
@@ -619,6 +619,8 @@ extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
 extern char *git_pathdup(const char *fmt, ...)
 	__attribute__((format (printf, 1, 2)));
+extern char *mkpathdup(const char *fmt, ...)
+	__attribute__((format (printf, 1, 2)));
 
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
@@ -708,6 +710,7 @@ int set_shared_perm(const char *path, int mode);
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
+extern void home_config_paths(char **global, char **xdg, char *file);
 extern char *expand_user_path(const char *path);
 const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
diff --git a/config.c b/config.c
index 71ef171..d28a499 100644
--- a/config.c
+++ b/config.c
@@ -929,7 +929,10 @@ int git_config_system(void)
 int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 {
 	int ret = 0, found = 0;
-	const char *home = NULL;
+	char *xdg_config = NULL;
+	char *user_config = NULL;
+
+	home_config_paths(&user_config, &xdg_config, "config");
 
 	if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {
 		ret += git_config_from_file(fn, git_etc_gitconfig(),
@@ -937,14 +940,14 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		found += 1;
 	}
 
-	home = getenv("HOME");
-	if (home) {
-		char buf[PATH_MAX];
-		char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
-		if (!access(user_config, R_OK)) {
-			ret += git_config_from_file(fn, user_config, data);
-			found += 1;
-		}
+	if (!access(xdg_config, R_OK)) {
+		ret += git_config_from_file(fn, xdg_config, data);
+		found += 1;
+	}
+
+	if (!access(user_config, R_OK)) {
+		ret += git_config_from_file(fn, user_config, data);
+		found += 1;
 	}
 
 	if (repo_config && !access(repo_config, R_OK)) {
@@ -963,6 +966,8 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		break;
 	}
 
+	free(xdg_config);
+	free(user_config);
 	return ret == 0 ? found : ret;
 }
 
diff --git a/path.c b/path.c
index 6f2aa69..ca29bdd 100644
--- a/path.c
+++ b/path.c
@@ -87,6 +87,20 @@ char *git_pathdup(const char *fmt, ...)
 	return xstrdup(path);
 }
 
+char *mkpathdup(const char *fmt, ...)
+{
+	char path[PATH_MAX];
+	va_list args;
+	unsigned len;
+
+	va_start(args, fmt);
+	len = vsnprintf(path, sizeof(path), fmt, args);
+	va_end(args);
+	if (len >= sizeof(path))
+		return xstrdup(bad_path);
+	return xstrdup(cleanup_path(path));
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
@@ -122,6 +136,32 @@ char *git_path(const char *fmt, ...)
 	return cleanup_path(pathname);
 }
 
+void home_config_paths(char **global, char **xdg, char *file)
+{
+	char *xdg_home = getenv("XDG_CONFIG_HOME");
+	char *home = getenv("HOME");
+	char *to_free = NULL;
+
+	if (!home) {
+		if (global)
+			*global = NULL;
+	} else {
+		if (!xdg_home) {
+			to_free = mkpathdup("%s/.config", home);
+			xdg_home = to_free;
+		}
+		if (global)
+			*global = mkpathdup("%s/.gitconfig", home);
+	}
+
+	if (!xdg_home)
+		*xdg = NULL;
+	else
+		*xdg = mkpathdup("%s/git/%s", xdg_home, file);
+
+	free(to_free);
+}
+
 char *git_path_submodule(const char *path, const char *fmt, ...)
 {
 	char *pathname = get_pathname();
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
new file mode 100755
index 0000000..5b971d9
--- /dev/null
+++ b/t/t1306-xdg-files.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Valentin Duperray, Lucien Kong, Franck Jonas,
+#		     Thomas Nguy, Khoi Nguyen
+#		     Grenoble INP Ensimag
+#
+
+test_description='Compatibility with $XDG_CONFIG_HOME/git/ files'
+
+. ./test-lib.sh
+
+test_expect_success 'read config: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p .config/git &&
+	echo "[alias]" >.config/git/config &&
+	echo "	myalias = !echo in_config" >>.config/git/config &&
+	echo in_config >expected &&
+	git myalias >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read config: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[alias]" >.gitconfig &&
+	echo "	myalias = !echo in_gitconfig" >>.gitconfig &&
+	echo in_gitconfig >expected &&
+	git myalias >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo "[user]" >.config/git/config &&
+	echo "	name = read_config" >>.config/git/config &&
+	echo read_config >expected &&
+	git config --get user.name >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo read_gitconfig >expected &&
+	git config --get user.name >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo user.name=read_config >expected &&
+	git config --global --list >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo user.name=read_gitconfig >expected &&
+	git config --global --list >actual &&
+	test_cmp expected actual
+'
+
+
+test_done
-- 
1.7.8

[PATCHv6 2/4] Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

If core.excludesfile is not defined, its default value will be
$XDG_CONFIG_HOME/git/ignore in order to follow XDG specification. If
$XDG_CONFIG_HOME is either not set or emty, $HOME/.config/git/ignore
will be used.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/config.txt    |    4 +++-
 Documentation/gitignore.txt |    4 +++-
 dir.c                       |    7 ++++++-
 t/t1306-xdg-files.sh        |   40 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 915cb5a..509bf25 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -483,7 +483,9 @@ core.excludesfile::
 	'.git/info/exclude', git looks into this file for patterns
 	of files which are not meant to be tracked.  "`~/`" is expanded
 	to the value of `$HOME` and "`~user/`" to the specified user's
-	home directory.  See linkgit:gitignore[5].
+	home directory. Its default value is $XDG_CONFIG_HOME/git/ignore.
+	If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/ignore
+	will be used. See linkgit:gitignore[5].
 
 core.askpass::
 	Some commands (e.g. svn and http interfaces) that interactively
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 2e7328b..a5c9b93 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -50,7 +50,9 @@ the repository but are specific to one user's workflow) should go into
 the `$GIT_DIR/info/exclude` file.  Patterns which a user wants git to
 ignore in all situations (e.g., backup or temporary files generated by
 the user's editor of choice) generally go into a file specified by
-`core.excludesfile` in the user's `~/.gitconfig`.
+`core.excludesfile` in the user's `~/.gitconfig`. Its default value is
+$XDG_CONFIG_HOME/git/ignore. If $XDG_CONFIG_HOME is either not set or empty,
+$HOME/.config/git/ignore will be used.
 
 The underlying git plumbing tools, such as
 'git ls-files' and 'git read-tree', read
diff --git a/dir.c b/dir.c
index ed1510f..8c6f47f 100644
--- a/dir.c
+++ b/dir.c
@@ -1234,12 +1234,17 @@ int remove_dir_recursively(struct strbuf *path, int flag)
 void setup_standard_excludes(struct dir_struct *dir)
 {
 	const char *path;
+	char *xdg_path;
 
 	dir->exclude_per_dir = ".gitignore";
 	path = git_path("info/exclude");
+	if (!excludes_file) {
+		home_config_paths(NULL, &xdg_path, "ignore");
+		excludes_file = xdg_path;
+	}
 	if (!access(path, R_OK))
 		add_excludes_from_file(dir, path);
-	if (excludes_file && !access(excludes_file, R_OK))
+	if (!access(excludes_file, R_OK))
 		add_excludes_from_file(dir, excludes_file);
 }
 
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 5b971d9..873bbcf 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -67,4 +67,44 @@ test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists'
 '
 
 
+test_expect_success 'Exclusion of a file in the XDG ignore file' '
+	test_when_finished "rm -rf git" &&
+	git init git &&
+	(
+		cd git &&
+		echo foo >to_be_excluded &&
+		mkdir -p "$HOME"/.config/git/ &&
+		echo to_be_excluded >"$HOME"/.config/git/ignore &&
+		test_must_fail git add to_be_excluded
+	)
+'
+
+
+test_expect_success 'Exclusion in both XDG and local ignore files' '
+	test_when_finished "rm -rf git" &&
+	git init git &&
+	(
+		cd git &&
+		echo foo >to_be_excluded &&
+		echo to_be_excluded >"$HOME"/.config/git/ignore &&
+		echo to_be_excluded >.gitignore &&
+		test_must_fail git add to_be_excluded
+	)
+'
+
+
+test_expect_success 'Exclusion in a non-XDG global ignore file' '
+	test_when_finished "rm -rf git" &&
+	git init git &&
+	(
+		cd git &&
+		echo foo >to_be_excluded &&
+		echo >"$HOME"/.config/git/ignore &&
+		echo to_be_excluded >"$HOME"/my_gitignore &&
+		git config core.excludesfile "$HOME"/my_gitignore &&
+		test_must_fail git add to_be_excluded
+	)
+'
+
+
 test_done
-- 
1.7.8

Re: [PATCHv6 2/4] Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore

[Junio C Hamano]

Huynh Khoi Nguyen NGUYEN  <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
writes:

> From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>
>
> If core.excludesfile is not defined, its default value will be
> $XDG_CONFIG_HOME/git/ignore in order to follow XDG specification.

I find the above statement hard to understand and harder to agree
with.

Right now, when core.excludesfile is not defined, the user does not
have to worry about any "global" ignore file to get in the way while
working on a specific project.  With this change, if the user does
not want random paths in the project that happens to match a pattern
in this XDG_CONFIG_HOME/git/ignore file that is totally outside of
the project's control, the user needs to make sure core.excludesfile
is set to an empty file or something.

That sounds like a huge downside; does XDG specification mandates
such a regression?  Do they say "you must treat XDG_CONFIG_HOME/git/ignore
as your global ignore file"?  It is hard to believe.

Perhaps you are doing two or more unrelated things and stating what
you are doing poorly, which is what is confusing me?

Perhaps the problem you are solving is something like this (by the
way, you didn't state what problem you are trying to solve at all):

 - Use of core.excludesfile is an opt-in feature to keep ignore
   patterns in a file and use it across projects defined per user.
   To use this feature, the user needs to create such a file, and
   add configuration variable to point at it.  The feature needs two
   user actions to enable.

 - Instead, we can define a filename that is unlikely to already
   exist on the system, and use the presense of the file as a cue
   that the user wants to use that feature.  The user can simply
   create the file without adding configuration variable to point at
   it. Now the feature needs only one user action to enable, which
   is the primary problem I wanted to solve.

 - Now we need to decide what that magic filename should be.  We
   could pick any random filename that user can create, and no other
   user can create for the user, which means that it has to be under
   $HOME.  Let's use "${XDG_CONFIG_HOME:-$HOME/.config/git}/ignore";
   it is as good as any random filename in $HOME/ and follows the
   XDG guideline.

But that may not be what you meant to say.  I dunno.

Re: [PATCHv6 2/4] Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Perhaps the problem you are solving is something like this (by the
> way, you didn't state what problem you are trying to solve at all):
> [...]

That is a very good summary of what the patch tries to do. Following XDG
is a nice side effect, but not the primary goal.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCHv6 2/4] Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore

[nguyenhu]

Junio C Hamano <gitster@pobox.com> a écrit :

> But that may not be what you meant to say.  I dunno.

It is exactly what we want to do. It is true the statement was very  
short and we didn't state what problem we are trying to solve. Your  
points raise the issues and explain clearly our solution.

[PATCHv6 3/4] Let core.attributesfile default to $XDG_CONFIG_HOME/git/attributes

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

If core.attributesfile is not defined, its default value will be
$XDG_CONFIG_HOME/git/attributes in order to follow XDG
specification. If $XDG_CONFIG_HOME is either not set or emty,
$HOME/.config/git/attributes will be used.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/config.txt        |    4 ++-
 Documentation/gitattributes.txt |    2 +
 attr.c                          |   17 ++++++++-----
 t/t1306-xdg-files.sh            |   47 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 509bf25..f691d8e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -500,7 +500,9 @@ core.attributesfile::
 	In addition to '.gitattributes' (per-directory) and
 	'.git/info/attributes', git looks into this file for attributes
 	(see linkgit:gitattributes[5]). Path expansions are made the same
-	way as for `core.excludesfile`.
+	way as for `core.excludesfile`. Its default value is
+	$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
+	set or empty, $HOME/.config/git/attributes will be used.
 
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 80120ea..484c614 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -75,6 +75,8 @@ repositories (i.e., attributes of interest to all users) should go into
 `.gitattributes` files. Attributes that should affect all repositories
 for a single user should be placed in a file specified by the
 `core.attributesfile` configuration option (see linkgit:git-config[1]).
+Its default value is $XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME
+is either not set or empty, $HOME/.config/git/attributes will be used.
 Attributes for all users on a system should be placed in the
 `$(prefix)/etc/gitattributes` file.
 
diff --git a/attr.c b/attr.c
index 303751f..aef93d8 100644
--- a/attr.c
+++ b/attr.c
@@ -497,6 +497,7 @@ static int git_attr_system(void)
 static void bootstrap_attr_stack(void)
 {
 	struct attr_stack *elem;
+	char *xdg_attributes_file;
 
 	if (attr_stack)
 		return;
@@ -515,13 +516,15 @@ static void bootstrap_attr_stack(void)
 		}
 	}
 
-	if (git_attributes_file) {
-		elem = read_attr_from_file(git_attributes_file, 1);
-		if (elem) {
-			elem->origin = NULL;
-			elem->prev = attr_stack;
-			attr_stack = elem;
-		}
+	if (!git_attributes_file) {
+		home_config_paths(NULL, &xdg_attributes_file, "attributes");
+		git_attributes_file = xdg_attributes_file;
+	}
+	elem = read_attr_from_file(git_attributes_file, 1);
+	if (elem) {
+		elem->origin = NULL;
+		elem->prev = attr_stack;
+		attr_stack = elem;
 	}
 
 	if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 873bbcf..eb0f40b 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -107,4 +107,51 @@ test_expect_success 'Exclusion in a non-XDG global ignore file' '
 '
 
 
+test_expect_success 'Checking attributes in the XDG attributes file' '
+	test_when_finished "rm -rf git" &&
+	git init git &&
+	(
+		cd git &&
+		echo foo >f &&
+		git check-attr -a f >actual &&
+		test_line_count -eq 0 actual &&
+		echo "f attr_f" >"$HOME"/.config/git/attributes &&
+		echo "f: attr_f: set" >expected &&
+		git check-attr -a f >actual &&
+		test_cmp expected actual
+	)
+'
+
+
+test_expect_success 'Checking attributes in both XDG and local ignore files' '
+	test_when_finished "rm -rf git" &&
+	git init git &&
+	(
+		cd git &&
+		echo foo >f &&
+		echo "f attr_f" >"$HOME"/.config/git/attributes &&
+		echo "f -attr_f" >.gitattributes &&
+		echo "f: attr_f: unset" >expected &&
+		git check-attr -a f >actual &&
+		test_cmp expected actual
+	)
+'
+
+
+test_expect_success 'Checking attributes in a non-XDG global attributes file' '
+	test_when_finished "rm -rf git" &&
+	git init git &&
+	(
+		cd git &&
+		echo foo >f &&
+		echo "f attr_f" >"$HOME"/.config/git/attributes &&
+		echo "f attr_f=test" >"$HOME"/my_gitattributes &&
+		git config core.attributesfile "$HOME"/my_gitattributes &&
+		echo "f: attr_f: test" >expected &&
+		git check-attr -a f >actual &&
+		test_cmp expected actual
+	)
+'
+
+
 test_done
-- 
1.7.8

[PATCHv6 4/4] Write to $XDG_CONFIG_HOME/git/config file

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>

Git will be able to write to $XDG_CONFIG_HOME/git/config, a new
configuration file following XDG specification, if:
- this file already exists, and
- $HOME/.gitconfig file doesn't, and
- global option is used.
Otherwise Git writes to $HOME/.gitconfig with global option, as usual.
If you don't create $XDG_CONFIG_HOME/git/config, there is ABSOLUTELY
NO change. Users can use this new file ONLY if they want.
If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/config
will be used.
Advice for users who often come back to an old version of GIT:
you shouldn't create this file.

Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/git-config.txt |    3 ++-
 builtin/config.c             |    5 +----
 t/t1306-xdg-files.sh         |   30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 7e344a2..a2b4786 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -97,7 +97,8 @@ OPTIONS
 
 --global::
 	For writing options: write to global ~/.gitconfig file rather than
-	the repository .git/config.
+	the repository .git/config, write to $XDG_CONFIG_HOME/git/config file
+	if this file exists and ~/.gitconfig file doesn't.
 +
 For reading options: read only from global ~/.gitconfig and from
 $XDG_CONFIG_HOME/git/config rather than from all available files.
diff --git a/builtin/config.c b/builtin/config.c
index da54fd1..e8e1c0a 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -387,10 +387,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 		home_config_paths(&user_config, &xdg_config, "config");
 
-		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&
-		    (actions == ACTION_LIST ||
-		     actions == ACTION_GET_COLOR ||
-		     actions == ACTION_GET_COLORBOOL))
+		if (access(user_config, R_OK) && !access(xdg_config, R_OK))
 			given_config_file = xdg_config;
 		else if (user_config)
 			given_config_file = user_config;
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index eb0f40b..dbf236a 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -154,4 +154,34 @@ test_expect_success 'Checking attributes in a non-XDG global attributes file' '
 '
 
 
+test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p .config/git &&
+	>.config/git/config &&
+	rm .gitconfig &&
+	git config --global user.name "write_config" &&
+	echo "[user]" >expected &&
+	echo "	name = write_config" >>expected &&
+	test_cmp expected .config/git/config
+'
+
+
+test_expect_success 'write: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	git config --global user.name "write_gitconfig" &&
+	echo "[user]" >expected &&
+	echo "	name = write_gitconfig" >>expected &&
+	test_cmp expected .gitconfig
+'
+
+
+test_expect_success 'write: ~/.config/git/ exists and config file doesn'\''t' '
+	rm .gitconfig &&
+	rm .config/git/config &&
+	git config --global user.name "write_gitconfig" &&
+	echo "[user]" >expected &&
+	echo "	name = write_gitconfig" >>expected &&
+	test_cmp expected .gitconfig
+'
+
+
 test_done
-- 
1.7.8

Re: [PATCHv6 4/4] Write to $XDG_CONFIG_HOME/git/config file

[David Aguilar]

On Wed, Jun 6, 2012 at 6:21 AM, Huynh Khoi Nguyen NGUYEN
<Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr> wrote:
> From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>
>
> Git will be able to write to $XDG_CONFIG_HOME/git/config, a new
> configuration file following XDG specification, if:
> - this file already exists, and
> - $HOME/.gitconfig file doesn't, and
> - global option is used.
> Otherwise Git writes to $HOME/.gitconfig with global option, as usual.
> If you don't create $XDG_CONFIG_HOME/git/config, there is ABSOLUTELY
> NO change. Users can use this new file ONLY if they want.
> If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/config
> will be used.
> Advice for users who often come back to an old version of GIT:
> you shouldn't create this file.
>
> Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
> Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
> Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
> Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
> Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> ---

I'm all for this change too.

I did a little research RE: the OSX and Windows question.

glib makes no differentiation between OSX and Linux,
but Windows does have its own #ifdef.

http://git.gnome.org/browse/glib/tree/glib/gutils.c#n1251

I certainly don't think this should affect this series,
I'm just noting it as something that the Windows folks might
care about.  Perhaps something for compat/ in the future?

The downside to doing the same as glib is more documentation.
The upside is... consistency?

Thanks,
David


>  Documentation/git-config.txt |    3 ++-
>  builtin/config.c             |    5 +----
>  t/t1306-xdg-files.sh         |   30 ++++++++++++++++++++++++++++++
>  3 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index 7e344a2..a2b4786 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -97,7 +97,8 @@ OPTIONS
>
>  --global::
>        For writing options: write to global ~/.gitconfig file rather than
> -       the repository .git/config.
> +       the repository .git/config, write to $XDG_CONFIG_HOME/git/config file
> +       if this file exists and ~/.gitconfig file doesn't.
>  +
>  For reading options: read only from global ~/.gitconfig and from
>  $XDG_CONFIG_HOME/git/config rather than from all available files.
> diff --git a/builtin/config.c b/builtin/config.c
> index da54fd1..e8e1c0a 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -387,10 +387,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>
>                home_config_paths(&user_config, &xdg_config, "config");
>
> -               if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&
> -                   (actions == ACTION_LIST ||
> -                    actions == ACTION_GET_COLOR ||
> -                    actions == ACTION_GET_COLORBOOL))
> +               if (access(user_config, R_OK) && !access(xdg_config, R_OK))
>                        given_config_file = xdg_config;
>                else if (user_config)
>                        given_config_file = user_config;
> diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
> index eb0f40b..dbf236a 100755
> --- a/t/t1306-xdg-files.sh
> +++ b/t/t1306-xdg-files.sh
> @@ -154,4 +154,34 @@ test_expect_success 'Checking attributes in a non-XDG global attributes file' '
>  '
>
>
> +test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'\''t' '
> +       mkdir -p .config/git &&
> +       >.config/git/config &&
> +       rm .gitconfig &&
> +       git config --global user.name "write_config" &&
> +       echo "[user]" >expected &&
> +       echo "  name = write_config" >>expected &&
> +       test_cmp expected .config/git/config
> +'
> +
> +
> +test_expect_success 'write: xdg file exists and ~/.gitconfig exists' '
> +       >.gitconfig &&
> +       git config --global user.name "write_gitconfig" &&
> +       echo "[user]" >expected &&
> +       echo "  name = write_gitconfig" >>expected &&
> +       test_cmp expected .gitconfig
> +'
> +
> +
> +test_expect_success 'write: ~/.config/git/ exists and config file doesn'\''t' '
> +       rm .gitconfig &&
> +       rm .config/git/config &&
> +       git config --global user.name "write_gitconfig" &&
> +       echo "[user]" >expected &&
> +       echo "  name = write_gitconfig" >>expected &&
> +       test_cmp expected .gitconfig
> +'
> +
> +
>  test_done
> --
> 1.7.8

Re: [PATCHv6 4/4] Write to $XDG_CONFIG_HOME/git/config file

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

> I'm all for this change too.

Do you mean just the general direction, or including the
implementation?

> I did a little research RE: the OSX and Windows question.
>
> glib makes no differentiation between OSX and Linux,
> but Windows does have its own #ifdef.
>
> http://git.gnome.org/browse/glib/tree/glib/gutils.c#n1251
>
> I certainly don't think this should affect this series,
> I'm just noting it as something that the Windows folks might
> care about.  Perhaps something for compat/ in the future?
>
> The downside to doing the same as glib is more documentation.
> The upside is... consistency?

Probably.  I think we should follow whatever existing and prevalent
practice is, and my gut feeling is that we would end up first doing
POSIX only thing in my tree, and msysgit folks will quickly feed me
updates to tweak the behaviour to match what of Windows version.

Re: [PATCHv6 4/4] Write to $XDG_CONFIG_HOME/git/config file

[David Aguilar]

On Fri, Jun 8, 2012 at 11:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> I'm all for this change too.
>
> Do you mean just the general direction, or including the
> implementation?

General direction, including the backwards-compatibility
concerns described in the commit message.


>> I did a little research RE: the OSX and Windows question.
>>
>> glib makes no differentiation between OSX and Linux,
>> but Windows does have its own #ifdef.
>>
>> http://git.gnome.org/browse/glib/tree/glib/gutils.c#n1251
>>
>> I certainly don't think this should affect this series,
>> I'm just noting it as something that the Windows folks might
>> care about.  Perhaps something for compat/ in the future?
>>
>> The downside to doing the same as glib is more documentation.
>> The upside is... consistency?
>
> Probably.  I think we should follow whatever existing and prevalent
> practice is, and my gut feeling is that we would end up first doing
> POSIX only thing in my tree, and msysgit folks will quickly feed me
> updates to tweak the behaviour to match what of Windows version.

That sounds good to me.
Thanks,
-- 
David

Re: [PATCHv6 4/4] Write to $XDG_CONFIG_HOME/git/config file

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Probably.  I think we should follow whatever existing and prevalent
> practice is, and my gut feeling is that we would end up first doing
> POSIX only thing in my tree, and msysgit folks will quickly feed me
> updates to tweak the behaviour to match what of Windows version.

I don't really have opinion on what Git should do on windows, but in any
case, the current patches only changes the behavior if the XDG file
exists, so it may be suboptimal on windows, but shouldn't harm windows
users.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCHv6 4/4] Write to $XDG_CONFIG_HOME/git/config file

[nguyenhu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=";"; format=flowed	DelSp=Yes, Size: 755 bytes --]



Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> a écrit :

> I don't really have opinion on what Git should do on windows, but in any
> case, the current patches only changes the behavior if the XDG file
> exists, so it may be suboptimal on windows, but shouldn't harm windows
> users.

Yes, if the XDG file doesn't exist, there is no change, and the behaviour is
the same, with Linux, OSX or Windows. But it is true that, although  
windows users
will not have problems with the current patches even if they use the XDG file,
it is not opimized for Windows, and it would be nice to optimize the case of
Windows. For the time being, it is not the goal of this series of patches.
Another patch will certainly add this optimization, I hope.

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Junio C Hamano]

> +char *mkpathdup(const char *fmt, ...)
> +{
> +	char path[PATH_MAX];
> +	va_list args;
> +	unsigned len;
> +
> +	va_start(args, fmt);
> +	len = vsnprintf(path, sizeof(path), fmt, args);
> +	va_end(args);
> +	if (len >= sizeof(path))
> +		return xstrdup(bad_path);
> +	return xstrdup(cleanup_path(path));
> +}

Hrmph. If a new helper is introduced anyway, wouldn't it be a better
idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
strbuf_vaddf() or something in the implementation of this function?

> diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
> new file mode 100755
> index 0000000..5b971d9
> --- /dev/null
> +++ b/t/t1306-xdg-files.sh
> @@ -0,0 +1,70 @@
> ...
> +test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\''t' '
> +	rm .gitconfig &&

"rm -f .gitconfig"; always consider the possibility that any part of
the previous test could have been omitted or failed.

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[nguyenhu]

Junio C Hamano <gitster@pobox.com> a écrit :

>> +char *mkpathdup(const char *fmt, ...)
>> +{
>> +	char path[PATH_MAX];
>> +	va_list args;
>> +	unsigned len;
>> +
>> +	va_start(args, fmt);
>> +	len = vsnprintf(path, sizeof(path), fmt, args);
>> +	va_end(args);
>> +	if (len >= sizeof(path))
>> +		return xstrdup(bad_path);
>> +	return xstrdup(cleanup_path(path));
>> +}
>
> Hrmph. If a new helper is introduced anyway, wouldn't it be a better
> idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
> strbuf_vaddf() or something in the implementation of this function?

Ramsay Jones, what do you think about this ?

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Ramsay Jones]

nguyenhu@minatec.inpg.fr wrote:
> Junio C Hamano <gitster@pobox.com> a écrit :
> 
>>> +char *mkpathdup(const char *fmt, ...)
>>> +{
>>> +	char path[PATH_MAX];
>>> +	va_list args;
>>> +	unsigned len;
>>> +
>>> +	va_start(args, fmt);
>>> +	len = vsnprintf(path, sizeof(path), fmt, args);
>>> +	va_end(args);
>>> +	if (len >= sizeof(path))
>>> +		return xstrdup(bad_path);
>>> +	return xstrdup(cleanup_path(path));
>>> +}
>> Hrmph. If a new helper is introduced anyway, wouldn't it be a better
>> idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
>> strbuf_vaddf() or something in the implementation of this function?
> 
> Ramsay Jones, what do you think about this ?

I think that I'm sorry for the late reply, I've been away ...  :-D

I noticed your new series (v7 I think) which looked good (as far as the
mkpathdup() implementation is concerned) and I don't think it will tickle
the cygwin bug. (I haven't actually fetched that mail yet, I've only read
it using my ISPs web-mail interface, but will do so soon and test it on
cygwin).

Thanks!

ATB,
Ramsay Jones

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[nguyenhu]

Junio C Hamano <gitster@pobox.com> a écrit :

>> +char *mkpathdup(const char *fmt, ...)
>> +{
>> +	char path[PATH_MAX];
>> +	va_list args;
>> +	unsigned len;
>> +
>> +	va_start(args, fmt);
>> +	len = vsnprintf(path, sizeof(path), fmt, args);
>> +	va_end(args);
>> +	if (len >= sizeof(path))
>> +		return xstrdup(bad_path);
>> +	return xstrdup(cleanup_path(path));
>> +}
>
> Hrmph. If a new helper is introduced anyway, wouldn't it be a better
> idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
> strbuf_vaddf() or something in the implementation of this function?

What about this ?

char *mkpathdup(const char *fmt, ...)
{
	char *path;
	struct strbuf sb = STRBUF_INIT;
	va_list args;

	va_start(args, fmt);
	strbuf_vaddf(&sb, fmt, args);
	va_end(args);
	path = sb.buf;

	strbuf_release(&sb);
	return xstrdup(cleanup_path(path));
}

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Erik Faye-Lund]

On Fri, Jun 8, 2012 at 2:26 PM,  <nguyenhu@minatec.inpg.fr> wrote:
>
> Junio C Hamano <gitster@pobox.com> a écrit :
>
>>> +char *mkpathdup(const char *fmt, ...)
>>> +{
>>> +       char path[PATH_MAX];
>>> +       va_list args;
>>> +       unsigned len;
>>> +
>>> +       va_start(args, fmt);
>>> +       len = vsnprintf(path, sizeof(path), fmt, args);
>>> +       va_end(args);
>>> +       if (len >= sizeof(path))
>>> +               return xstrdup(bad_path);
>>> +       return xstrdup(cleanup_path(path));
>>> +}
>>
>>
>> Hrmph. If a new helper is introduced anyway, wouldn't it be a better
>> idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
>> strbuf_vaddf() or something in the implementation of this function?
>
>
> What about this ?
>
>
> char *mkpathdup(const char *fmt, ...)
> {
>        char *path;
>        struct strbuf sb = STRBUF_INIT;
>        va_list args;
>
>        va_start(args, fmt);
>        strbuf_vaddf(&sb, fmt, args);
>        va_end(args);
>        path = sb.buf;
>
>        strbuf_release(&sb);
>        return xstrdup(cleanup_path(path));
>
> }

No, strbuf_release(&sb) frees 'sb.buf', causing 'path' to point to
unallocated memory. You can fix that by doing something along these
lines on top:

 	va_end(args);
-	path = sb.buf;
+	path = xstrdup(cleanup_path(path));

 	strbuf_release(&sb);
-	return xstrdup(cleanup_path(path));
+	return path;
 }

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[nguyenhu]

Erik Faye-Lund <kusmabite@gmail.com> a écrit :

>> char *mkpathdup(const char *fmt, ...)
>> {
>>        char *path;
>>        struct strbuf sb = STRBUF_INIT;
>>        va_list args;
>>
>>        va_start(args, fmt);
>>        strbuf_vaddf(&sb, fmt, args);
>>        va_end(args);
>>        path = sb.buf;
>>
>>        strbuf_release(&sb);
>>        return xstrdup(cleanup_path(path));
>>
>> }
>
> No, strbuf_release(&sb) frees 'sb.buf', causing 'path' to point to
> unallocated memory. You can fix that by doing something along these
> lines on top:
>
>  	va_end(args);
> -	path = sb.buf;
> +	path = xstrdup(cleanup_path(path));
>
>  	strbuf_release(&sb);
> -	return xstrdup(cleanup_path(path));
> +	return path;
>  }

You are right, but I think you rather mean this, no?

  	va_end(args);
-	path = sb.buf;
+	path = xstrdup(cleanup_path(sb.buf));

  	strbuf_release(&sb);
-	return xstrdup(cleanup_path(path));
+	return path;
  }

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Erik Faye-Lund]

On Fri, Jun 8, 2012 at 2:54 PM,  <nguyenhu@minatec.inpg.fr> wrote:
>
> Erik Faye-Lund <kusmabite@gmail.com> a écrit :
>
>
>>> char *mkpathdup(const char *fmt, ...)
>>> {
>>>        char *path;
>>>        struct strbuf sb = STRBUF_INIT;
>>>        va_list args;
>>>
>>>        va_start(args, fmt);
>>>        strbuf_vaddf(&sb, fmt, args);
>>>        va_end(args);
>>>        path = sb.buf;
>>>
>>>        strbuf_release(&sb);
>>>        return xstrdup(cleanup_path(path));
>>>
>>> }
>>
>>
>> No, strbuf_release(&sb) frees 'sb.buf', causing 'path' to point to
>> unallocated memory. You can fix that by doing something along these
>> lines on top:
>>
>>        va_end(args);
>> -       path = sb.buf;
>> +       path = xstrdup(cleanup_path(path));
>>
>>        strbuf_release(&sb);
>> -       return xstrdup(cleanup_path(path));
>> +       return path;
>>  }
>
>
> You are right, but I think you rather mean this, no?
>
>
>        va_end(args);
> -       path = sb.buf;
> +       path = xstrdup(cleanup_path(sb.buf));
>
>
>        strbuf_release(&sb);
> -       return xstrdup(cleanup_path(path));
> +       return path;
>  }
>

Yes, sorry for the fat fingers :)

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Junio C Hamano]

nguyenhu@minatec.inpg.fr writes:

> What about this ?

What about that ;-)?

> char *mkpathdup(const char *fmt, ...)
> {
> 	char *path;
> 	struct strbuf sb = STRBUF_INIT;
> 	va_list args;
>
> 	va_start(args, fmt);
> 	strbuf_vaddf(&sb, fmt, args);
> 	va_end(args);
> 	path = sb.buf;
>
> 	strbuf_release(&sb);
> 	return xstrdup(cleanup_path(path));
> }

Modulo

	path = strbuf_detach(&sb, NULL);

that is more or less what I meant.

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[nguyenhu]

Junio C Hamano <gitster@pobox.com> a écrit :

>> char *mkpathdup(const char *fmt, ...)
>> {
>> 	char *path;
>> 	struct strbuf sb = STRBUF_INIT;
>> 	va_list args;
>>
>> 	va_start(args, fmt);
>> 	strbuf_vaddf(&sb, fmt, args);
>> 	va_end(args);
>> 	path = sb.buf;
>>
>> 	strbuf_release(&sb);
>> 	return xstrdup(cleanup_path(path));
>> }
>
> Modulo
>
> 	path = strbuf_detach(&sb, NULL);
>
> that is more or less what I meant.

So now the mkpathdup() function looks like:

char *mkpathdup(const char *fmt, ...)
{
	char *path;
	struct strbuf sb = STRBUF_INIT;
	va_list args;

	va_start(args, fmt);
	strbuf_vaddf(&sb, fmt, args);
	va_end(args);
	path = strbuf_detach(&sb, NULL);

	strbuf_release(&sb);
	return path;
}

This new variation of mkpathdup() function both fix the bug addressed
by commit 05bab3ea and avoid the use of bounded buffer.

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Junio C Hamano]

nguyenhu@minatec.inpg.fr writes:

>> Modulo
>>
>> 	path = strbuf_detach(&sb, NULL);
>>
>> that is more or less what I meant.
>
> So now the mkpathdup() function looks like:
>
> char *mkpathdup(const char *fmt, ...)
> {
> 	char *path;
> 	struct strbuf sb = STRBUF_INIT;
> 	va_list args;
>
> 	va_start(args, fmt);
> 	strbuf_vaddf(&sb, fmt, args);
> 	va_end(args);
> 	path = strbuf_detach(&sb, NULL);
>
> 	strbuf_release(&sb);
> 	return path;
> }
>
> This new variation of mkpathdup() function both fix the bug addressed
> by commit 05bab3ea and avoid the use of bounded buffer.

I didn't mean to suggest removing the call to clean-up-path
function.  What I meant was that strbuf_detach() is a way to take
the ownership of the buffer, so that you do not have to call
strbuf_release() on it.

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[nguyenhu]

Junio C Hamano <gitster@pobox.com> a écrit :

>> char *mkpathdup(const char *fmt, ...)
>> {
>> 	char *path;
>> 	struct strbuf sb = STRBUF_INIT;
>> 	va_list args;
>>
>> 	va_start(args, fmt);
>> 	strbuf_vaddf(&sb, fmt, args);
>> 	va_end(args);
>> 	path = strbuf_detach(&sb, NULL);
>>
>> 	strbuf_release(&sb);
>> 	return path;
>> }
>
> I didn't mean to suggest removing the call to clean-up-path
> function.  What I meant was that strbuf_detach() is a way to take
> the ownership of the buffer, so that you do not have to call
> strbuf_release() on it.

So with the call to clean-up-path function and without the call to
strbuf_release(), mkpathdup() function becomes :

char *mkpathdup(const char *fmt, ...)
{
	struct strbuf sb = STRBUF_INIT;
	va_list args;

	va_start(args, fmt);
	strbuf_vaddf(&sb, fmt, args);
	va_end(args);

	return cleanup_path(strbuf_detach(&sb, NULL));
}

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Erik Faye-Lund]

On Sun, Jun 10, 2012 at 3:48 PM,  <nguyenhu@minatec.inpg.fr> wrote:
>
> Junio C Hamano <gitster@pobox.com> a écrit :
>
>>> char *mkpathdup(const char *fmt, ...)
>>> {
>>>        char *path;
>>>        struct strbuf sb = STRBUF_INIT;
>>>        va_list args;
>>>
>>>        va_start(args, fmt);
>>>        strbuf_vaddf(&sb, fmt, args);
>>>        va_end(args);
>>>        path = strbuf_detach(&sb, NULL);
>>>
>>>        strbuf_release(&sb);
>>>        return path;
>>> }
>>
>>
>> I didn't mean to suggest removing the call to clean-up-path
>> function.  What I meant was that strbuf_detach() is a way to take
>> the ownership of the buffer, so that you do not have to call
>> strbuf_release() on it.
>
>
> So with the call to clean-up-path function and without the call to
> strbuf_release(), mkpathdup() function becomes :
>
>
> char *mkpathdup(const char *fmt, ...)
> {
>        struct strbuf sb = STRBUF_INIT;
>        va_list args;
>
>        va_start(args, fmt);
>        strbuf_vaddf(&sb, fmt, args);
>        va_end(args);
>
>        return cleanup_path(strbuf_detach(&sb, NULL));
>
> }

The awkward thing about doing this, is that the memory allocated by
the strbuf cannot be reclaimed if you go with this. A pointer that has
been adjusted (like cleanup_path can do) cannot be successfully fed to
free.

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[nguyenhu]

Erik Faye-Lund <kusmabite@gmail.com> a écrit :

>> char *mkpathdup(const char *fmt, ...)
>> {
>>        struct strbuf sb = STRBUF_INIT;
>>        va_list args;
>>
>>        va_start(args, fmt);
>>        strbuf_vaddf(&sb, fmt, args);
>>        va_end(args);
>>
>>        return cleanup_path(strbuf_detach(&sb, NULL));
>>
>> }
>
> The awkward thing about doing this, is that the memory allocated by
> the strbuf cannot be reclaimed if you go with this. A pointer that has
> been adjusted (like cleanup_path can do) cannot be successfully fed to
> free.

Do you mean that the previous version is preferable in keeping  
clean-up-path function ?

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Erik Faye-Lund]

On Sun, Jun 10, 2012 at 10:02 PM,  <nguyenhu@minatec.inpg.fr> wrote:
>
> Erik Faye-Lund <kusmabite@gmail.com> a écrit :
>
>>> char *mkpathdup(const char *fmt, ...)
>>> {
>>>        struct strbuf sb = STRBUF_INIT;
>>>        va_list args;
>>>
>>>        va_start(args, fmt);
>>>        strbuf_vaddf(&sb, fmt, args);
>>>        va_end(args);
>>>
>>>        return cleanup_path(strbuf_detach(&sb, NULL));
>>>
>>> }
>>
>>
>> The awkward thing about doing this, is that the memory allocated by
>> the strbuf cannot be reclaimed if you go with this. A pointer that has
>> been adjusted (like cleanup_path can do) cannot be successfully fed to
>> free.
>
>
> Do you mean that the previous version is preferable in keeping clean-up-path
> function ?
>

No, that wasn't my intention. Since this is only used for a few config
files, I don't think leaking the memory is a big deal. But it's
probably worth putting a comment in the code about it, to warn
potential future users.

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Junio C Hamano]

Erik Faye-Lund <kusmabite@gmail.com> writes:

> The awkward thing about doing this, is that the memory allocated by
> the strbuf cannot be reclaimed if you go with this. A pointer that has
> been adjusted (like cleanup_path can do) cannot be successfully fed to
> free.

Yeah, I wouldn't recommend doing that.  Either

	path = strbuf_detach(&sb, NULL);
	retval = xstrdup(cleanup_path(path));
        free(path);
        return retval;

or

	path = xstrdup(cleanup_path(sb.buf));
        strbuf_release(&sb);
        return path;
	
would be more sensible.

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[nguyenhu]

Junio C Hamano <gitster@pobox.com> a écrit :

> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> The awkward thing about doing this, is that the memory allocated by
>> the strbuf cannot be reclaimed if you go with this. A pointer that has
>> been adjusted (like cleanup_path can do) cannot be successfully fed to
>> free.
>
> Yeah, I wouldn't recommend doing that.  Either
>
> 	path = strbuf_detach(&sb, NULL);
> 	retval = xstrdup(cleanup_path(path));
>         free(path);
>         return retval;
>
> or
>
> 	path = xstrdup(cleanup_path(sb.buf));
>         strbuf_release(&sb);
>         return path;
>
> would be more sensible.

Erik, what do you prefer ? You can have the final answer.

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[nguyenhu]

nguyenhu@minatec.inpg.fr a écrit :

> Junio C Hamano <gitster@pobox.com> a écrit :
>
>> Erik Faye-Lund <kusmabite@gmail.com> writes:
>>
>>> The awkward thing about doing this, is that the memory allocated by
>>> the strbuf cannot be reclaimed if you go with this. A pointer that has
>>> been adjusted (like cleanup_path can do) cannot be successfully fed to
>>> free.
>>
>> Yeah, I wouldn't recommend doing that.  Either
>>
>> 	path = strbuf_detach(&sb, NULL);
>> 	retval = xstrdup(cleanup_path(path));
>>        free(path);
>>        return retval;
>>
>> or
>>
>> 	path = xstrdup(cleanup_path(sb.buf));
>>        strbuf_release(&sb);
>>        return path;
>>
>> would be more sensible.

In our next patch, mkpathdup() function will be

char *mkpathdup(const char *fmt, ...)
{
	char *path;
	struct strbuf sb = STRBUF_INIT;
	va_list args;

	va_start(args, fmt);
	strbuf_vaddf(&sb, fmt, args);
	va_end(args);
	path = xstrdup(cleanup_path(sb.buf));

	strbuf_release(&sb);
	return path;
}

which looks like our previous proposal with xstrdup()
and cleanup_path() functions at the right place.

Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Erik Faye-Lund]

On Mon, Jun 11, 2012 at 6:53 PM,  <nguyenhu@minatec.inpg.fr> wrote:
>
> Junio C Hamano <gitster@pobox.com> a écrit :
>
>> Erik Faye-Lund <kusmabite@gmail.com> writes:
>>
>>> The awkward thing about doing this, is that the memory allocated by
>>> the strbuf cannot be reclaimed if you go with this. A pointer that has
>>> been adjusted (like cleanup_path can do) cannot be successfully fed to
>>> free.
>>
>>
>> Yeah, I wouldn't recommend doing that.  Either
>>
>>        path = strbuf_detach(&sb, NULL);
>>        retval = xstrdup(cleanup_path(path));
>>        free(path);
>>        return retval;
>>
>> or
>>
>>        path = xstrdup(cleanup_path(sb.buf));
>>        strbuf_release(&sb);
>>        return path;
>>
>> would be more sensible.
>
>
> Erik, what do you prefer ? You can have the final answer.
>

I think the latter is the neater one, but I don't really care too much
either way :)

[PATCHv7 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Huynh Khoi Nguyen Nguyen]

Git will be able to read from $XDG_CONFIG_HOME/git/config, a new
configuration file following XDG specification. In the order of
reading, this file is between global configuration file and system
wide configuration file. Git currently does not write to this new
configuration file. If $XDG_CONFIG_HOME is either not set or empty,
$HOME/.config/git/config will be used.

Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/git-config.txt |   12 +++++--
 builtin/config.c             |   28 +++++++++++-----
 cache.h                      |    3 ++
 config.c                     |   23 ++++++++-----
 path.c                       |   41 ++++++++++++++++++++++++
 t/t1306-xdg-files.sh         |   70 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 156 insertions(+), 21 deletions(-)
 create mode 100755 t/t1306-xdg-files.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index d9463cb..7e344a2 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -99,8 +99,8 @@ OPTIONS
 	For writing options: write to global ~/.gitconfig file rather than
 	the repository .git/config.
 +
-For reading options: read only from global ~/.gitconfig rather than
-from all available files.
+For reading options: read only from global ~/.gitconfig and from
+$XDG_CONFIG_HOME/git/config rather than from all available files.
 +
 See also <<FILES>>.
 
@@ -194,7 +194,7 @@ See also <<FILES>>.
 FILES
 -----
 
-If not set explicitly with '--file', there are three files where
+If not set explicitly with '--file', there are four files where
 'git config' will search for configuration options:
 
 $GIT_DIR/config::
@@ -204,6 +204,12 @@ $GIT_DIR/config::
 	User-specific configuration file. Also called "global"
 	configuration file.
 
+$XDG_CONFIG_HOME/git/config::
+	Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
+	or empty, $HOME/.config/git/config will be used. Any single-valued
+	variable set in this file will be overwritten by whatever is in
+	~/.gitconfig.
+
 $(prefix)/etc/gitconfig::
 	System-wide configuration file.
 
diff --git a/builtin/config.c b/builtin/config.c
index 33c8820..da54fd1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -161,7 +161,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = -1;
-	char *global = NULL, *repo_config = NULL;
+	char *global = NULL, *xdg = NULL, *repo_config = NULL;
 	const char *system_wide = NULL, *local;
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 	config_fn_t fn;
@@ -169,12 +169,10 @@ static int get_value(const char *key_, const char *regex_)
 
 	local = given_config_file;
 	if (!local) {
-		const char *home = getenv("HOME");
 		local = repo_config = git_pathdup("config");
-		if (home)
-			global = xstrdup(mkpath("%s/.gitconfig", home));
 		if (git_config_system())
 			system_wide = git_etc_gitconfig();
+		home_config_paths(&global, &xdg, "config");
 	}
 
 	if (use_key_regexp) {
@@ -229,6 +227,8 @@ static int get_value(const char *key_, const char *regex_)
 
 	if (do_all && system_wide)
 		git_config_from_file(fn, system_wide, data);
+	if (do_all && xdg)
+		git_config_from_file(fn, xdg, data);
 	if (do_all && global)
 		git_config_from_file(fn, global, data);
 	if (do_all)
@@ -238,6 +238,8 @@ static int get_value(const char *key_, const char *regex_)
 		git_config_from_file(fn, local, data);
 	if (!do_all && !seen && global)
 		git_config_from_file(fn, global, data);
+	if (!do_all && !seen && xdg)
+		git_config_from_file(fn, xdg, data);
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
@@ -255,6 +257,7 @@ static int get_value(const char *key_, const char *regex_)
 free_strings:
 	free(repo_config);
 	free(global);
+	free(xdg);
 	return ret;
 }
 
@@ -379,13 +382,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 
 	if (use_global_config) {
-		char *home = getenv("HOME");
-		if (home) {
-			char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
+		char *user_config = NULL;
+		char *xdg_config = NULL;
+
+		home_config_paths(&user_config, &xdg_config, "config");
+
+		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&
+		    (actions == ACTION_LIST ||
+		     actions == ACTION_GET_COLOR ||
+		     actions == ACTION_GET_COLORBOOL))
+			given_config_file = xdg_config;
+		else if (user_config)
 			given_config_file = user_config;
-		} else {
+		else
 			die("$HOME not set");
-		}
 	}
 	else if (use_system_config)
 		given_config_file = git_etc_gitconfig();
diff --git a/cache.h b/cache.h
index 06413e1..fbba2d6 100644
--- a/cache.h
+++ b/cache.h
@@ -619,6 +619,8 @@ extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
 extern char *git_pathdup(const char *fmt, ...)
 	__attribute__((format (printf, 1, 2)));
+extern char *mkpathdup(const char *fmt, ...)
+	__attribute__((format (printf, 1, 2)));
 
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
@@ -708,6 +710,7 @@ int set_shared_perm(const char *path, int mode);
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
+extern void home_config_paths(char **global, char **xdg, char *file);
 extern char *expand_user_path(const char *path);
 const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
diff --git a/config.c b/config.c
index 71ef171..d28a499 100644
--- a/config.c
+++ b/config.c
@@ -929,7 +929,10 @@ int git_config_system(void)
 int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 {
 	int ret = 0, found = 0;
-	const char *home = NULL;
+	char *xdg_config = NULL;
+	char *user_config = NULL;
+
+	home_config_paths(&user_config, &xdg_config, "config");
 
 	if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {
 		ret += git_config_from_file(fn, git_etc_gitconfig(),
@@ -937,14 +940,14 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		found += 1;
 	}
 
-	home = getenv("HOME");
-	if (home) {
-		char buf[PATH_MAX];
-		char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
-		if (!access(user_config, R_OK)) {
-			ret += git_config_from_file(fn, user_config, data);
-			found += 1;
-		}
+	if (!access(xdg_config, R_OK)) {
+		ret += git_config_from_file(fn, xdg_config, data);
+		found += 1;
+	}
+
+	if (!access(user_config, R_OK)) {
+		ret += git_config_from_file(fn, user_config, data);
+		found += 1;
 	}
 
 	if (repo_config && !access(repo_config, R_OK)) {
@@ -963,6 +966,8 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		break;
 	}
 
+	free(xdg_config);
+	free(user_config);
 	return ret == 0 ? found : ret;
 }
 
diff --git a/path.c b/path.c
index 6f2aa69..66acd24 100644
--- a/path.c
+++ b/path.c
@@ -87,6 +87,21 @@ char *git_pathdup(const char *fmt, ...)
 	return xstrdup(path);
 }
 
+char *mkpathdup(const char *fmt, ...)
+{
+	char *path;
+	struct strbuf sb = STRBUF_INIT;
+	va_list args;
+
+	va_start(args, fmt);
+	strbuf_vaddf(&sb, fmt, args);
+	va_end(args);
+	path = xstrdup(cleanup_path(sb.buf));
+
+	strbuf_release(&sb);
+	return path;
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
@@ -122,6 +137,32 @@ char *git_path(const char *fmt, ...)
 	return cleanup_path(pathname);
 }
 
+void home_config_paths(char **global, char **xdg, char *file)
+{
+	char *xdg_home = getenv("XDG_CONFIG_HOME");
+	char *home = getenv("HOME");
+	char *to_free = NULL;
+
+	if (!home) {
+		if (global)
+			*global = NULL;
+	} else {
+		if (!xdg_home) {
+			to_free = mkpathdup("%s/.config", home);
+			xdg_home = to_free;
+		}
+		if (global)
+			*global = mkpathdup("%s/.gitconfig", home);
+	}
+
+	if (!xdg_home)
+		*xdg = NULL;
+	else
+		*xdg = mkpathdup("%s/git/%s", xdg_home, file);
+
+	free(to_free);
+}
+
 char *git_path_submodule(const char *path, const char *fmt, ...)
 {
 	char *pathname = get_pathname();
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
new file mode 100755
index 0000000..5b971d9
--- /dev/null
+++ b/t/t1306-xdg-files.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Valentin Duperray, Lucien Kong, Franck Jonas,
+#		     Thomas Nguy, Khoi Nguyen
+#		     Grenoble INP Ensimag
+#
+
+test_description='Compatibility with $XDG_CONFIG_HOME/git/ files'
+
+. ./test-lib.sh
+
+test_expect_success 'read config: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p .config/git &&
+	echo "[alias]" >.config/git/config &&
+	echo "	myalias = !echo in_config" >>.config/git/config &&
+	echo in_config >expected &&
+	git myalias >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read config: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[alias]" >.gitconfig &&
+	echo "	myalias = !echo in_gitconfig" >>.gitconfig &&
+	echo in_gitconfig >expected &&
+	git myalias >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo "[user]" >.config/git/config &&
+	echo "	name = read_config" >>.config/git/config &&
+	echo read_config >expected &&
+	git config --get user.name >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo read_gitconfig >expected &&
+	git config --get user.name >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo user.name=read_config >expected &&
+	git config --global --list >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo user.name=read_gitconfig >expected &&
+	git config --global --list >actual &&
+	test_cmp expected actual
+'
+
+
+test_done
-- 
1.7.8

[PATCHv7 2/4] Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore

[Huynh Khoi Nguyen Nguyen]

To use the feature of core.excludesfile, the user needs:
- to create such a file,
- and add configuration variable to point at it.
Instead, we can define a filename that is unlikely to already exist on
the system, and only use the presence of the file as a cue that the user
wants to use that feature.
We could pick any random filename that only user can create, which
means that it has to be under $HOME. Let's use
"${XDG_CONFIG_HOME:-$HOME/.config/git}/ignore". It is as good as any
random filename in $HOME/ and follows the XDG specification.

Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/config.txt    |    4 +++-
 Documentation/gitignore.txt |    4 +++-
 dir.c                       |    7 ++++++-
 t/t1306-xdg-files.sh        |   29 +++++++++++++++++++++++++++++
 4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 915cb5a..509bf25 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -483,7 +483,9 @@ core.excludesfile::
 	'.git/info/exclude', git looks into this file for patterns
 	of files which are not meant to be tracked.  "`~/`" is expanded
 	to the value of `$HOME` and "`~user/`" to the specified user's
-	home directory.  See linkgit:gitignore[5].
+	home directory. Its default value is $XDG_CONFIG_HOME/git/ignore.
+	If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/ignore
+	will be used. See linkgit:gitignore[5].
 
 core.askpass::
 	Some commands (e.g. svn and http interfaces) that interactively
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 2e7328b..a5c9b93 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -50,7 +50,9 @@ the repository but are specific to one user's workflow) should go into
 the `$GIT_DIR/info/exclude` file.  Patterns which a user wants git to
 ignore in all situations (e.g., backup or temporary files generated by
 the user's editor of choice) generally go into a file specified by
-`core.excludesfile` in the user's `~/.gitconfig`.
+`core.excludesfile` in the user's `~/.gitconfig`. Its default value is
+$XDG_CONFIG_HOME/git/ignore. If $XDG_CONFIG_HOME is either not set or empty,
+$HOME/.config/git/ignore will be used.
 
 The underlying git plumbing tools, such as
 'git ls-files' and 'git read-tree', read
diff --git a/dir.c b/dir.c
index ed1510f..8c6f47f 100644
--- a/dir.c
+++ b/dir.c
@@ -1234,12 +1234,17 @@ int remove_dir_recursively(struct strbuf *path, int flag)
 void setup_standard_excludes(struct dir_struct *dir)
 {
 	const char *path;
+	char *xdg_path;
 
 	dir->exclude_per_dir = ".gitignore";
 	path = git_path("info/exclude");
+	if (!excludes_file) {
+		home_config_paths(NULL, &xdg_path, "ignore");
+		excludes_file = xdg_path;
+	}
 	if (!access(path, R_OK))
 		add_excludes_from_file(dir, path);
-	if (excludes_file && !access(excludes_file, R_OK))
+	if (!access(excludes_file, R_OK))
 		add_excludes_from_file(dir, excludes_file);
 }
 
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 5b971d9..05103f5 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -67,4 +67,33 @@ test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists'
 '
 
 
+test_expect_success 'Setup' '
+	git init git &&
+	cd git &&
+	echo foo >to_be_excluded
+'
+
+
+test_expect_success 'Exclusion of a file in the XDG ignore file' '
+	mkdir -p "$HOME"/.config/git/ &&
+	echo to_be_excluded >"$HOME"/.config/git/ignore &&
+	test_must_fail git add to_be_excluded
+'
+
+
+test_expect_success 'Exclusion in both XDG and local ignore files' '
+	echo to_be_excluded >.gitignore &&
+	test_must_fail git add to_be_excluded
+'
+
+
+test_expect_success 'Exclusion in a non-XDG global ignore file' '
+	rm .gitignore &&
+	echo >"$HOME"/.config/git/ignore &&
+	echo to_be_excluded >"$HOME"/my_gitignore &&
+	git config core.excludesfile "$HOME"/my_gitignore &&
+	test_must_fail git add to_be_excluded
+'
+
+
 test_done
-- 
1.7.8

[PATCHv7 3/4] Let core.attributesfile default to $XDG_CONFIG_HOME/git/attributes

[Huynh Khoi Nguyen Nguyen]

To use the feature of core.attributesfile, the user needs:
- to create such a file,
- and add configuration variable to point at it.
Instead, we can define a filename that is unlikely to already exist on
the system, and only use the presence of the file as a cue that the user
wants to use that feature.
We could pick any random filename that only user can create, which
means that it has to be under $HOME. Let's use
"${XDG_CONFIG_HOME:-$HOME/.config/git}/attributes". It is as good as any
random filename in $HOME/ and follows the XDG specification.

Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/config.txt        |    4 +++-
 Documentation/gitattributes.txt |    2 ++
 attr.c                          |   17 ++++++++++-------
 t/t1306-xdg-files.sh            |   29 +++++++++++++++++++++++++++++
 4 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 509bf25..f691d8e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -500,7 +500,9 @@ core.attributesfile::
 	In addition to '.gitattributes' (per-directory) and
 	'.git/info/attributes', git looks into this file for attributes
 	(see linkgit:gitattributes[5]). Path expansions are made the same
-	way as for `core.excludesfile`.
+	way as for `core.excludesfile`. Its default value is
+	$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
+	set or empty, $HOME/.config/git/attributes will be used.
 
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 80120ea..484c614 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -75,6 +75,8 @@ repositories (i.e., attributes of interest to all users) should go into
 `.gitattributes` files. Attributes that should affect all repositories
 for a single user should be placed in a file specified by the
 `core.attributesfile` configuration option (see linkgit:git-config[1]).
+Its default value is $XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME
+is either not set or empty, $HOME/.config/git/attributes will be used.
 Attributes for all users on a system should be placed in the
 `$(prefix)/etc/gitattributes` file.
 
diff --git a/attr.c b/attr.c
index 303751f..aef93d8 100644
--- a/attr.c
+++ b/attr.c
@@ -497,6 +497,7 @@ static int git_attr_system(void)
 static void bootstrap_attr_stack(void)
 {
 	struct attr_stack *elem;
+	char *xdg_attributes_file;
 
 	if (attr_stack)
 		return;
@@ -515,13 +516,15 @@ static void bootstrap_attr_stack(void)
 		}
 	}
 
-	if (git_attributes_file) {
-		elem = read_attr_from_file(git_attributes_file, 1);
-		if (elem) {
-			elem->origin = NULL;
-			elem->prev = attr_stack;
-			attr_stack = elem;
-		}
+	if (!git_attributes_file) {
+		home_config_paths(NULL, &xdg_attributes_file, "attributes");
+		git_attributes_file = xdg_attributes_file;
+	}
+	elem = read_attr_from_file(git_attributes_file, 1);
+	if (elem) {
+		elem->origin = NULL;
+		elem->prev = attr_stack;
+		attr_stack = elem;
 	}
 
 	if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 05103f5..e8cd78a 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -96,4 +96,33 @@ test_expect_success 'Exclusion in a non-XDG global ignore file' '
 '
 
 
+test_expect_success 'Checking attributes in the XDG attributes file' '
+	echo foo >f &&
+	git check-attr -a f >actual &&
+	test_line_count -eq 0 actual &&
+	echo "f attr_f" >"$HOME"/.config/git/attributes &&
+	echo "f: attr_f: set" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'Checking attributes in both XDG and local attributes files' '
+	echo "f -attr_f" >.gitattributes &&
+	echo "f: attr_f: unset" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'Checking attributes in a non-XDG global attributes file' '
+	test_might_fail rm .gitattributes &&
+	echo "f attr_f=test" >"$HOME"/my_gitattributes &&
+	git config core.attributesfile "$HOME"/my_gitattributes &&
+	echo "f: attr_f: test" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+
 test_done
-- 
1.7.8

[PATCHv7 4/4] Write to $XDG_CONFIG_HOME/git/config file

[Huynh Khoi Nguyen Nguyen]

Git will be able to write to $XDG_CONFIG_HOME/git/config, a new
configuration file following XDG specification, if:
- this file already exists, and
- $HOME/.gitconfig file doesn't, and
- global option is used.
Otherwise Git writes to $HOME/.gitconfig with global option, as usual.
If you don't create $XDG_CONFIG_HOME/git/config, there is absolutely
no change. Users can use this new file only if they want.
If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/config
will be used.
Advice for users who often come back to an old version of Git:
you shouldn't create this file.

Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
---
 Documentation/git-config.txt |    3 ++-
 builtin/config.c             |    5 +----
 t/t1306-xdg-files.sh         |   30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 7e344a2..a2b4786 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -97,7 +97,8 @@ OPTIONS
 
 --global::
 	For writing options: write to global ~/.gitconfig file rather than
-	the repository .git/config.
+	the repository .git/config, write to $XDG_CONFIG_HOME/git/config file
+	if this file exists and ~/.gitconfig file doesn't.
 +
 For reading options: read only from global ~/.gitconfig and from
 $XDG_CONFIG_HOME/git/config rather than from all available files.
diff --git a/builtin/config.c b/builtin/config.c
index da54fd1..e8e1c0a 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -387,10 +387,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 		home_config_paths(&user_config, &xdg_config, "config");
 
-		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&
-		    (actions == ACTION_LIST ||
-		     actions == ACTION_GET_COLOR ||
-		     actions == ACTION_GET_COLORBOOL))
+		if (access(user_config, R_OK) && !access(xdg_config, R_OK))
 			given_config_file = xdg_config;
 		else if (user_config)
 			given_config_file = user_config;
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index e8cd78a..3c75c3f 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -125,4 +125,34 @@ test_expect_success 'Checking attributes in a non-XDG global attributes file' '
 '
 
 
+test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p "$HOME"/.config/git &&
+	>"$HOME"/.config/git/config &&
+	test_might_fail rm "$HOME"/.gitconfig &&
+	git config --global user.name "write_config" &&
+	echo "[user]" >expected &&
+	echo "	name = write_config" >>expected &&
+	test_cmp expected "$HOME"/.config/git/config
+'
+
+
+test_expect_success 'write: xdg file exists and ~/.gitconfig exists' '
+	>"$HOME"/.gitconfig &&
+	git config --global user.name "write_gitconfig" &&
+	echo "[user]" >expected &&
+	echo "	name = write_gitconfig" >>expected &&
+	test_cmp expected "$HOME"/.gitconfig
+'
+
+
+test_expect_success 'write: ~/.config/git/ exists and config file doesn'\''t' '
+	test_might_fail rm "$HOME"/.gitconfig &&
+	test_might_fail rm "$HOME"/.config/git/config &&
+	git config --global user.name "write_gitconfig" &&
+	echo "[user]" >expected &&
+	echo "	name = write_gitconfig" >>expected &&
+	test_cmp expected "$HOME"/.gitconfig
+'
+
+
 test_done
-- 
1.7.8

Re: [PATCHv7 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Ramsay Jones]

Huynh Khoi Nguyen Nguyen wrote:
> Git will be able to read from $XDG_CONFIG_HOME/git/config, a new
> configuration file following XDG specification. In the order of
> reading, this file is between global configuration file and system
> wide configuration file. Git currently does not write to this new
> configuration file. If $XDG_CONFIG_HOME is either not set or empty,
> $HOME/.config/git/config will be used.
> 
> Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
> Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
> Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
> Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
> Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> ---
>  Documentation/git-config.txt |   12 +++++--
>  builtin/config.c             |   28 +++++++++++-----
>  cache.h                      |    3 ++
>  config.c                     |   23 ++++++++-----
>  path.c                       |   41 ++++++++++++++++++++++++
>  t/t1306-xdg-files.sh         |   70 ++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 156 insertions(+), 21 deletions(-)
>  create mode 100755 t/t1306-xdg-files.sh

[...]

> diff --git a/path.c b/path.c
> index 6f2aa69..66acd24 100644
> --- a/path.c
> +++ b/path.c
> @@ -87,6 +87,21 @@ char *git_pathdup(const char *fmt, ...)
>  	return xstrdup(path);
>  }
>  
> +char *mkpathdup(const char *fmt, ...)
> +{
> +	char *path;
> +	struct strbuf sb = STRBUF_INIT;
> +	va_list args;
> +
> +	va_start(args, fmt);
> +	strbuf_vaddf(&sb, fmt, args);
> +	va_end(args);
> +	path = xstrdup(cleanup_path(sb.buf));
> +
> +	strbuf_release(&sb);
> +	return path;
> +}
> +

As expected, this version avoids re-introducing the bug on Cygwin.

I tested the series on Cygwin, MinGW and Linux and it passes it's
own tests (t1306-xdg-files.sh) on all platforms. (Well, on MinGW
I had to run it thus:

    $ GIT_TEST_CMP='diff -ub' ./t1306-xdg-files.sh

otherwise the first two tests fail because of CRLF vs LF issues).

Thanks!

ATB,
Ramsay Jones

Re: [PATCHv7 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Matthieu Moy]

Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
writes:

> Git will be able to read from $XDG_CONFIG_HOME/git/config, a new
> configuration file following XDG specification. In the order of
> reading, this file is between global configuration file and system
> wide configuration file. Git currently does not write to this new
> configuration file. If $XDG_CONFIG_HOME is either not set or empty,
> $HOME/.config/git/config will be used.

What happened to this serie? I think all comments have been addressed,
but I don't see it in pu... Should I resend and Cc Junio ?

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCHv7 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
> writes:
>
>> Git will be able to read from $XDG_CONFIG_HOME/git/config, a new
>> configuration file following XDG specification. In the order of
>> reading, this file is between global configuration file and system
>> wide configuration file. Git currently does not write to this new
>> configuration file. If $XDG_CONFIG_HOME is either not set or empty,
>> $HOME/.config/git/config will be used.
>
> What happened to this serie? I think all comments have been addressed,
> but I don't see it in pu... Should I resend and Cc Junio ?

If it is ready for testing, please do.  I wasn't interested and was
not paying attention during the feature freeze.

[PATCH 0/4 v8] Git configuration directory

[Matthieu Moy]

This is a resend of a serie already discussed here:

  http://thread.gmane.org/gmane.comp.version-control.git/198837

I took the liberty of rewording the commit messages, and change a few
words in the documentation (changed some "will be used" to "is used
instead", which sounded more consistant with the context).

A reminder of the context and objectives of the serie:

Git currently stores its configuration file in ~/.gitconfig, which is
nice and customary on Unix, as long as one has only one configuration
file.

But a typical user may want to store not only the config file, but
also the files pointed to by core.excludesfile and
core.attributesfile, which currently have no default values. To store
several configuration files, it makes sense to have a configuration
_directory_ instead of a set of configuration files, all right into
$HOME. Calling this configuration directory ~/.git or ~/.gitconfig is
not an option, since these paths already have another meaning. Using
the XDG specification (in short: ~/.config/git) doesn't have this
drawback, and allows the user to store his configuration files right
next to files from other applications following the XDG standard.

The first 3 patches add read support for this "configuration
directory", and the last one gives opt-in write support, allowing
users to make this "configuration directory" their way of life, and
never hear again about ~/.gitconfig if (and only if) they do not want to.

None of the patches change the behavior for people who do not create
the new configuration files.

Huynh Khoi Nguyen Nguyen (4):
  config: read (but not write) from $XDG_CONFIG_HOME/git/config file
  Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore
  Let core.attributesfile default to $XDG_CONFIG_HOME/git/ignore
  config: write to $XDG_CONFIG_HOME/git/config file if appropriate

 Documentation/config.txt        |   8 +-
 Documentation/git-config.txt    |  15 +++-
 Documentation/gitattributes.txt |   2 +
 Documentation/gitignore.txt     |   4 +-
 attr.c                          |  17 +++--
 builtin/config.c                |  25 ++++---
 cache.h                         |   3 +
 config.c                        |  23 +++---
 dir.c                           |   7 +-
 path.c                          |  41 +++++++++++
 t/t1306-xdg-files.sh            | 158 ++++++++++++++++++++++++++++++++++++++++
 11 files changed, 270 insertions(+), 33 deletions(-)
 create mode 100755 t/t1306-xdg-files.sh

-- 
1.7.11.rc3.235.gd0d1d08

[PATCH 1/4 v8] config: read (but not write) from $XDG_CONFIG_HOME/git/config file

[Matthieu Moy]

From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>

Git will be able to read from $XDG_CONFIG_HOME/git/config, a new
configuration file following XDG specification. In the order of
reading, this file is between global configuration file and system
wide configuration file. Git currently does not write to this new
configuration file. If $XDG_CONFIG_HOME is either not set or empty,
$HOME/.config/git/config will be used. If the new file does not exist,
the behavior is unchanged.

Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-config.txt | 12 ++++++--
 builtin/config.c             | 28 ++++++++++++------
 cache.h                      |  3 ++
 config.c                     | 23 +++++++++------
 path.c                       | 41 ++++++++++++++++++++++++++
 t/t1306-xdg-files.sh         | 70 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 156 insertions(+), 21 deletions(-)
 create mode 100755 t/t1306-xdg-files.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index d9463cb..7e344a2 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -99,8 +99,8 @@ OPTIONS
 	For writing options: write to global ~/.gitconfig file rather than
 	the repository .git/config.
 +
-For reading options: read only from global ~/.gitconfig rather than
-from all available files.
+For reading options: read only from global ~/.gitconfig and from
+$XDG_CONFIG_HOME/git/config rather than from all available files.
 +
 See also <<FILES>>.
 
@@ -194,7 +194,7 @@ See also <<FILES>>.
 FILES
 -----
 
-If not set explicitly with '--file', there are three files where
+If not set explicitly with '--file', there are four files where
 'git config' will search for configuration options:
 
 $GIT_DIR/config::
@@ -204,6 +204,12 @@ $GIT_DIR/config::
 	User-specific configuration file. Also called "global"
 	configuration file.
 
+$XDG_CONFIG_HOME/git/config::
+	Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
+	or empty, $HOME/.config/git/config will be used. Any single-valued
+	variable set in this file will be overwritten by whatever is in
+	~/.gitconfig.
+
 $(prefix)/etc/gitconfig::
 	System-wide configuration file.
 
diff --git a/builtin/config.c b/builtin/config.c
index 33c8820..da54fd1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -161,7 +161,7 @@ static int show_config(const char *key_, const char *value_, void *cb)
 static int get_value(const char *key_, const char *regex_)
 {
 	int ret = -1;
-	char *global = NULL, *repo_config = NULL;
+	char *global = NULL, *xdg = NULL, *repo_config = NULL;
 	const char *system_wide = NULL, *local;
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 	config_fn_t fn;
@@ -169,12 +169,10 @@ static int get_value(const char *key_, const char *regex_)
 
 	local = given_config_file;
 	if (!local) {
-		const char *home = getenv("HOME");
 		local = repo_config = git_pathdup("config");
-		if (home)
-			global = xstrdup(mkpath("%s/.gitconfig", home));
 		if (git_config_system())
 			system_wide = git_etc_gitconfig();
+		home_config_paths(&global, &xdg, "config");
 	}
 
 	if (use_key_regexp) {
@@ -229,6 +227,8 @@ static int get_value(const char *key_, const char *regex_)
 
 	if (do_all && system_wide)
 		git_config_from_file(fn, system_wide, data);
+	if (do_all && xdg)
+		git_config_from_file(fn, xdg, data);
 	if (do_all && global)
 		git_config_from_file(fn, global, data);
 	if (do_all)
@@ -238,6 +238,8 @@ static int get_value(const char *key_, const char *regex_)
 		git_config_from_file(fn, local, data);
 	if (!do_all && !seen && global)
 		git_config_from_file(fn, global, data);
+	if (!do_all && !seen && xdg)
+		git_config_from_file(fn, xdg, data);
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
@@ -255,6 +257,7 @@ static int get_value(const char *key_, const char *regex_)
 free_strings:
 	free(repo_config);
 	free(global);
+	free(xdg);
 	return ret;
 }
 
@@ -379,13 +382,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 
 	if (use_global_config) {
-		char *home = getenv("HOME");
-		if (home) {
-			char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
+		char *user_config = NULL;
+		char *xdg_config = NULL;
+
+		home_config_paths(&user_config, &xdg_config, "config");
+
+		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&
+		    (actions == ACTION_LIST ||
+		     actions == ACTION_GET_COLOR ||
+		     actions == ACTION_GET_COLORBOOL))
+			given_config_file = xdg_config;
+		else if (user_config)
 			given_config_file = user_config;
-		} else {
+		else
 			die("$HOME not set");
-		}
 	}
 	else if (use_system_config)
 		given_config_file = git_etc_gitconfig();
diff --git a/cache.h b/cache.h
index 06413e1..fbba2d6 100644
--- a/cache.h
+++ b/cache.h
@@ -619,6 +619,8 @@ extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
 extern char *git_pathdup(const char *fmt, ...)
 	__attribute__((format (printf, 1, 2)));
+extern char *mkpathdup(const char *fmt, ...)
+	__attribute__((format (printf, 1, 2)));
 
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
@@ -708,6 +710,7 @@ int set_shared_perm(const char *path, int mode);
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
+extern void home_config_paths(char **global, char **xdg, char *file);
 extern char *expand_user_path(const char *path);
 const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
diff --git a/config.c b/config.c
index 71ef171..d28a499 100644
--- a/config.c
+++ b/config.c
@@ -929,7 +929,10 @@ int git_config_system(void)
 int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 {
 	int ret = 0, found = 0;
-	const char *home = NULL;
+	char *xdg_config = NULL;
+	char *user_config = NULL;
+
+	home_config_paths(&user_config, &xdg_config, "config");
 
 	if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {
 		ret += git_config_from_file(fn, git_etc_gitconfig(),
@@ -937,14 +940,14 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		found += 1;
 	}
 
-	home = getenv("HOME");
-	if (home) {
-		char buf[PATH_MAX];
-		char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
-		if (!access(user_config, R_OK)) {
-			ret += git_config_from_file(fn, user_config, data);
-			found += 1;
-		}
+	if (!access(xdg_config, R_OK)) {
+		ret += git_config_from_file(fn, xdg_config, data);
+		found += 1;
+	}
+
+	if (!access(user_config, R_OK)) {
+		ret += git_config_from_file(fn, user_config, data);
+		found += 1;
 	}
 
 	if (repo_config && !access(repo_config, R_OK)) {
@@ -963,6 +966,8 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		break;
 	}
 
+	free(xdg_config);
+	free(user_config);
 	return ret == 0 ? found : ret;
 }
 
diff --git a/path.c b/path.c
index 6f2aa69..66acd24 100644
--- a/path.c
+++ b/path.c
@@ -87,6 +87,21 @@ char *git_pathdup(const char *fmt, ...)
 	return xstrdup(path);
 }
 
+char *mkpathdup(const char *fmt, ...)
+{
+	char *path;
+	struct strbuf sb = STRBUF_INIT;
+	va_list args;
+
+	va_start(args, fmt);
+	strbuf_vaddf(&sb, fmt, args);
+	va_end(args);
+	path = xstrdup(cleanup_path(sb.buf));
+
+	strbuf_release(&sb);
+	return path;
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
@@ -122,6 +137,32 @@ char *git_path(const char *fmt, ...)
 	return cleanup_path(pathname);
 }
 
+void home_config_paths(char **global, char **xdg, char *file)
+{
+	char *xdg_home = getenv("XDG_CONFIG_HOME");
+	char *home = getenv("HOME");
+	char *to_free = NULL;
+
+	if (!home) {
+		if (global)
+			*global = NULL;
+	} else {
+		if (!xdg_home) {
+			to_free = mkpathdup("%s/.config", home);
+			xdg_home = to_free;
+		}
+		if (global)
+			*global = mkpathdup("%s/.gitconfig", home);
+	}
+
+	if (!xdg_home)
+		*xdg = NULL;
+	else
+		*xdg = mkpathdup("%s/git/%s", xdg_home, file);
+
+	free(to_free);
+}
+
 char *git_path_submodule(const char *path, const char *fmt, ...)
 {
 	char *pathname = get_pathname();
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
new file mode 100755
index 0000000..5b971d9
--- /dev/null
+++ b/t/t1306-xdg-files.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Valentin Duperray, Lucien Kong, Franck Jonas,
+#		     Thomas Nguy, Khoi Nguyen
+#		     Grenoble INP Ensimag
+#
+
+test_description='Compatibility with $XDG_CONFIG_HOME/git/ files'
+
+. ./test-lib.sh
+
+test_expect_success 'read config: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p .config/git &&
+	echo "[alias]" >.config/git/config &&
+	echo "	myalias = !echo in_config" >>.config/git/config &&
+	echo in_config >expected &&
+	git myalias >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read config: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[alias]" >.gitconfig &&
+	echo "	myalias = !echo in_gitconfig" >>.gitconfig &&
+	echo in_gitconfig >expected &&
+	git myalias >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo "[user]" >.config/git/config &&
+	echo "	name = read_config" >>.config/git/config &&
+	echo read_config >expected &&
+	git config --get user.name >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo read_gitconfig >expected &&
+	git config --get user.name >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig doesn'\''t' '
+	rm .gitconfig &&
+	echo user.name=read_config >expected &&
+	git config --global --list >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists' '
+	>.gitconfig &&
+	echo "[user]" >.gitconfig &&
+	echo "	name = read_gitconfig" >>.gitconfig &&
+	echo user.name=read_gitconfig >expected &&
+	git config --global --list >actual &&
+	test_cmp expected actual
+'
+
+
+test_done
-- 
1.7.11.rc3.235.gd0d1d08

Re: [PATCH 1/4 v8] config: read (but not write) from $XDG_CONFIG_HOME/git/config file

[Thomas Rast]

Hi all,

This patch causes valgrind warnings in t1300.81 (get --path copes with
unset $HOME) about passing NULL to access():

==25286== Syscall param access(pathname) points to unaddressable byte(s)
==25286==    at 0x56E2227: access (in /lib64/libc-2.14.1.so)
==25286==    by 0x48CA42: git_config_early (config.c:948)
==25286==    by 0x4F0C20: check_repository_format_gently (setup.c:369)
==25286==    by 0x4F1A52: setup_git_directory_gently_1 (setup.c:531)
==25286==    by 0x4F24ED: setup_git_directory_gently (setup.c:725)
==25286==    by 0x405D8C: run_builtin (git.c:287)
==25286==    by 0x40548E: main (git.c:467)
==25286==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

AFAICT it is this bit of code:

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> +void home_config_paths(char **global, char **xdg, char *file)
> +{
> +	char *xdg_home = getenv("XDG_CONFIG_HOME");
> +	char *home = getenv("HOME");
> +	char *to_free = NULL;
> +
> +	if (!home) {
> +		if (global)
> +			*global = NULL;
[...]
>  int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  {
>  	int ret = 0, found = 0;
> -	const char *home = NULL;
> +	char *xdg_config = NULL;
> +	char *user_config = NULL;
> +
> +	home_config_paths(&user_config, &xdg_config, "config");
>  
>  	if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {
>  		ret += git_config_from_file(fn, git_etc_gitconfig(),
> @@ -937,14 +940,14 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  		found += 1;
>  	}
>  
> -	home = getenv("HOME");
> -	if (home) {
> -		char buf[PATH_MAX];
> -		char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
> -		if (!access(user_config, R_OK)) {
> -			ret += git_config_from_file(fn, user_config, data);
> -			found += 1;
> -		}
> +	if (!access(xdg_config, R_OK)) {
> +		ret += git_config_from_file(fn, xdg_config, data);
> +		found += 1;
> +	}
> +
> +	if (!access(user_config, R_OK)) {
> +		ret += git_config_from_file(fn, user_config, data);
> +		found += 1;
>  	}

That is, while the old code was careful about home==NULL, the new code
checks this only in home_config_paths(); after that, it does no further
NULL check.

There's a similar instance in your changes to cmd_config(), found by
running 'unset HOME; valgrind git config --global --get foo.bar':

==27841== Syscall param access(pathname) points to unaddressable byte(s)
==27841==    at 0x56E2227: access (in /lib64/libc-2.14.1.so)
==27841==    by 0x425280: cmd_config (config.c:390)
==27841==    by 0x405C34: run_builtin (git.c:306)
==27841==    by 0x40548E: main (git.c:467)
==27841==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

around here:

> @@ -379,13 +382,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
[...]
> +		char *user_config = NULL;
> +		char *xdg_config = NULL;
> +
> +		home_config_paths(&user_config, &xdg_config, "config");
> +
> +		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&

Can you fix this?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[PATCH] config: fix several access(NULL) calls

[Matthieu Moy]

When $HOME is unset, home_config_paths fails and returns NULL pointers
for user_config and xdg_config. Valgrind complains with Syscall param
access(pathname) points to unaddressable byte(s).

Don't call blindly access() on these variables, but test them for
NULL-ness before.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
> This patch causes valgrind warnings in t1300.81 (get --path copes with
> unset $HOME) about passing NULL to access():

Indeed. The following patch should fix it.

 builtin/config.c | 3 ++-
 config.c         | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index e8e1c0a..67945b2 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -387,7 +387,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 		home_config_paths(&user_config, &xdg_config, "config");
 
-		if (access(user_config, R_OK) && !access(xdg_config, R_OK))
+		if (user_config && access(user_config, R_OK) &&
+		    xdg_config && !access(xdg_config, R_OK))
 			given_config_file = xdg_config;
 		else if (user_config)
 			given_config_file = user_config;
diff --git a/config.c b/config.c
index d28a499..6b97503 100644
--- a/config.c
+++ b/config.c
@@ -940,12 +940,12 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		found += 1;
 	}
 
-	if (!access(xdg_config, R_OK)) {
+	if (xdg_config && !access(xdg_config, R_OK)) {
 		ret += git_config_from_file(fn, xdg_config, data);
 		found += 1;
 	}
 
-	if (!access(user_config, R_OK)) {
+	if (user_config && !access(user_config, R_OK)) {
 		ret += git_config_from_file(fn, user_config, data);
 		found += 1;
 	}
-- 
1.7.11.1.29.g0e8593d.dirty

Re: [PATCH] config: fix several access(NULL) calls

[Thomas Rast]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> When $HOME is unset, home_config_paths fails and returns NULL pointers
> for user_config and xdg_config. Valgrind complains with Syscall param
> access(pathname) points to unaddressable byte(s).
>
> Don't call blindly access() on these variables, but test them for
> NULL-ness before.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>> This patch causes valgrind warnings in t1300.81 (get --path copes with
>> unset $HOME) about passing NULL to access():
>
> Indeed. The following patch should fix it.

Thanks!

Tested-by: Thomas Rast <trast@student.ethz.ch>

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] config: fix several access(NULL) calls

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> When $HOME is unset, home_config_paths fails and returns NULL pointers
> for user_config and xdg_config. Valgrind complains with Syscall param
> access(pathname) points to unaddressable byte(s).
>
> Don't call blindly access() on these variables, but test them for
> NULL-ness before.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>> This patch causes valgrind warnings in t1300.81 (get --path copes with
>> unset $HOME) about passing NULL to access():
>
> Indeed. The following patch should fix it.
>
>  builtin/config.c | 3 ++-
>  config.c         | 4 ++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index e8e1c0a..67945b2 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -387,7 +387,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>  
>  		home_config_paths(&user_config, &xdg_config, "config");
>  
> -		if (access(user_config, R_OK) && !access(xdg_config, R_OK))
> +		if (user_config && access(user_config, R_OK) &&
> +		    xdg_config && !access(xdg_config, R_OK))
>  			given_config_file = xdg_config;

Shouldn't we be using xdg_config, if user_config is NULL and
xdg_config is defined and accessible?

In other words, isn't the objective of this "fix" is to replace any
call to access(PATH, PERM) whose PATH can potentially be NULL with

	(PATH ? access(PATH, PERM) : -1)

because we want to pretend access(PATH, PERM) returned an error,
saying "The variable PATH holds a path to the file that is not
accessible", when PATH is NULL?

And that is equivalent to

	(!PATH || access(PATH, PERM))

in the context of boolean.  The original

	if (access(user_config, R_OK) && !access(xdg_config, R_OK))

becomes 

	if ((!user_config || access(user_config, R_OK)) &&
	    !(!xdg_config || access(xdg_config, R_OK)))

and the latter half of it is the same as

	    (xdg_config && !access(xdg_config, R_OK))

but the former half is not. Shouldn't it be

	if ((!user_config || access(user_config, R_OK)) &&
	    (xdg_config && !access(xdg_config, R_OK)))

Confused.

> diff --git a/config.c b/config.c
> index d28a499..6b97503 100644
> --- a/config.c
> +++ b/config.c
> @@ -940,12 +940,12 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  		found += 1;
>  	}
>  
> -	if (!access(xdg_config, R_OK)) {
> +	if (xdg_config && !access(xdg_config, R_OK)) {
>  		ret += git_config_from_file(fn, xdg_config, data);
>  		found += 1;
>  	}
>  
> -	if (!access(user_config, R_OK)) {
> +	if (user_config && !access(user_config, R_OK)) {
>  		ret += git_config_from_file(fn, user_config, data);
>  		found += 1;
>  	}

Re: [PATCH] config: fix several access(NULL) calls

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> When $HOME is unset, home_config_paths fails and returns NULL pointers
>> for user_config and xdg_config. Valgrind complains with Syscall param
>> access(pathname) points to unaddressable byte(s).
>>
>> Don't call blindly access() on these variables, but test them for
>> NULL-ness before.
>>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>>> This patch causes valgrind warnings in t1300.81 (get --path copes with
>>> unset $HOME) about passing NULL to access():
>>
>> Indeed. The following patch should fix it.
>>
>>  builtin/config.c | 3 ++-
>>  config.c         | 4 ++--
>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/builtin/config.c b/builtin/config.c
>> index e8e1c0a..67945b2 100644
>> --- a/builtin/config.c
>> +++ b/builtin/config.c
>> @@ -387,7 +387,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>>  
>>  		home_config_paths(&user_config, &xdg_config, "config");
>>  
>> -		if (access(user_config, R_OK) && !access(xdg_config, R_OK))
>> +		if (user_config && access(user_config, R_OK) &&
>> +		    xdg_config && !access(xdg_config, R_OK))
>>  			given_config_file = xdg_config;
>
> Shouldn't we be using xdg_config, if user_config is NULL and
> xdg_config is defined and accessible?

I don't think so. If user_config is NULL, it means something went wrong,
because $HOME is unset. In this case, I'd rather die than using some
other configuration file silently (which would be possible if
$XDG_CONFIG_HOME is set), and this is what the code does:

		if (user_config && access(user_config, R_OK) &&
		    xdg_config && !access(xdg_config, R_OK))
			given_config_file = xdg_config;
		else if (user_config)
			given_config_file = user_config;
		else
			die("$HOME not set");

Perhaps it could actually be made even clearer with

		if (!user_config)
			die("$HOME not set");
		else if (access(user_config, R_OK) &&
			 xdg_config && !access(xdg_config, R_OK))
			given_config_file = xdg_config;
		else
			given_config_file = user_config;

That said, I don't care very strongly about it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] config: fix several access(NULL) calls

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

>>> +		if (user_config && access(user_config, R_OK) &&
>>> +		    xdg_config && !access(xdg_config, R_OK))
>>>  			given_config_file = xdg_config;
>>
>> Shouldn't we be using xdg_config, if user_config is NULL and
>> xdg_config is defined and accessible?
>
> I don't think so. If user_config is NULL, it means something went wrong,
> because $HOME is unset. In this case, I'd rather die than using some
> other configuration file silently (which would be possible if
> $XDG_CONFIG_HOME is set), and this is what the code does:
>
> 		if (user_config && access(user_config, R_OK) &&
> 		    xdg_config && !access(xdg_config, R_OK))
> 			given_config_file = xdg_config;
> 		else if (user_config)
> 			given_config_file = user_config;
> 		else
> 			die("$HOME not set");
>
> Perhaps it could actually be made even clearer with
>
> 		if (!user_config)
> 			die("$HOME not set");
> 		else if (access(user_config, R_OK) &&
> 			 xdg_config && !access(xdg_config, R_OK))
> 			given_config_file = xdg_config;
> 		else
> 			given_config_file = user_config;

At least the code needs to break lines at different point and a comment.

	if (user_config &&
		access(user_config, R_OK) &&
                (xdg_config && !access(xdg_config, R_OK)))
                /*
                 * Even if we have usable XDG stuff, we want to
		 * error out on missing HOME!!!
                 */
		use xdg config;
	else if (user_config)
		use user config;
	else
		unusable situation;

But is it really true that we want to error out on missing HOME if
we have usable XDG stuff?

Re: [PATCH] config: fix several access(NULL) calls

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> But is it really true that we want to error out on missing HOME if
> we have usable XDG stuff?

Anyone else have an opinion on this?

In short, the question is whether

  export XDG_CONFIG_HOME=some-existing-dir
  unset HOME
  git config foo.baz boz

should die("$HOME is unset") or use the XDG config file.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

[PATCH v2] config: fix several access(NULL) calls

[Matthieu Moy]

When $HOME is unset, home_config_paths fails and returns NULL pointers
for user_config and xdg_config. Valgrind complains with Syscall param
access(pathname) points to unaddressable byte(s).

Don't call blindly access() on these variables, but test them for
NULL-ness before.

The when the XDG configuration file can be found but not $HOME/.gitconfig
requires a bit of attention. We chose to error out in "git config --set"
if $HOME is unset anyway.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---

Before I forget about it, here's the patch assuming people do want to
error out when $HOME is unset. It should be functionally equivalent to
the previous one, but the code should be clearer.

 builtin/config.c | 15 +++++++++++----
 config.c         |  4 ++--
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index e8e1c0a..f064d6e 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -387,12 +387,19 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 		home_config_paths(&user_config, &xdg_config, "config");
 
-		if (access(user_config, R_OK) && !access(xdg_config, R_OK))
+		if (!user_config)
+			/*
+			 * Don't even try to access the xdg_config, as
+			 * unset $HOME means something is really
+			 * broken and should be fixed. Silently
+			 * writing to xdg_config may be confusing.
+			 */
+			die("$HOME not set");
+		else if (access(user_config, R_OK) &&
+			 xdg_config && !access(xdg_config, R_OK))
 			given_config_file = xdg_config;
-		else if (user_config)
-			given_config_file = user_config;
 		else
-			die("$HOME not set");
+			given_config_file = user_config;
 	}
 	else if (use_system_config)
 		given_config_file = git_etc_gitconfig();
diff --git a/config.c b/config.c
index d28a499..6b97503 100644
--- a/config.c
+++ b/config.c
@@ -940,12 +940,12 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 		found += 1;
 	}
 
-	if (!access(xdg_config, R_OK)) {
+	if (xdg_config && !access(xdg_config, R_OK)) {
 		ret += git_config_from_file(fn, xdg_config, data);
 		found += 1;
 	}
 
-	if (!access(user_config, R_OK)) {
+	if (user_config && !access(user_config, R_OK)) {
 		ret += git_config_from_file(fn, user_config, data);
 		found += 1;
 	}
-- 
1.7.11.1.30.g7e1baf9.dirty

Re: [PATCH] config: fix several access(NULL) calls

[Jeff King]

On Fri, Jul 13, 2012 at 10:48:18AM +0200, Matthieu Moy wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > But is it really true that we want to error out on missing HOME if
> > we have usable XDG stuff?
> 
> Anyone else have an opinion on this?
> 
> In short, the question is whether
> 
>   export XDG_CONFIG_HOME=some-existing-dir
>   unset HOME
>   git config foo.baz boz
> 
> should die("$HOME is unset") or use the XDG config file.

What did previous versions of git do? From my reading of 21cf32279, the
previous behavior was that if $HOME was not set, git would silently
avoid reading from $HOME/.gitconfig entirely. Wouldn't dying be a huge
regression?

-Peff

Re: [PATCH] config: fix several access(NULL) calls

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> On Fri, Jul 13, 2012 at 10:48:18AM +0200, Matthieu Moy wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> > But is it really true that we want to error out on missing HOME if
>> > we have usable XDG stuff?
>> 
>> Anyone else have an opinion on this?
>> 
>> In short, the question is whether
>> 
>>   export XDG_CONFIG_HOME=some-existing-dir
>>   unset HOME
>>   git config foo.baz boz
>> 
>> should die("$HOME is unset") or use the XDG config file.
>
> What did previous versions of git do? From my reading of 21cf32279, the
> previous behavior was that if $HOME was not set, git would silently
> avoid reading from $HOME/.gitconfig entirely.

Yes, and this is still the case for _reading_. But the current case is
about deciding which file to use when _writing_. Git was already dying
when writing with an unset $HOME. There is no behavior change in this
case.

With Junio's suggestion, we would have a behavior change in that we
would write to the XDG file if we can find it (using XDG_CONFIG_HOME,
obviously, since $HOME is unset in this case).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] config: fix several access(NULL) calls

[Thomas Rast]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Jeff King <peff@peff.net> writes:
>
>> On Fri, Jul 13, 2012 at 10:48:18AM +0200, Matthieu Moy wrote:
>>
>>> Junio C Hamano <gitster@pobox.com> writes:
>>> 
>>> > But is it really true that we want to error out on missing HOME if
>>> > we have usable XDG stuff?
>>> 
>>> Anyone else have an opinion on this?
>>> 
>>> In short, the question is whether
>>> 
>>>   export XDG_CONFIG_HOME=some-existing-dir
>>>   unset HOME
>>>   git config foo.baz boz
>>> 
>>> should die("$HOME is unset") or use the XDG config file.
>>
>> What did previous versions of git do? From my reading of 21cf32279, the
>> previous behavior was that if $HOME was not set, git would silently
>> avoid reading from $HOME/.gitconfig entirely.
>
> Yes, and this is still the case for _reading_. But the current case is
> about deciding which file to use when _writing_. Git was already dying
> when writing with an unset $HOME.

Umm, are you sure?  I may be somewhat confused about this, but the tests
I used to trigger the access(NULL) were IIRC

  unset HOME
  git config --get foo.bar
  git config --global --get foo.bar

none of which is writing....

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] config: fix several access(NULL) calls

[Matthieu Moy]

Thomas Rast <trast@student.ethz.ch> writes:

> Umm, are you sure?  I may be somewhat confused about this, but the tests
> I used to trigger the access(NULL) were IIRC
>
>   unset HOME
>   git config --get foo.bar
>   git config --global --get foo.bar
>
> none of which is writing....

I was inaccurate, but that doesn't change the conclusion: the question
is not reading Vs writting, but use of --global Vs no use of it (and
most of the time, --global is used for writing, hence my confusion).

Both the patch that introduce the NULL bug and the one that fix it touch
two files: $git/config.c, and $git/builtin/config.c.

My changes to $git/config.c, are straightforward because they touch
positive tests for file existance. The controversial one is
$git/builtin/config.c, which is inside a "if (use_global_config)".

This piece of code was already dying on unset $HOME, and in my proposal
it still is.

The old code was:

        if (use_global_config) {
                char *home = getenv("HOME");
                if (home) {
                        char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
                        given_config_file = user_config;
                } else {
                        die("$HOME not set");
                }
        }

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] config: fix several access(NULL) calls

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> The old code was:
>
>         if (use_global_config) {
>                 char *home = getenv("HOME");
>                 if (home) {
>                         char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
>                         given_config_file = user_config;
>                 } else {
>                         die("$HOME not set");
>                 }
>         }

I think the above is a good illustration of the crux of the issue,
as the natural way to introduce XDG that is used when the one at
$HOME/.gitconfig is not there seems at least to me to do this:

	if (use_global_config) {
                if (is $HOME/.gitconfig usable?) {
			use it;
		} else if (is $XDG/git/config usable?) {
                	use $XDG location;
		} else {
			die("Neither $XDG or $HOME is usable");
		}
	}

In other words, we are not trying to say "You do not have $HOME --
there is something wrong, so we will give up" in this codepath.  We
were saying "Can we use _the_ place we place personal config?  It is
an error if we cannot find it, so we die." and the above says "Can
we use this place?  Can we use that place?  If no place is found to
be usable, we die."

And the intent of the "NULL-buggy" version seems to want the same
"If we cannot use home/.gitconfig but xdg_config one is usable, use
xdg one" semantics to me.

		if (access(user_config, R_OK) && !access(xdg_config, R_OK))
 			given_config_file = xdg_config;
 		else if (user_config)
 			given_config_file = user_config;

Re: [PATCH] config: fix several access(NULL) calls

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> 	if (use_global_config) {
>                 if (is $HOME/.gitconfig usable?) {
> 			use it;

Yes, but when $HOME is unset, the question doesn't really make sense.
Maybe the file exists, but we can't know since the user broke his
configuration by unsetting $HOME. The intent was to avoid writing to the
XDG file unless it was very clear that the user wanted it, so in doubt,
dying seems the best option.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] config: fix several access(NULL) calls

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> 	if (use_global_config) {
>>                 if (is $HOME/.gitconfig usable?) {
>> 			use it;
>
> Yes, but when $HOME is unset, the question doesn't really make sense.
> Maybe the file exists, but we can't know since the user broke his
> configuration by unsetting $HOME. The intent was to avoid writing to the
> XDG file unless it was very clear that the user wanted it, so in doubt,
> dying seems the best option.

I would think that it is plausible that the user wanted to write
into XDG one and used "unset HOME" as a way to signal that wish. 

Are there ways to force writing into XDG ones without having to
remove the $HOME/ ones (perhaps the user wants to keep them for use
with older versions of Git on a different machine that shares the
same $HOME directory)?  Temporarily unsetting HOME may be how a user
might achieve it.

If you want to disallow such a use case, that is fine, but at least
the logic needs to be described in comment.  Perhaps based on one of
your rewrites earlier in the thread, like this?

 builtin/config.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 67945b2..a788409 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -387,13 +387,26 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 		home_config_paths(&user_config, &xdg_config, "config");
 
-		if (user_config && access(user_config, R_OK) &&
+		if (!user_config)
+			/*
+			 * We do not know HOME/.gitconfig exists or
+			 * not, hence we do not know if we should
+			 * write to XDG location, so we error out even
+			 * if XDG_CONFIG_HOME is set and points at a
+			 * sane location.
+			 *
+			 * In other words, we forbid the user from
+			 * telling us to write to XDG location,
+			 * pretending that $HOME/.gitconfig does not
+			 * exist, by temporarily unsetting HOME.
+			 */
+			die("$HOME not set");
+
+		if (access(user_config, R_OK) &&
 		    xdg_config && !access(xdg_config, R_OK))
 			given_config_file = xdg_config;
-		else if (user_config)
-			given_config_file = user_config;
 		else
-			die("$HOME not set");
+			given_config_file = user_config;
 	}
 	else if (use_system_config)
 		given_config_file = git_etc_gitconfig();

Re: [PATCH] config: fix several access(NULL) calls

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> I would think that it is plausible that the user wanted to write
> into XDG one and used "unset HOME" as a way to signal that wish. 

I didn't think of this case, but it makes sense.

Anyway, I don't really care either way, so I leave it up to you (either
your patch below, or the fixup in pu).

> -		if (user_config && access(user_config, R_OK) &&
> +		if (!user_config)
> +			/*
> +			 * We do not know HOME/.gitconfig exists or
> +			 * not, hence we do not know if we should
> +			 * write to XDG location, so we error out even
> +			 * if XDG_CONFIG_HOME is set and points at a
> +			 * sane location.
> +			 *
> +			 * In other words, we forbid the user from
> +			 * telling us to write to XDG location,
> +			 * pretending that $HOME/.gitconfig does not
> +			 * exist, by temporarily unsetting HOME.
> +			 */
> +			die("$HOME not set");
> +
> +		if (access(user_config, R_OK) &&
>  		    xdg_config && !access(xdg_config, R_OK))
>  			given_config_file = xdg_config;
> -		else if (user_config)
> -			given_config_file = user_config;
>  		else
> -			die("$HOME not set");
> +			given_config_file = user_config;

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] config: fix several access(NULL) calls

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> I would think that it is plausible that the user wanted to write
>> into XDG one and used "unset HOME" as a way to signal that wish. 
>
> I didn't think of this case, but it makes sense.
>
> Anyway, I don't really care either way, so I leave it up to you (either
> your patch below, or the fixup in pu).

I do not care deeply either, as there always is "config -f $file" if
somebody wants to write into a specific location.

But if you do something that is different from a straight-and-dumb
conversion from access(A, B) to (A ? access(A, B) : -1), the
conversion is bound to be questioned by somebody in 2 years "isn't
this a misconversion?".  If you did that on purpose, a comment to
explain why it is not a misconversion would help them understand the
reasoning behind it.

[PATCH 2/4 v8] Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore

[Matthieu Moy]

From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>

To use the feature of core.excludesfile, the user needs:

 1. to create such a file,

 2. and add configuration variable to point at it.

Instead, we can make this a one-step process by choosing a default value
which points to a filename in the user's $HOME, that is unlikely to
already exist on the system, and only use the presence of the file as a
cue that the user wants to use that feature.

We use "${XDG_CONFIG_HOME:-$HOME/.config/git}/ignore" as a default value.
It goes in the same directory as the newly added configuration
file ("${XDG_CONFIG_HOME:-$HOME/.config/git}/config), and follows the XDG
specification.

Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt    |  4 +++-
 Documentation/gitignore.txt |  4 +++-
 dir.c                       |  7 ++++++-
 t/t1306-xdg-files.sh        | 29 +++++++++++++++++++++++++++++
 4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 915cb5a..20e9531 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -483,7 +483,9 @@ core.excludesfile::
 	'.git/info/exclude', git looks into this file for patterns
 	of files which are not meant to be tracked.  "`~/`" is expanded
 	to the value of `$HOME` and "`~user/`" to the specified user's
-	home directory.  See linkgit:gitignore[5].
+	home directory. Its default value is $XDG_CONFIG_HOME/git/ignore.
+	If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/ignore
+	is used instead. See linkgit:gitignore[5].
 
 core.askpass::
 	Some commands (e.g. svn and http interfaces) that interactively
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 2e7328b..c1f692a 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -50,7 +50,9 @@ the repository but are specific to one user's workflow) should go into
 the `$GIT_DIR/info/exclude` file.  Patterns which a user wants git to
 ignore in all situations (e.g., backup or temporary files generated by
 the user's editor of choice) generally go into a file specified by
-`core.excludesfile` in the user's `~/.gitconfig`.
+`core.excludesfile` in the user's `~/.gitconfig`. Its default value is
+$XDG_CONFIG_HOME/git/ignore. If $XDG_CONFIG_HOME is either not set or empty,
+$HOME/.config/git/ignore is used instead.
 
 The underlying git plumbing tools, such as
 'git ls-files' and 'git read-tree', read
diff --git a/dir.c b/dir.c
index ed1510f..8c6f47f 100644
--- a/dir.c
+++ b/dir.c
@@ -1234,12 +1234,17 @@ int remove_dir_recursively(struct strbuf *path, int flag)
 void setup_standard_excludes(struct dir_struct *dir)
 {
 	const char *path;
+	char *xdg_path;
 
 	dir->exclude_per_dir = ".gitignore";
 	path = git_path("info/exclude");
+	if (!excludes_file) {
+		home_config_paths(NULL, &xdg_path, "ignore");
+		excludes_file = xdg_path;
+	}
 	if (!access(path, R_OK))
 		add_excludes_from_file(dir, path);
-	if (excludes_file && !access(excludes_file, R_OK))
+	if (!access(excludes_file, R_OK))
 		add_excludes_from_file(dir, excludes_file);
 }
 
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 5b971d9..05103f5 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -67,4 +67,33 @@ test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists'
 '
 
 
+test_expect_success 'Setup' '
+	git init git &&
+	cd git &&
+	echo foo >to_be_excluded
+'
+
+
+test_expect_success 'Exclusion of a file in the XDG ignore file' '
+	mkdir -p "$HOME"/.config/git/ &&
+	echo to_be_excluded >"$HOME"/.config/git/ignore &&
+	test_must_fail git add to_be_excluded
+'
+
+
+test_expect_success 'Exclusion in both XDG and local ignore files' '
+	echo to_be_excluded >.gitignore &&
+	test_must_fail git add to_be_excluded
+'
+
+
+test_expect_success 'Exclusion in a non-XDG global ignore file' '
+	rm .gitignore &&
+	echo >"$HOME"/.config/git/ignore &&
+	echo to_be_excluded >"$HOME"/my_gitignore &&
+	git config core.excludesfile "$HOME"/my_gitignore &&
+	test_must_fail git add to_be_excluded
+'
+
+
 test_done
-- 
1.7.11.rc3.235.gd0d1d08

[PATCH 3/4 v8] Let core.attributesfile default to $XDG_CONFIG_HOME/git/ignore

[Matthieu Moy]

From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>

To use the feature of core.attributesfile, the user needs:

 1. to create such a file,

 2. and add configuration variable to point at it.

Instead, we can make this a one-step process by choosing a default value
which points to a filename in the user's $HOME, that is unlikely to
already exist on the system, and only use the presence of the file as a
cue that the user wants to use that feature.

We use "${XDG_CONFIG_HOME:-$HOME/.config/git}/ignore" as a default value.
It goes in the same directory as the newly added configuration
file ("${XDG_CONFIG_HOME:-$HOME/.config/git}/config), and follows the XDG
specification.

Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---

The commit message is amlost identical to the previous one to make the
patch self-contained, but this can be changed to something shorter
like "like the previous patch, we set the default avlue ..." if needed.

 Documentation/config.txt        |  4 +++-
 Documentation/gitattributes.txt |  2 ++
 attr.c                          | 17 ++++++++++-------
 t/t1306-xdg-files.sh            | 29 +++++++++++++++++++++++++++++
 4 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 20e9531..db07459 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -500,7 +500,9 @@ core.attributesfile::
 	In addition to '.gitattributes' (per-directory) and
 	'.git/info/attributes', git looks into this file for attributes
 	(see linkgit:gitattributes[5]). Path expansions are made the same
-	way as for `core.excludesfile`.
+	way as for `core.excludesfile`. Its default value is
+	$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
+	set or empty, $HOME/.config/git/attributes is used instead.
 
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 80120ea..e16f3e1 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -75,6 +75,8 @@ repositories (i.e., attributes of interest to all users) should go into
 `.gitattributes` files. Attributes that should affect all repositories
 for a single user should be placed in a file specified by the
 `core.attributesfile` configuration option (see linkgit:git-config[1]).
+Its default value is $XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME
+is either not set or empty, $HOME/.config/git/attributes is used instead.
 Attributes for all users on a system should be placed in the
 `$(prefix)/etc/gitattributes` file.
 
diff --git a/attr.c b/attr.c
index 303751f..aef93d8 100644
--- a/attr.c
+++ b/attr.c
@@ -497,6 +497,7 @@ static int git_attr_system(void)
 static void bootstrap_attr_stack(void)
 {
 	struct attr_stack *elem;
+	char *xdg_attributes_file;
 
 	if (attr_stack)
 		return;
@@ -515,13 +516,15 @@ static void bootstrap_attr_stack(void)
 		}
 	}
 
-	if (git_attributes_file) {
-		elem = read_attr_from_file(git_attributes_file, 1);
-		if (elem) {
-			elem->origin = NULL;
-			elem->prev = attr_stack;
-			attr_stack = elem;
-		}
+	if (!git_attributes_file) {
+		home_config_paths(NULL, &xdg_attributes_file, "attributes");
+		git_attributes_file = xdg_attributes_file;
+	}
+	elem = read_attr_from_file(git_attributes_file, 1);
+	if (elem) {
+		elem->origin = NULL;
+		elem->prev = attr_stack;
+		attr_stack = elem;
 	}
 
 	if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 05103f5..e8cd78a 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -96,4 +96,33 @@ test_expect_success 'Exclusion in a non-XDG global ignore file' '
 '
 
 
+test_expect_success 'Checking attributes in the XDG attributes file' '
+	echo foo >f &&
+	git check-attr -a f >actual &&
+	test_line_count -eq 0 actual &&
+	echo "f attr_f" >"$HOME"/.config/git/attributes &&
+	echo "f: attr_f: set" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'Checking attributes in both XDG and local attributes files' '
+	echo "f -attr_f" >.gitattributes &&
+	echo "f: attr_f: unset" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+
+test_expect_success 'Checking attributes in a non-XDG global attributes file' '
+	test_might_fail rm .gitattributes &&
+	echo "f attr_f=test" >"$HOME"/my_gitattributes &&
+	git config core.attributesfile "$HOME"/my_gitattributes &&
+	echo "f: attr_f: test" >expected &&
+	git check-attr -a f >actual &&
+	test_cmp expected actual
+'
+
+
 test_done
-- 
1.7.11.rc3.235.gd0d1d08

Re: [PATCH 3/4 v8] Let core.attributesfile default to $XDG_CONFIG_HOME/git/ignore

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
>
> To use the feature of core.attributesfile, the user needs:
>
>  1. to create such a file,
>
>  2. and add configuration variable to point at it.
>
> Instead, we can make this a one-step process by choosing a default value
> which points to a filename in the user's $HOME, that is unlikely to
> already exist on the system, and only use the presence of the file as a
> cue that the user wants to use that feature.
>
> We use "${XDG_CONFIG_HOME:-$HOME/.config/git}/ignore" as a default value.
> It goes in the same directory as the newly added configuration
> file ("${XDG_CONFIG_HOME:-$HOME/.config/git}/config), and follows the XDG
> specification.

The patch text may be OK but look at the Subject: line and notice
something funny ;-)

Given the root cause of that "something funny", I'll reduce the log
message of this one down to:

    Let core.attributesfile default to $XDG_CONFIG_HOME/git/attributes
    
    This gives the default value for the core.attributesfile variable
    following the exact same logic of the previous change for the
    core.excludesfile setting.
    
    Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
    ...
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

I am not sure about use of git/ignore in [2/2], though.

Shouldn't the default value for core.excludesfile be git/exclude,
not git/ignore?

Re: [PATCH 3/4 v8] Let core.attributesfile default to $XDG_CONFIG_HOME/git/ignore

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> The patch text may be OK but look at the Subject: line and notice
> something funny ;-)

Nice catch.

> Given the root cause of that "something funny", I'll reduce the log
> message of this one down to:
>
>     Let core.attributesfile default to $XDG_CONFIG_HOME/git/attributes
>     
>     This gives the default value for the core.attributesfile variable
>     following the exact same logic of the previous change for the
>     core.excludesfile setting.
>     
>     Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
>     ...
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>

Good (I see you did the change yourself in pu).

> I am not sure about use of git/ignore in [2/2], though.
>
> Shouldn't the default value for core.excludesfile be git/exclude,
> not git/ignore?

Both are valid options, and unfortunately, Git currently use both
"ignore" and "exclude" to mean the same thing. 

I have a slight preference for "ignore", because this file is the
user-wide version of the .gitignore file. I think most non-advanced
users have already been exposed to the wording "ignore" with this file,
but much less have been exposed to "exclude" (for example, I don't
remember having ever used the per-directory .git/info/exclude). So,
going for "ignore" means that the wording used for the most common
operations will be consistant, and only relatively advanced users (who
use .git/info/exclude, or "git ls-files --exclude-standard", or need to
set core.excludesfile to some value other than the default) will need to
learn that "exclude" is synonym for "ignore" in the Git world.

Actually, it would even make sense to reduce the number of occurences of
"excludes" in the UI, e.g. support something like core.ignoresfile as an
alias for core.excludesfile, along the lines of:

--- a/config.c
+++ b/config.c
@@ -723,7 +723,7 @@ static int git_default_core_config(const char *var, const char *value)
        if (!strcmp(var, "core.askpass"))
                return git_config_string(&askpass_program, var, value);
 
-       if (!strcmp(var, "core.excludesfile"))
+       if (!strcmp(var, "core.excludesfile") || !strcmp(var, "core.ignoresfile"))
                return git_config_pathname(&excludes_file, var, value);
 
        if (!strcmp(var, "core.whitespace")) {

(which would off course require some documentation)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 3/4 v8] Let core.attributesfile default to $XDG_CONFIG_HOME/git/ignore

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Actually, it would even make sense to reduce the number of occurences of
> "excludes" in the UI, e.g. support something like core.ignoresfile as an
> alias for core.excludesfile, along the lines of:
>
> --- a/config.c
> +++ b/config.c
> @@ -723,7 +723,7 @@ static int git_default_core_config(const char *var, const char *value)
>         if (!strcmp(var, "core.askpass"))
>                 return git_config_string(&askpass_program, var, value);
>  
> -       if (!strcmp(var, "core.excludesfile"))
> +       if (!strcmp(var, "core.excludesfile") || !strcmp(var, "core.ignoresfile"))
>                 return git_config_pathname(&excludes_file, var, value);

I somehow suspect this is going backwards.  ".gitignore" may have
been the original mistake; it is much more than ".cvsignore".

But I do not deeply care either way.  It just felt $path/git/ignore
was out of place given $path/.git/info/exclude has been with us
almost forever.

Re: [PATCH 3/4 v8] Let core.attributesfile default to $XDG_CONFIG_HOME/git/ignore

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> I somehow suspect this is going backwards.  ".gitignore" may have
> been the original mistake; it is much more than ".cvsignore".

I don't think they are _that_ different, and if we're talking about how
other VCS call this, I can think of ".hgignore", ".bzrignore", ...

Even within the Git world, see how "man git-add" describes the behavior
of ignore/exclude:

       The git add command will not add ignored files by default. If any
       ignored files were explicitly specified on the command line, git
       add will fail with a list of ignored files. Ignored files reached
       by directory recursion or filename globbing performed by Git
       (quote your globs before the shell) will be silently ignored. The
       git add command can be used to add ignored files with the -f
       (force) option.

Another fun fact: google "git exclude" gives this as first result

  http://www.kernel.org/pub/software/scm/git/docs/gitignore.html

;-)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

[PATCH 4/4 v8] config: write to $XDG_CONFIG_HOME/git/config file if appropriate

[Matthieu Moy]

From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>

Git will be able to write to $XDG_CONFIG_HOME/git/config, a new
configuration file following XDG specification, if:

 1. this file already exists, and

 2. $HOME/.gitconfig file doesn't, and

 3. The --global option is used.

Otherwise Git writes to $HOME/.gitconfig with the --global option, as
usual. If the user doesn't create $XDG_CONFIG_HOME/git/config, there is
absolutely no change. Users can use this new file only if they want.

If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/config
will be used.

Advice for users who often come back to an old version of Git: you
shouldn't create this file.

Signed-off-by: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <Valentin.Duperray@ensimag.imag.fr>
Signed-off-by: Franck Jonas <Franck.Jonas@ensimag.imag.fr>
Signed-off-by: Lucien Kong <Lucien.Kong@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <Thomas.Nguy@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-config.txt |  3 ++-
 builtin/config.c             |  5 +----
 t/t1306-xdg-files.sh         | 30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 7e344a2..fb10082 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -97,7 +97,8 @@ OPTIONS
 
 --global::
 	For writing options: write to global ~/.gitconfig file rather than
-	the repository .git/config.
+	the repository .git/config, write to $XDG_CONFIG_HOME/git/config file
+	if this file exists and the ~/.gitconfig file doesn't.
 +
 For reading options: read only from global ~/.gitconfig and from
 $XDG_CONFIG_HOME/git/config rather than from all available files.
diff --git a/builtin/config.c b/builtin/config.c
index da54fd1..e8e1c0a 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -387,10 +387,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 		home_config_paths(&user_config, &xdg_config, "config");
 
-		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&
-		    (actions == ACTION_LIST ||
-		     actions == ACTION_GET_COLOR ||
-		     actions == ACTION_GET_COLORBOOL))
+		if (access(user_config, R_OK) && !access(xdg_config, R_OK))
 			given_config_file = xdg_config;
 		else if (user_config)
 			given_config_file = user_config;
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index e8cd78a..3c75c3f 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -125,4 +125,34 @@ test_expect_success 'Checking attributes in a non-XDG global attributes file' '
 '
 
 
+test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'\''t' '
+	mkdir -p "$HOME"/.config/git &&
+	>"$HOME"/.config/git/config &&
+	test_might_fail rm "$HOME"/.gitconfig &&
+	git config --global user.name "write_config" &&
+	echo "[user]" >expected &&
+	echo "	name = write_config" >>expected &&
+	test_cmp expected "$HOME"/.config/git/config
+'
+
+
+test_expect_success 'write: xdg file exists and ~/.gitconfig exists' '
+	>"$HOME"/.gitconfig &&
+	git config --global user.name "write_gitconfig" &&
+	echo "[user]" >expected &&
+	echo "	name = write_gitconfig" >>expected &&
+	test_cmp expected "$HOME"/.gitconfig
+'
+
+
+test_expect_success 'write: ~/.config/git/ exists and config file doesn'\''t' '
+	test_might_fail rm "$HOME"/.gitconfig &&
+	test_might_fail rm "$HOME"/.config/git/config &&
+	git config --global user.name "write_gitconfig" &&
+	echo "[user]" >expected &&
+	echo "	name = write_gitconfig" >>expected &&
+	test_cmp expected "$HOME"/.gitconfig
+'
+
+
 test_done
-- 
1.7.11.rc3.235.gd0d1d08

Re: [PATCH 4/4 v8] config: write to $XDG_CONFIG_HOME/git/config file if appropriate

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
>
> Git will be able to write to $XDG_CONFIG_HOME/git/config, a new
> ...
> Advice for users who often come back to an old version of Git: you
> shouldn't create this file.

Hrm, is there a better way to give this advice to the _users_?  The
log message is primarily for people who need to dig the development
history of Git, and they are not the people who will be hurt by this
change.  Perhaps the change to Documentation/git-config.txt in this
patch needs to be a bit more detailed?

Ideally, it would be nice if we could do the usual "advise" thing
by detecting a possible confusion, but I do not offhand see a clever
way to do so.

Re: [PATCH 4/4 v8] config: write to $XDG_CONFIG_HOME/git/config file if appropriate

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
>>
>> Git will be able to write to $XDG_CONFIG_HOME/git/config, a new
>> ...
>> Advice for users who often come back to an old version of Git: you
>> shouldn't create this file.
>
> Hrm, is there a better way to give this advice to the _users_?  The
> log message is primarily for people who need to dig the development
> history of Git, and they are not the people who will be hurt by this
> change.  Perhaps the change to Documentation/git-config.txt in this
> patch needs to be a bit more detailed?

We can squash this into PATCH 1:

--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -209,7 +209,9 @@ $XDG_CONFIG_HOME/git/config::
        Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
        or empty, $HOME/.config/git/config will be used. Any single-valued
        variable set in this file will be overwritten by whatever is in
-       ~/.gitconfig.
+       ~/.gitconfig. Support for this file was added after Git
+       1.7.11, so if you sometimes use older versions of Git, it's a
+       good idea not to create this file.
 
 $(prefix)/etc/gitconfig::
        System-wide configuration file.

(If your cristal ball tells you what version number will follow 1.7.11
(1.7.12 or 1.8.0?), we can change "after 1.7.11" to "in X". Do you want
to squash it yourself, or shall I resend?

> Ideally, it would be nice if we could do the usual "advise" thing
> by detecting a possible confusion, but I do not offhand see a clever
> way to do so.

I don't see either, since the advice should be given only to people who
use various versions of Git.

Anyway, I don't think we should bother too much with this: it's really
an opt-in feature, Git will never create the new file for you (it will
only write in it if it exists). So, people potentially harmed by this
change are only those who read the documentation. If the documentation
warns them, I think it's sufficient. It's not very different from any
new feature actually.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 4/4 v8] config: write to $XDG_CONFIG_HOME/git/config file if appropriate

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> We can squash this into PATCH 1:
>
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -209,7 +209,9 @@ $XDG_CONFIG_HOME/git/config::
>         Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
>         or empty, $HOME/.config/git/config will be used. Any single-valued
>         variable set in this file will be overwritten by whatever is in
> -       ~/.gitconfig.
> +       ~/.gitconfig. Support for this file was added after Git
> +       1.7.11, so if you sometimes use older versions of Git, it's a
> +       good idea not to create this file.

Okay.  Thanks.

Re: [PATCH 0/4 v8] Git configuration directory

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> I took the liberty of rewording the commit messages, and change a few
> words in the documentation (changed some "will be used" to "is used
> instead", which sounded more consistant with the context).

Thanks; the series overall looks good.

I may have made further tweaks by the time I push the result
out.  Please watch 'pu' and holler if you find anything objectionable.



> A reminder of the context and objectives of the serie:
>
> Git currently stores its configuration file in ~/.gitconfig, which is
> nice and customary on Unix, as long as one has only one configuration
> file.
>
> But a typical user may want to store not only the config file, but
> also the files pointed to by core.excludesfile and
> core.attributesfile, which currently have no default values. To store
> several configuration files, it makes sense to have a configuration
> _directory_ instead of a set of configuration files, all right into
> $HOME. Calling this configuration directory ~/.git or ~/.gitconfig is
> not an option, since these paths already have another meaning. Using
> the XDG specification (in short: ~/.config/git) doesn't have this
> drawback, and allows the user to store his configuration files right
> next to files from other applications following the XDG standard.
>
> The first 3 patches add read support for this "configuration
> directory", and the last one gives opt-in write support, allowing
> users to make this "configuration directory" their way of life, and
> never hear again about ~/.gitconfig if (and only if) they do not want to.
>
> None of the patches change the behavior for people who do not create
> the new configuration files.
>
> Huynh Khoi Nguyen Nguyen (4):
>   config: read (but not write) from $XDG_CONFIG_HOME/git/config file
>   Let core.excludesfile default to $XDG_CONFIG_HOME/git/ignore
>   Let core.attributesfile default to $XDG_CONFIG_HOME/git/ignore
>   config: write to $XDG_CONFIG_HOME/git/config file if appropriate
>
>  Documentation/config.txt        |   8 +-
>  Documentation/git-config.txt    |  15 +++-
>  Documentation/gitattributes.txt |   2 +
>  Documentation/gitignore.txt     |   4 +-
>  attr.c                          |  17 +++--
>  builtin/config.c                |  25 ++++---
>  cache.h                         |   3 +
>  config.c                        |  23 +++---
>  dir.c                           |   7 +-
>  path.c                          |  41 +++++++++++
>  t/t1306-xdg-files.sh            | 158 ++++++++++++++++++++++++++++++++++++++++
>  11 files changed, 270 insertions(+), 33 deletions(-)
>  create mode 100755 t/t1306-xdg-files.sh

Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files

[Ramsay Jones]

Huynh Khoi Nguyen NGUYEN wrote:
> From: NGUYEN Huynh Khoi Nguyen <nguyenhu@ensibm.imag.fr>
> 
> Git will be able to read in $XDG_CONFIG_HOME/git/config, a new
> configuration file following XDG specification. In the order of
> reading, this file is between global configuration file and system
> wide configuration file. Git will not be able to write in this new
> configuration file. If core.excludesfile is not define, Git will read
> the global exclude files in $XDG_CONFIG_HOME/git/ignore. Same goes for
> core.attributesfile in $XDG_CONFIG_HOME/git/attributes. If
> $XDG_CONFIG_HOME is either not set or empty, $HOME/.config will be
> used.
> 
> Signed-off-by: Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
> Signed-off-by: Lucien KONG <Lucien.Kong@ensimag.imag.fr>
> Signed-off-by: Valentin DUPERRAY <Valentin.Duperray@ensimag.imag.fr>
> Signed-off-by: Thomas NGUY <Thomas.Nguy@ensimag.imag.fr>
> Signed-off-by: Franck JONAS <Franck.Jonas@ensimag.imag.fr>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> ---
>  Documentation/git-config.txt    |   12 +++-
>  attr.c                          |   10 +++
>  builtin/config.c                |   28 ++++++---
>  cache.h                         |    1 +
>  config.c                        |   21 ++++---
>  dir.c                           |    4 +
>  path.c                          |   26 ++++++++
>  t/t1306-read-xdg-config-file.sh |  133 +++++++++++++++++++++++++++++++++++++++
>  8 files changed, 214 insertions(+), 21 deletions(-)
>  create mode 100755 t/t1306-read-xdg-config-file.sh
> 

[...]

> diff --git a/attr.c b/attr.c
> index 303751f..441387f 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -497,6 +497,9 @@ static int git_attr_system(void)
>  static void bootstrap_attr_stack(void)
>  {
>  	struct attr_stack *elem;
> +	char *xdg_attributes_file;
> +
> +	home_config_paths(NULL, &xdg_attributes_file, "attributes");

who free()'s xdg_attributes_file ?

[...]

> diff --git a/config.c b/config.c
> index 71ef171..d1393b8 100644
> --- a/config.c
> +++ b/config.c
> @@ -929,7 +929,10 @@ int git_config_system(void)
>  int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  {
>  	int ret = 0, found = 0;
> -	const char *home = NULL;
> +	char *xdg_config = NULL;
> +	char *user_config = NULL;
> +
> +	home_config_paths(&user_config, &xdg_config, "config");

who free()'s user_config and xdg_config?

[...]

> diff --git a/dir.c b/dir.c
> index ed1510f..e0c3589 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1234,13 +1234,17 @@ int remove_dir_recursively(struct strbuf *path, int flag)
>  void setup_standard_excludes(struct dir_struct *dir)
>  {
>  	const char *path;
> +	char *xdg_path;
>  
>  	dir->exclude_per_dir = ".gitignore";
>  	path = git_path("info/exclude");
> +	home_config_paths(NULL, &xdg_path, "ignore");

ditto xdg_path ;-)

>  	if (!access(path, R_OK))
>  		add_excludes_from_file(dir, path);
>  	if (excludes_file && !access(excludes_file, R_OK))
>  		add_excludes_from_file(dir, excludes_file);
> +	else if (!access(xdg_path, R_OK))
> +		add_excludes_from_file(dir, xdg_path);
>  }
>  
>  int remove_path(const char *name)
> diff --git a/path.c b/path.c
> index 6f2aa69..53f3f53 100644
> --- a/path.c
> +++ b/path.c
> @@ -122,6 +122,32 @@ char *git_path(const char *fmt, ...)
>  	return cleanup_path(pathname);
>  }
>  
> +void home_config_paths(char **global, char **xdg, char *file)
> +{
> +	char *xdg_home = getenv("XDG_CONFIG_HOME");
> +	char *home = getenv("HOME");
> +	char *to_free = NULL;
> +
> +	if (!home) {
> +		if (global)
> +			*global = NULL;
> +	} else {
> +		if (!xdg_home) {
> +			to_free = strdup(mkpath("%s/.config", home));
> +			xdg_home = to_free;
> +		}
> +		if (global)
> +			*global = xstrdup(mkpath("%s/.gitconfig", home));
> +	}
> +
> +	if (!xdg_home)
> +		*xdg = NULL;
> +	else
> +		*xdg = xstrdup(mkpath("%s/git/%s", xdg_home, file));
> +
> +	free(to_free);
> +}
> +

So, this re-introduces the bug addressed by commit 05bab3ea. The test number
is now 29 (rather than 21) but the same test is failing; namely t3200-branch.sh
test #29 (git branch -m q q2 without config should succeed).

In order to fix the bug, I created the patch given below (on top of this patch).
(Note that it does not address the above issues).

HTH

ATB,
Ramsay Jones

-- >8 --
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Subject: [PATCH] path.c: Fix a static buffer overwrite bug by avoiding mkpath()

The v4 version of the "Read (but not write) from XDG configuration,
XDG attributes and XDG ignore files" patch, re-introduced the bug
addressed by commit 05bab3ea ("config.c: Fix a static buffer overwrite
bug by avoiding mkpath()", 19-11-2011). Note that the patch refactored
the code to determine the user (or home) configuration filename into
a new function (home_config_paths()). In doing so, the new code once
again uses mkpath() rather than mksnpath().

In order to fix the bug, we introduce a new variation of the mkpath()
function, mkpathdup(), which avoids the use of the internal static
buffers. As the name implies, the new function returns a pointer to
the pathname as a dynamically allocated string. It is the callers
responsibility to free the memory for the returned string.

Having introduced the new function, we can now replace the calls to
'xstrdup(mkpath(...))' in the home_config_paths() function with a
call to mkpathdup() to achieve the same effect, without tickling the
original bug.

(Also, note that the 'xstrdup(mkpath(...))' pattern occurs in several
other places in the source ...)

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 cache.h |    2 ++
 path.c  |   20 +++++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 0632503..fbba2d6 100644
--- a/cache.h
+++ b/cache.h
@@ -619,6 +619,8 @@ extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
 extern char *git_pathdup(const char *fmt, ...)
 	__attribute__((format (printf, 1, 2)));
+extern char *mkpathdup(const char *fmt, ...)
+	__attribute__((format (printf, 1, 2)));
 
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/path.c b/path.c
index 53f3f53..ca29bdd 100644
--- a/path.c
+++ b/path.c
@@ -87,6 +87,20 @@ char *git_pathdup(const char *fmt, ...)
 	return xstrdup(path);
 }
 
+char *mkpathdup(const char *fmt, ...)
+{
+	char path[PATH_MAX];
+	va_list args;
+	unsigned len;
+
+	va_start(args, fmt);
+	len = vsnprintf(path, sizeof(path), fmt, args);
+	va_end(args);
+	if (len >= sizeof(path))
+		return xstrdup(bad_path);
+	return xstrdup(cleanup_path(path));
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
@@ -133,17 +147,17 @@ void home_config_paths(char **global, char **xdg, char *file)
 			*global = NULL;
 	} else {
 		if (!xdg_home) {
-			to_free = strdup(mkpath("%s/.config", home));
+			to_free = mkpathdup("%s/.config", home);
 			xdg_home = to_free;
 		}
 		if (global)
-			*global = xstrdup(mkpath("%s/.gitconfig", home));
+			*global = mkpathdup("%s/.gitconfig", home);
 	}
 
 	if (!xdg_home)
 		*xdg = NULL;
 	else
-		*xdg = xstrdup(mkpath("%s/git/%s", xdg_home, file));
+		*xdg = mkpathdup("%s/git/%s", xdg_home, file);
 
 	free(to_free);
 }
-- 
1.7.10

Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> Subject: [PATCH] path.c: Fix a static buffer overwrite bug by avoiding mkpath()
>
> In order to fix the bug, we introduce a new variation of the mkpath()
> function, mkpathdup(), which avoids the use of the internal static
> buffers.

Shouldn't we aim a bit higher to also avoid the use of bounded
buffer?  Instead of returning bad_path, retry with lengthened buffer
until we succeed, or something.

Better yet, internally use strbuf_vaddf().

Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files

[Ramsay Jones]

Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> Subject: [PATCH] path.c: Fix a static buffer overwrite bug by avoiding mkpath()
>>
>> In order to fix the bug, we introduce a new variation of the mkpath()
>> function, mkpathdup(), which avoids the use of the internal static
>> buffers.
> 
> Shouldn't we aim a bit higher to also avoid the use of bounded
> buffer?  Instead of returning bad_path, retry with lengthened buffer
> until we succeed, or something.
> 
> Better yet, internally use strbuf_vaddf().

Sorry for the late reply, I've been away ...

Yes, I wasn't aiming too high. In fact I was only aiming for "I spent the
last 20 minutes fixing up your patch so that it doesn't tickle the bug
on cygwin, and it looks like this..." :-P

While away, I did rewrite mkpathdup() to address your concerns (although I
didn't use strbuf_vaddf()).

However, I see that Nguyen has not been idle and, with help from others, has
fixed up the patch and re-rolled the series (v7 I think). I haven't fetched
that mail yet, but it looked good and should not tickle the cygwin bug. I will
test it soon.

ATB,
Ramsay Jones

Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files

[nguyenhu]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> a écrit :
>
> So, this re-introduces the bug addressed by commit 05bab3ea. The test number
> is now 29 (rather than 21) but the same test is failing; namely  
> t3200-branch.sh
> test #29 (git branch -m q q2 without config should succeed).
>
> In order to fix the bug, I created the patch given below (on top of  
> this patch).
> (Note that it does not address the above issues).
>
> HTH
>
> ATB,
> Ramsay Jones
>
> -- >8 --
> From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> Subject: [PATCH] path.c: Fix a static buffer overwrite bug by  
> avoiding mkpath()
>
> The v4 version of the "Read (but not write) from XDG configuration,
> XDG attributes and XDG ignore files" patch, re-introduced the bug
> addressed by commit 05bab3ea ("config.c: Fix a static buffer overwrite
> bug by avoiding mkpath()", 19-11-2011). Note that the patch refactored
> the code to determine the user (or home) configuration filename into
> a new function (home_config_paths()). In doing so, the new code once
> again uses mkpath() rather than mksnpath().
>
> In order to fix the bug, we introduce a new variation of the mkpath()
> function, mkpathdup(), which avoids the use of the internal static
> buffers. As the name implies, the new function returns a pointer to
> the pathname as a dynamically allocated string. It is the callers
> responsibility to free the memory for the returned string.
>
> Having introduced the new function, we can now replace the calls to
> 'xstrdup(mkpath(...))' in the home_config_paths() function with a
> call to mkpathdup() to achieve the same effect, without tickling the
> original bug.
>
> (Also, note that the 'xstrdup(mkpath(...))' pattern occurs in several
> other places in the source ...)
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>  cache.h |    2 ++
>  path.c  |   20 +++++++++++++++++---
>  2 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 0632503..fbba2d6 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -619,6 +619,8 @@ extern char *git_snpath(char *buf, size_t n,  
> const char *fmt, ...)
>  	__attribute__((format (printf, 3, 4)));
>  extern char *git_pathdup(const char *fmt, ...)
>  	__attribute__((format (printf, 1, 2)));
> +extern char *mkpathdup(const char *fmt, ...)
> +	__attribute__((format (printf, 1, 2)));
>
>  /* Return a statically allocated filename matching the sha1 signature */
>  extern char *mkpath(const char *fmt, ...) __attribute__((format  
> (printf, 1, 2)));
> diff --git a/path.c b/path.c
> index 53f3f53..ca29bdd 100644
> --- a/path.c
> +++ b/path.c
> @@ -87,6 +87,20 @@ char *git_pathdup(const char *fmt, ...)
>  	return xstrdup(path);
>  }
>
> +char *mkpathdup(const char *fmt, ...)
> +{
> +	char path[PATH_MAX];
> +	va_list args;
> +	unsigned len;
> +
> +	va_start(args, fmt);
> +	len = vsnprintf(path, sizeof(path), fmt, args);
> +	va_end(args);
> +	if (len >= sizeof(path))
> +		return xstrdup(bad_path);
> +	return xstrdup(cleanup_path(path));
> +}
> +
>  char *mkpath(const char *fmt, ...)
>  {
>  	va_list args;
> @@ -133,17 +147,17 @@ void home_config_paths(char **global, char  
> **xdg, char *file)
>  			*global = NULL;
>  	} else {
>  		if (!xdg_home) {
> -			to_free = strdup(mkpath("%s/.config", home));
> +			to_free = mkpathdup("%s/.config", home);
>  			xdg_home = to_free;
>  		}
>  		if (global)
> -			*global = xstrdup(mkpath("%s/.gitconfig", home));
> +			*global = mkpathdup("%s/.gitconfig", home);
>  	}
>
>  	if (!xdg_home)
>  		*xdg = NULL;
>  	else
> -		*xdg = xstrdup(mkpath("%s/git/%s", xdg_home, file));
> +		*xdg = mkpathdup("%s/git/%s", xdg_home, file);
>
>  	free(to_free);
>  }
> --
> 1.7.10
>

Thank you for having fixed this bug we have re-introduced with your  
patch, we will add your modifications in our next v6 version.

Re: [PATCHv2] Possibility to read both from ~/.gitconfig and from $XDG_CONFIG_HOME/git/config

[nguyenhu]

Junio C Hamano <gitster@pobox.com> wrote:
> Is it just me who finds the above three lines extremely unreadable?
>
> Also can you give this patch a bit more sensible title?
> "Possibility to" does not tell us much---anything is possible if you
> change code after all.
>
> I see the patch does not touch the writing codepath, which is
> probably a good thing, but the log message should explicitly state
> that.

I will change that in my next version.

> I am not sure in what way $HOME/.gitconfig has "priority".
>
> Your proposed log message says that You read from $HOME/.gitconfig
> and then from $XDG_CONFIG_HOME/git/config, which means that any
> single-valued variable set in $HOME/.gitconfig will be overwritten
> by whatever is in $XDG_CONFIG_HOME/git/config, no?  That sounds like
> you are giving priority to the latter to me.

You are right. Git takes into account $HOME/.gitconfig if one variable  
is defined in both configuration files. I will explains that more  
clearly in documentation.

> The original that read from $HOME/.gitconfig was simple enough so
> having three copies of getenv("HOME") was perfectly fine, but as you
> are introduce this much complexity to to decide which two files to
> read from, the code added this patch needs to be refactored and
> three copies of the same logic need to be consolidated, I would have
> to say.

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Shouldn't there be a helper function to get the path ~/.config/git/
> and then append config? You're already computing this path twice, and
> we'll need more instances of it if we want to give default values to
> core.excludesfile and core.attributesfile in this directory too.

I agree. I will write a function to get ~/.config/git/ and refactor code.

Re: [PATCHv2] Possibility to read both from ~/.gitconfig and from $XDG_CONFIG_HOME/git/config

[nguyenhu]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> I have not tried this patch (or the v3 version, which I haven't read  
> yet), but
> is it likely that this has re-introduced the bug addressed by commit 05bab3ea
> ("config.c: Fix a static buffer overwrite bug by avoiding mkpath()",  
> >19-11-2011)?.
>
> I don't know the answer, but I suspect that it may have done just  
> that. >(indeed, it
> may well have made the bug more likely to appear).
>
>
>> The original that read from $HOME/.gitconfig was simple enough so
>> having three copies of getenv("HOME") was perfectly fine, but as you
>> are introduce this much complexity to to decide which two files to
>> read from, the code added this patch needs to be refactored and
>> three copies of the same logic need to be consolidated, I would have
>> to say.
>
> I agree. Also, using mksnpath() in the refactored code (rather than
> mkpath()) would be a good idea. :-P
>
> ATB,
> Ramsay Jones

Is not mkpath() the same function as mksnpath with char *buff =  
buf[PATH_MAX] and size_t n = sizeof(buf) ?

Re: [PATCHv2] Possibility to read both from ~/.gitconfig and from $XDG_CONFIG_HOME/git/config

[Ramsay Jones]

nguyenhu@minatec.inpg.fr wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>> I have not tried this patch (or the v3 version, which I haven't read  
>> yet), but
>> is it likely that this has re-introduced the bug addressed by commit 05bab3ea
>> ("config.c: Fix a static buffer overwrite bug by avoiding mkpath()",  
>>> 19-11-2011)?.
>> I don't know the answer, but I suspect that it may have done just  
>> that. >(indeed, it
>> may well have made the bug more likely to appear).
>>
>>
>>> The original that read from $HOME/.gitconfig was simple enough so
>>> having three copies of getenv("HOME") was perfectly fine, but as you
>>> are introduce this much complexity to to decide which two files to
>>> read from, the code added this patch needs to be refactored and
>>> three copies of the same logic need to be consolidated, I would have
>>> to say.
>> I agree. Also, using mksnpath() in the refactored code (rather than
>> mkpath()) would be a good idea. :-P
>>
>> ATB,
>> Ramsay Jones
> 
> Is not mkpath() the same function as mksnpath with char *buff =  
> buf[PATH_MAX] and size_t n = sizeof(buf) ?

I'm sorry but I just can't understand your question. :(

Have you looked at commit 05bab3ea? Is the commit message unclear?

The main difference between mkpath() and mksnpath(), as far as this bug
is concerned, is that mkpath() returns a reference to *recycled* internal
static buffers, whereas mksnpath() does not (you provide your own).

This evening I finally had a look at your patch, well v4 of the patch, and
I can confirm that it does indeed re-introduce the bug. I will reply to the
v4 patch email with more comments.

HTH

ATB,
Ramsay Jones