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

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Lucien KONG <***@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <***@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <***@ensimag.imag.fr>
Signed-off-by: Franck JONAS <***@ensimag.imag.fr>
---
Documentation/git-config.txt | 9 +++-
builtin/config.c | 48 +++++++++++++++++----
config.c | 21 +++++++--
t/t1306-read-xdg-config-file.sh | 88 +++++++++++++++++++++++++++++++++++++++
4 files changed, 150 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..2ce5e4b
--- /dev/null
+++ b/t/t1306-read-xdg-config-file.sh
@@ -0,0 +1,88 @@
+#!/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 "[user]" >.config/git/config &&
+ echo " name = read_config" >>.config/git/config &&
+ git config --global advice.statushints false &&
+ cat >expect <<-\EOF &&
+ # On branch master
+ #
+ # Initial commit
+ #
+ # Untracked files:
+ # .config/
+ # .gitconfig
+ # 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 "[user]" >.gitconfig &&
+ echo " name = read_gitconfig" >>.gitconfig &&
+ git config --global advice.statushints true &&
+ 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 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

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Lucien KONG <***@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <***@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <***@ensimag.imag.fr>
Signed-off-by: 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

[Junio C Hamano]

Post by Huynh Khoi Nguyen NGUYEN
.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.

Post by Huynh Khoi Nguyen NGUYEN
@@ -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
User-specific configuration file. Also called "global"
configuration file.
+ 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.

Post by Huynh Khoi Nguyen NGUYEN
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?

Post by Huynh Khoi Nguyen NGUYEN
@@ -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?

Post by Huynh Khoi Nguyen NGUYEN
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.

[Ramsay Jones]

[...]

Post by Junio C Hamano

Post by Huynh Khoi Nguyen NGUYEN
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).

Post by Junio C Hamano
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

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Lucien KONG <***@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <***@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <***@ensimag.imag.fr>
Signed-off-by: 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

[Junio C Hamano]

Post by Huynh Khoi Nguyen NGUYEN
Git will be able to read in $XDG_CONFIG_HOME/git/config, a new

Getting nicer.

Post by Huynh Khoi Nguyen NGUYEN
@@ -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);
}

Post by Huynh Khoi Nguyen NGUYEN
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.

Post by Huynh Khoi Nguyen NGUYEN
+ #
+ # .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.

Post by Huynh Khoi Nguyen NGUYEN
+ nothing added to commit but untracked files present
+ EOF
+ git status >output &&
+ test_cmp expect output
+'

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Lucien KONG <***@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <***@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <***@ensimag.imag.fr>
Signed-off-by: Franck JONAS <***@ensimag.imag.fr>
Signed-off-by: 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

[Matthieu Moy]

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")

[n***@minatec.inpg.fr]

I'd prefer having two separate patches for the config file and for th=

e

two others. If ignore and attributes are simple enough, they may go t=

o

the same patch, but ideally, there would be two separate patches agai=

n.

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 ;)

Post by Huynh Khoi Nguyen NGUYEN
--- 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 =3D attr_stack;
attr_stack =3D elem;
}
+ } else if (!access(xdg_attributes_file, R_OK)) {
+ elem =3D read_attr_from_file(xdg_attributes_file, 1);
+ if (elem) {
+ elem->origin =3D NULL;
+ elem->prev =3D attr_stack;
+ attr_stack =3D elem;
+ }
}
if (!is_bare_repository() || direction =3D=3D 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
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 =3D "foo";
+
if (git_attributes_file) {
elem =3D 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 t=

he

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 =
=20
existence of the xdg_attributes_file so it is indeed more clean to use =
=20
your version, Thank.

Post by Huynh Khoi Nguyen NGUYEN
--- a/dir.c
+++ b/dir.c
@@ -1234,13 +1234,17 @@ int remove_dir_recursively(struct strbuf =20
*path, int flag)
void setup_standard_excludes(struct dir_struct *dir)
{
const char *path;
+ char *xdg_path;
dir->exclude_per_dir =3D ".gitignore";
path =3D 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
http://thread.gmane.org/gmane.comp.version-control.git/133343/focus=3D=

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 =20
core.excludesfile is not set.
After checking your patch, it may be more logical to inialize the defau=
lt
value of excludes_file to the xdg_path as done for the attributes file.

Post by Huynh Khoi Nguyen NGUYEN
+ 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 beginnin=
g
of the test before ignoring it but that's seem a bit over-testing, I'll
remove it.

Post by Huynh Khoi Nguyen NGUYEN
+ cd .. &&
+ mkdir -p .config/git/ &&

I don't like these relative references to $HOME. If you mean $HOME, w=

hy

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-conf=
ig-file
so "$HOME" is needed for the tests to work.

[Matthieu Moy]

Post by n***@minatec.inpg.fr

Post by Matthieu Moy

Post by Huynh Khoi Nguyen NGUYEN
--- 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
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.

Post by n***@minatec.inpg.fr
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.

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <***@ensimag.imag.fr>
Signed-off-by: Franck JONAS <***@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <***@ensimag.imag.fr>
Signed-off-by: Lucien KONG <***@ensimag.imag.fr>
Signed-off-by: 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

[Matthieu Moy]

Post by Huynh Khoi Nguyen NGUYEN
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.

Post by Huynh Khoi Nguyen NGUYEN
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
'.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.

Post by Huynh Khoi Nguyen NGUYEN
--- 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.

[n***@minatec.inpg.fr]

Post by Matthieu Moy

Post by Huynh Khoi Nguyen NGUYEN
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.

Post by Huynh Khoi Nguyen NGUYEN
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

Likewise.

Post by Huynh Khoi Nguyen NGUYEN
--- 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 =
=20
v6 version.

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <***@ensimag.imag.fr>
Signed-off-by: Franck JONAS <***@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <***@ensimag.imag.fr>
Signed-off-by: Lucien KONG <***@ensimag.imag.fr>
Signed-off-by: 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

[Matthieu Moy]

Post by Huynh Khoi Nguyen NGUYEN
+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.

[n***@minatec.inpg.fr]

Post by Matthieu Moy

+test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'=

\''t' '

Post by Matthieu Moy

+ mkdir -p .config/git &&
+ >.config/git/config &&
+ rm .gitconfig &&
+ git config --global user.name "write_config" &&
+ echo "[user]" >expected &&
+ echo " name =3D 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 th=

e

Post by Matthieu Moy
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.

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <***@ensimag.imag.fr>
Signed-off-by: Franck JONAS <***@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <***@ensimag.imag.fr>
Signed-off-by: Lucien KONG <***@ensimag.imag.fr>
Signed-off-by: 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

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin DUPERRAY <***@ensimag.imag.fr>
Signed-off-by: Franck JONAS <***@ensimag.imag.fr>
Signed-off-by: Thomas NGUY <***@ensimag.imag.fr>
Signed-off-by: Lucien KONG <***@ensimag.imag.fr>
Signed-off-by: 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

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[David Aguilar]

On Wed, Jun 6, 2012 at 6:21 AM, Huynh Khoi Nguyen NGUYEN

Post by Huynh Khoi Nguyen NGUYEN
Git will be able to write to $XDG_CONFIG_HOME/git/config, a new
- 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=

=2E

Post by Huynh Khoi Nguyen NGUYEN
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/con=

fig

Post by Huynh Khoi Nguyen NGUYEN
will be used.
you shouldn't create this file.

imag.imag.fr>

Post by Huynh Khoi Nguyen NGUYEN
---

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

Post by Huynh Khoi Nguyen NGUYEN
=C2=A0Documentation/git-config.txt | =C2=A0 =C2=A03 ++-
=C2=A0builtin/config.c =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=

=A0 =C2=A05 +----

Post by Huynh Khoi Nguyen NGUYEN
=C2=A0t/t1306-xdg-files.sh =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 30 ++=

++++++++++++++++++++++++++++

Post by Huynh Khoi Nguyen NGUYEN
=C2=A03 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.=

txt

Post by Huynh Khoi Nguyen NGUYEN
index 7e344a2..a2b4786 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -97,7 +97,8 @@ OPTIONS
=C2=A0 =C2=A0 =C2=A0 =C2=A0For writing options: write to global ~/.gi=

tconfig file rather than

Post by Huynh Khoi Nguyen NGUYEN
- =C2=A0 =C2=A0 =C2=A0 the repository .git/config.
+ =C2=A0 =C2=A0 =C2=A0 the repository .git/config, write to $XDG_CONF=

IG_HOME/git/config file

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 if this file exists and ~/.gitconfig file does=

n't.

Post by Huynh Khoi Nguyen NGUYEN
=C2=A0+
=C2=A0For reading options: read only from global ~/.gitconfig and fro=

m

Post by Huynh Khoi Nguyen NGUYEN
=C2=A0$XDG_CONFIG_HOME/git/config rather than from all available file=

s.

Post by Huynh Khoi Nguyen NGUYEN
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, cons=

t char *prefix)

Post by Huynh Khoi Nguyen NGUYEN
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0home_config_pa=

ths(&user_config, &xdg_config, "config");

Post by Huynh Khoi Nguyen NGUYEN
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (access(user_co=

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

Post by Huynh Khoi Nguyen NGUYEN
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (act=

ions =3D=3D ACTION_LIST ||

Post by Huynh Khoi Nguyen NGUYEN
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

actions =3D=3D ACTION_GET_COLOR ||

Post by Huynh Khoi Nguyen NGUYEN
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

actions =3D=3D ACTION_GET_COLORBOOL))

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (access(user_co=

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

Post by Huynh Khoi Nguyen NGUYEN
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

=C2=A0 =C2=A0given_config_file =3D xdg_config;

Post by Huynh Khoi Nguyen NGUYEN
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else if (user_=

config)

Post by Huynh Khoi Nguyen NGUYEN
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=

=C2=A0 =C2=A0given_config_file =3D user_config;

Post by Huynh Khoi Nguyen NGUYEN
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 no=

n-XDG global attributes file' '

Post by Huynh Khoi Nguyen NGUYEN
=C2=A0'
+test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'\=

''t' '

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 mkdir -p .config/git &&
+ =C2=A0 =C2=A0 =C2=A0 >.config/git/config &&
+ =C2=A0 =C2=A0 =C2=A0 rm .gitconfig &&
+ =C2=A0 =C2=A0 =C2=A0 git config --global user.name "write_config" &=

&

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 echo "[user]" >expected &&
+ =C2=A0 =C2=A0 =C2=A0 echo " =C2=A0name =3D write_config" >>expected=

&&

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 test_cmp expected .config/git/config
+'
+
+
+test_expect_success 'write: xdg file exists and ~/.gitconfig exists'=

'

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 >.gitconfig &&
+ =C2=A0 =C2=A0 =C2=A0 git config --global user.name "write_gitconfig=

" &&

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 echo "[user]" >expected &&
+ =C2=A0 =C2=A0 =C2=A0 echo " =C2=A0name =3D write_gitconfig" >>expec=

ted &&

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 test_cmp expected .gitconfig
+'
+
+
+test_expect_success 'write: ~/.config/git/ exists and config file do=

esn'\''t' '

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 rm .gitconfig &&
+ =C2=A0 =C2=A0 =C2=A0 rm .config/git/config &&
+ =C2=A0 =C2=A0 =C2=A0 git config --global user.name "write_gitconfig=

" &&

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 echo "[user]" >expected &&
+ =C2=A0 =C2=A0 =C2=A0 echo " =C2=A0name =3D write_gitconfig" >>expec=

ted &&

Post by Huynh Khoi Nguyen NGUYEN
+ =C2=A0 =C2=A0 =C2=A0 test_cmp expected .gitconfig
+'
+
+
=C2=A0test_done
--
1.7.8

[Junio C Hamano]

Post by David Aguilar
I'm all for this change too.

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

Post by David Aguilar
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.

[David Aguilar]

Post by Junio C Hamano

Post by David Aguilar
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.

Post by Junio C Hamano

Post by David Aguilar
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. =C2=A0Perhaps something for compat/ in the future?
The downside to doing the same as glib is more documentation.
The upside is... consistency?

Probably. =C2=A0I think we should follow whatever existing and preval=

ent

Post by Junio C Hamano
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,
--=20
David

[Matthieu Moy]

Post by Junio C Hamano
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.

[n***@minatec.inpg.fr]

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 window=

s

users.

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

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[Junio C Hamano]

Post by Huynh Khoi Nguyen NGUYEN
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.

[Matthieu Moy]

Post by Junio C Hamano
Perhaps the problem you are solving is something like this (by the
[...]

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.

[n***@minatec.inpg.fr]

Post by Junio C Hamano
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 =20
short and we didn't state what problem we are trying to solve. Your =20
points raise the issues and explain clearly our solution.

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[Huynh Khoi Nguyen NGUYEN]

From: NGUYEN Huynh Khoi Nguyen <***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: Ramsay Jones <***@ramsay1.demon.co.uk>
Signed-off-by: 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

[Junio C Hamano]

Post by Ramsay Jones
+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?

Post by Ramsay Jones
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.

[n***@minatec.inpg.fr]

Post by Junio C Hamano

Post by Ramsay Jones
+char *mkpathdup(const char *fmt, ...)
+{
+ char path[PATH_MAX];
+ va_list args;
+ unsigned len;
+
+ va_start(args, fmt);
+ len =3D vsnprintf(path, sizeof(path), fmt, args);
+ va_end(args);
+ if (len >=3D 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 ?

[Ramsay Jones]

Post by n***@minatec.inpg.fr
=20

Post by Junio C Hamano

Post by Ramsay Jones
+char *mkpathdup(const char *fmt, ...)
+{
+ char path[PATH_MAX];
+ va_list args;
+ unsigned len;
+
+ va_start(args, fmt);
+ len =3D vsnprintf(path, sizeof(path), fmt, args);
+ va_end(args);
+ if (len >=3D 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?

=20
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 tick=
le
the cygwin bug. (I haven't actually fetched that mail yet, I've only re=
ad
it using my ISPs web-mail interface, but will do so soon and test it on
cygwin).

Thanks!

ATB,
Ramsay Jones

[n***@minatec.inpg.fr]

Post by Junio C Hamano

Post by Ramsay Jones
+char *mkpathdup(const char *fmt, ...)
+{
+ char path[PATH_MAX];
+ va_list args;
+ unsigned len;
+
+ va_start(args, fmt);
+ len =3D vsnprintf(path, sizeof(path), fmt, args);
+ va_end(args);
+ if (len >=3D 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 =3D STRBUF_INIT;
va_list args;

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

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

[Erik Faye-Lund]

Post by n***@minatec.inpg.fr

Post by Junio C Hamano

Post by Ramsay Jones
+char *mkpathdup(const char *fmt, ...)
+{
+ =A0 =A0 =A0 char path[PATH_MAX];
+ =A0 =A0 =A0 va_list args;
+ =A0 =A0 =A0 unsigned len;
+
+ =A0 =A0 =A0 va_start(args, fmt);
+ =A0 =A0 =A0 len =3D vsnprintf(path, sizeof(path), fmt, args);
+ =A0 =A0 =A0 va_end(args);
+ =A0 =A0 =A0 if (len >=3D sizeof(path))
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 return xstrdup(bad_path);
+ =A0 =A0 =A0 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, ...)
{
=A0 =A0 =A0 =A0char *path;
=A0 =A0 =A0 =A0struct strbuf sb =3D STRBUF_INIT;
=A0 =A0 =A0 =A0va_list args;
=A0 =A0 =A0 =A0va_start(args, fmt);
=A0 =A0 =A0 =A0strbuf_vaddf(&sb, fmt, args);
=A0 =A0 =A0 =A0va_end(args);
=A0 =A0 =A0 =A0path =3D sb.buf;
=A0 =A0 =A0 =A0strbuf_release(&sb);
=A0 =A0 =A0 =A0return 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 =3D sb.buf;
+ path =3D xstrdup(cleanup_path(path));

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

[n***@minatec.inpg.fr]

Post by Erik Faye-Lund

Post by Ramsay Jones
char *mkpathdup(const char *fmt, ...)
{
=A0 =A0 =A0 =A0char *path;
=A0 =A0 =A0 =A0struct strbuf sb =3D STRBUF_INIT;
=A0 =A0 =A0 =A0va_list args;
=A0 =A0 =A0 =A0va_start(args, fmt);
=A0 =A0 =A0 =A0strbuf_vaddf(&sb, fmt, args);
=A0 =A0 =A0 =A0va_end(args);
=A0 =A0 =A0 =A0path =3D sb.buf;
=A0 =A0 =A0 =A0strbuf_release(&sb);
=A0 =A0 =A0 =A0return 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
va_end(args);
- path =3D sb.buf;
+ path =3D 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 =3D sb.buf;
+ path =3D xstrdup(cleanup_path(sb.buf));

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

[Erik Faye-Lund]

Post by n***@minatec.inpg.fr

Post by Erik Faye-Lund

Post by Ramsay Jones
char *mkpathdup(const char *fmt, ...)
{
=A0 =A0 =A0 =A0char *path;
=A0 =A0 =A0 =A0struct strbuf sb =3D STRBUF_INIT;
=A0 =A0 =A0 =A0va_list args;
=A0 =A0 =A0 =A0va_start(args, fmt);
=A0 =A0 =A0 =A0strbuf_vaddf(&sb, fmt, args);
=A0 =A0 =A0 =A0va_end(args);
=A0 =A0 =A0 =A0path =3D sb.buf;
=A0 =A0 =A0 =A0strbuf_release(&sb);
=A0 =A0 =A0 =A0return 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
=A0 =A0 =A0 =A0va_end(args);
- =A0 =A0 =A0 path =3D sb.buf;
+ =A0 =A0 =A0 path =3D xstrdup(cleanup_path(path));
=A0 =A0 =A0 =A0strbuf_release(&sb);
- =A0 =A0 =A0 return xstrdup(cleanup_path(path));
+ =A0 =A0 =A0 return path;
=A0}

You are right, but I think you rather mean this, no?
=A0 =A0 =A0 =A0va_end(args);
- =A0 =A0 =A0 path =3D sb.buf;
+ =A0 =A0 =A0 path =3D xstrdup(cleanup_path(sb.buf));
=A0 =A0 =A0 =A0strbuf_release(&sb);
- =A0 =A0 =A0 return xstrdup(cleanup_path(path));
+ =A0 =A0 =A0 return path;
=A0}

Yes, sorry for the fat fingers :)

[Junio C Hamano]

Post by n***@minatec.inpg.fr
What about this ?

What about that ;-)?

Post by n***@minatec.inpg.fr
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.

[n***@minatec.inpg.fr]

Post by Junio C Hamano

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

Modulo
path =3D 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 =3D STRBUF_INIT;
va_list args;

va_start(args, fmt);
strbuf_vaddf(&sb, fmt, args);
va_end(args);
path =3D 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.

[Junio C Hamano]

Post by Ramsay Jones

Post by Junio C Hamano
Modulo
path = strbuf_detach(&sb, NULL);
that is more or less what I meant.

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.

[n***@minatec.inpg.fr]

Post by Junio C Hamano

Post by Ramsay Jones
char *mkpathdup(const char *fmt, ...)
{
char *path;
struct strbuf sb =3D STRBUF_INIT;
va_list args;
va_start(args, fmt);
strbuf_vaddf(&sb, fmt, args);
va_end(args);
path =3D 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 =3D STRBUF_INIT;
va_list args;

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

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

[Erik Faye-Lund]

Post by n***@minatec.inpg.fr

Post by Junio C Hamano

Post by Ramsay Jones
char *mkpathdup(const char *fmt, ...)
{
=A0 =A0 =A0 =A0char *path;
=A0 =A0 =A0 =A0struct strbuf sb =3D STRBUF_INIT;
=A0 =A0 =A0 =A0va_list args;
=A0 =A0 =A0 =A0va_start(args, fmt);
=A0 =A0 =A0 =A0strbuf_vaddf(&sb, fmt, args);
=A0 =A0 =A0 =A0va_end(args);
=A0 =A0 =A0 =A0path =3D strbuf_detach(&sb, NULL);
=A0 =A0 =A0 =A0strbuf_release(&sb);
=A0 =A0 =A0 =A0return path;
}

I didn't mean to suggest removing the call to clean-up-path
function. =A0What 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
char *mkpathdup(const char *fmt, ...)
{
=A0 =A0 =A0 =A0struct strbuf sb =3D STRBUF_INIT;
=A0 =A0 =A0 =A0va_list args;
=A0 =A0 =A0 =A0va_start(args, fmt);
=A0 =A0 =A0 =A0strbuf_vaddf(&sb, fmt, args);
=A0 =A0 =A0 =A0va_end(args);
=A0 =A0 =A0 =A0return 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.

[n***@minatec.inpg.fr]

Post by Erik Faye-Lund

Post by Ramsay Jones
char *mkpathdup(const char *fmt, ...)
{
=A0 =A0 =A0 =A0struct strbuf sb =3D STRBUF_INIT;
=A0 =A0 =A0 =A0va_list args;
=A0 =A0 =A0 =A0va_start(args, fmt);
=A0 =A0 =A0 =A0strbuf_vaddf(&sb, fmt, args);
=A0 =A0 =A0 =A0va_end(args);
=A0 =A0 =A0 =A0return 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 ha=

s

Post by Erik Faye-Lund
been adjusted (like cleanup_path can do) cannot be successfully fed t=

o

Post by Erik Faye-Lund
free.

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

[Erik Faye-Lund]

Post by Erik Faye-Lund

Post by Ramsay Jones
char *mkpathdup(const char *fmt, ...)
{
=A0 =A0 =A0 =A0struct strbuf sb =3D STRBUF_INIT;
=A0 =A0 =A0 =A0va_list args;
=A0 =A0 =A0 =A0va_start(args, fmt);
=A0 =A0 =A0 =A0strbuf_vaddf(&sb, fmt, args);
=A0 =A0 =A0 =A0va_end(args);
=A0 =A0 =A0 =A0return 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 h=

as

Post by Erik Faye-Lund
been adjusted (like cleanup_path can do) cannot be successfully fed =

to

Post by Erik Faye-Lund
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.

[Junio C Hamano]

Post by Erik Faye-Lund
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.

[n***@minatec.inpg.fr]

Post by Junio C Hamano

Post by Erik Faye-Lund
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 h=

as

Post by Junio C Hamano

Post by Erik Faye-Lund
been adjusted (like cleanup_path can do) cannot be successfully fed =

to

Post by Junio C Hamano

Post by Erik Faye-Lund
free.

Yeah, I wouldn't recommend doing that. Either
path =3D strbuf_detach(&sb, NULL);
retval =3D xstrdup(cleanup_path(path));
free(path);
return retval;
or
path =3D 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.

[n***@minatec.inpg.fr]

Post by Junio C Hamano

Post by Erik Faye-Lund
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

Post by Junio C Hamano

Post by Erik Faye-Lund
been adjusted (like cleanup_path can do) cannot be successfully fed=

to

Post by Junio C Hamano

Post by Erik Faye-Lund
free.

Yeah, I wouldn't recommend doing that. Either
path =3D strbuf_detach(&sb, NULL);
retval =3D xstrdup(cleanup_path(path));
free(path);
return retval;
or
path =3D 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 =3D STRBUF_INIT;
va_list args;

va_start(args, fmt);
strbuf_vaddf(&sb, fmt, args);
va_end(args);
path =3D 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.

[Erik Faye-Lund]

Post by n***@minatec.inpg.fr

Post by Erik Faye-Lund
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

Post by n***@minatec.inpg.fr

Post by Erik Faye-Lund
been adjusted (like cleanup_path can do) cannot be successfully fed=

to

Post by n***@minatec.inpg.fr

Post by Erik Faye-Lund
free.

Yeah, I wouldn't recommend doing that. =A0Either
=A0 =A0 =A0 =A0path =3D strbuf_detach(&sb, NULL);
=A0 =A0 =A0 =A0retval =3D xstrdup(cleanup_path(path));
=A0 =A0 =A0 =A0free(path);
=A0 =A0 =A0 =A0return retval;
or
=A0 =A0 =A0 =A0path =3D xstrdup(cleanup_path(sb.buf));
=A0 =A0 =A0 =A0strbuf_release(&sb);
=A0 =A0 =A0 =A0return 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 :)

[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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[Ramsay Jones]

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

[...]

Post by Huynh Khoi Nguyen NGUYEN
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

[Matthieu Moy]

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

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,

[Junio C Hamano]

Post by Matthieu Moy

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

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.

[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

[Matthieu Moy]

From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[Matthieu Moy]

From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[Junio C Hamano]

Post by Huynh Khoi Nguyen NGUYEN
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.

[Matthieu Moy]

Post by Junio C Hamano

Post by Huynh Khoi Nguyen NGUYEN
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?

Post by Junio C Hamano
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.

[Junio C Hamano]

Post by Huynh Khoi Nguyen NGUYEN
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
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.

[Matthieu Moy]

From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[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

Post by Huynh Khoi Nguyen NGUYEN
+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;

[...]

Post by Huynh Khoi Nguyen NGUYEN
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

Post by Huynh Khoi Nguyen NGUYEN
@@ -379,13 +382,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)

[...]

Post by Huynh Khoi Nguyen NGUYEN
+ 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?

[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 <***@imag.fr>
---

Post by Thomas Rast
This patch causes valgrind warnings in t1300.81 (get --path copes with

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;
}

[Thomas Rast]

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

Post by Thomas Rast
This patch causes valgrind warnings in t1300.81 (get --path copes with

Indeed. The following patch should fix it.

Thanks!

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

[Junio C Hamano]

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

Post by Thomas Rast
This patch causes valgrind warnings in t1300.81 (get --path copes with

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.

Post by Matthieu Moy
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;
}

[Matthieu Moy]

Post by Junio C Hamano

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

Post by Thomas Rast
This patch causes valgrind warnings in t1300.81 (get --path copes with

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.

[Junio C Hamano]

Post by Matthieu Moy

Post by Junio C Hamano

Post by Matthieu Moy
+ 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
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?

[Matthieu Moy]

Post by Junio C Hamano
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]

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 <***@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;
}

[Jeff King]

Post by Matthieu Moy

Post by Junio C Hamano
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

[Matthieu Moy]

Post by Jeff King

Post by Matthieu Moy

Post by Junio C Hamano
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).

[Thomas Rast]

Post by Matthieu Moy

Post by Jeff King

Post by Matthieu Moy

Post by Junio C Hamano
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....

[Matthieu Moy]

Post by Thomas Rast
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");
}
}

[Junio C Hamano]

Post by Huynh Khoi Nguyen NGUYEN
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;

[Matthieu Moy]

Post by Huynh Khoi Nguyen NGUYEN
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.

[Junio C Hamano]

Post by Matthieu Moy

Post by Huynh Khoi Nguyen NGUYEN
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();

[Matthieu Moy]

Post by Junio C Hamano
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).

Post by Junio C Hamano
- 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;

[Junio C Hamano]

Post by Matthieu Moy

Post by Junio C Hamano
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.

[Matthieu Moy]

From: Huynh Khoi Nguyen Nguyen <Huynh-Khoi-***@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-***@ensimag.imag.fr>
Signed-off-by: Valentin Duperray <***@ensimag.imag.fr>
Signed-off-by: Franck Jonas <***@ensimag.imag.fr>
Signed-off-by: Lucien Kong <***@ensimag.imag.fr>
Signed-off-by: Thomas Nguy <***@ensimag.imag.fr>
Signed-off-by: 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

[Junio C Hamano]

Post by Matthieu Moy
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-***@ensimag.imag.fr>
...
Signed-off-by: Junio C Hamano <***@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?

[Matthieu Moy]

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

Nice catch.

Post by Junio C Hamano
Given the root cause of that "something funny", I'll reduce the log
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.
...

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

Post by Junio C Hamano
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)

[Junio C Hamano]

Post by Matthieu Moy
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
--- 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.

[Matthieu Moy]

Post by Junio C Hamano
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

;-)

[Junio C Hamano]

Post by Matthieu Moy
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.

Post by Matthieu Moy
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.
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

[Ramsay Jones]

Post by Huynh Khoi Nguyen NGUYEN
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.
---
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

[...]

Post by Huynh Khoi Nguyen NGUYEN
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 ?

[...]

Post by Huynh Khoi Nguyen NGUYEN
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?

[...]

Post by Huynh Khoi Nguyen NGUYEN
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 ;-)

Post by Huynh Khoi Nguyen NGUYEN
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 <***@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 <***@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);
}

[Junio C Hamano]

Post by Ramsay Jones
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().

[Ramsay Jones]

Post by Junio C Hamano

Post by Ramsay Jones
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

[n***@minatec.inpg.fr]

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 =20
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 =20
this patch).
(Note that it does not address the above issues).
HTH
ATB,
Ramsay Jones
-- >8 --
Subject: [PATCH] path.c: Fix a static buffer overwrite bug by =20
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 overwrit=

e

bug by avoiding mkpath()", 19-11-2011). Note that the patch refactore=

d

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 ...)
---
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, =20
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 signatur=

e */

extern char *mkpath(const char *fmt, ...) __attribute__((format =20
(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 =3D vsnprintf(path, sizeof(path), fmt, args);
+ va_end(args);
+ if (len >=3D 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 =20
**xdg, char *file)
*global =3D NULL;
} else {
if (!xdg_home) {
- to_free =3D strdup(mkpath("%s/.config", home));
+ to_free =3D mkpathdup("%s/.config", home);
xdg_home =3D to_free;
}
if (global)
- *global =3D xstrdup(mkpath("%s/.gitconfig", home));
+ *global =3D mkpathdup("%s/.gitconfig", home);
}
if (!xdg_home)
*xdg =3D NULL;
else
- *xdg =3D xstrdup(mkpath("%s/git/%s", xdg_home, file));
+ *xdg =3D 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 =20
patch, we will add your modifications in our next v6 version.

[Matthieu Moy]

Post by Huynh Khoi Nguyen NGUYEN
+ Second user-specific configuration file. ~/.gitconfig has priority.

You should mention the behavior if $XDG_CONFIG_HOME is not set.

Post by Huynh Khoi Nguyen NGUYEN
+ 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));

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.

[n***@minatec.inpg.fr]

Post by Junio C Hamano
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.

Post by Junio C Hamano
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.

Post by Junio C Hamano
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.
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.

[n***@minatec.inpg.fr]

Post by Ramsay Jones
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()",

Post by Ramsay Jones
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).

Post by Ramsay Jones
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) ?

[Ramsay Jones]

Post by n***@minatec.inpg.fr

Post by Ramsay Jones
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()",

Post by Ramsay Jones
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).

Post by Ramsay Jones
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

[n***@minatec.inpg.fr]

Post by Junio C Hamano
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);
}
...
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);

We adapted this code to allow reuse and introduce default values to
core.excludesfile and core.attributesfile in the XDG directory too,
in our V4 version.