Skip to content

config: read both home and xdg files for --global #1938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

config: read both home and xdg files for --global #1938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f681918
config: test home and xdg files in `list --global`
delilahw May 22, 2025
c97026c
config: read from config sequence for global scope
delilahw May 22, 2025
a44ef7c
config: keep bailing on unreadable global files
delilahw May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions builtin/config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,18 @@ static void location_options_init(struct config_location_options *opts,
}

if (opts->use_global_config) {
/*
* Reading config is handled by `config.c#do_git_config_sequence()`
* because the global config can be sourced from multiple files.
* Writing config should point to a single destination as set in
* `opts->source.file` below.
*/
opts->options.ignore_repo = 1;
opts->options.ignore_cmdline= 1;
opts->options.ignore_worktree = 1;
opts->options.ignore_system = 1;
opts->source.scope = CONFIG_SCOPE_GLOBAL;

opts->source.file = opts->file_to_free = git_global_config();
if (!opts->source.file)
/*
Expand Down
53 changes: 39 additions & 14 deletions config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2014,11 +2014,13 @@ int git_config_system(void)
}

static int do_git_config_sequence(const struct config_options *opts,
const struct repository *repo,
config_fn_t fn, void *data)
const struct repository *repo, config_fn_t fn,
void *data, enum config_scope scope)
{
int ret = 0;
char *system_config = git_system_config();
int global_config_success_count = 0;
int nonzero_ret_on_global_config_error = scope == CONFIG_SCOPE_GLOBAL;
char *xdg_config = NULL;
char *user_config = NULL;
char *repo_config;
Expand All @@ -2040,22 +2042,43 @@ static int do_git_config_sequence(const struct config_options *opts,
worktree_config = NULL;
}

if (git_config_system() && system_config &&
if (!opts->ignore_system && git_config_system() && system_config &&
!access_or_die(system_config, R_OK,
opts->system_gently ? ACCESS_EACCES_OK : 0))
ret += git_config_from_file_with_options(fn, system_config,
data, CONFIG_SCOPE_SYSTEM,
NULL);

git_global_config_paths(&user_config, &xdg_config);
if (!opts->ignore_global) {
git_global_config_paths(&user_config, &xdg_config);

if (xdg_config &&
!access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) {
ret += git_config_from_file_with_options(fn, xdg_config,
data,
CONFIG_SCOPE_GLOBAL,
NULL);
if (nonzero_ret_on_global_config_error && !ret)
++global_config_success_count;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: on its own you'd probably want the postfix version... global_config_success_count++;. I think the prefix version is only used when the return value matters.

}

if (user_config &&
!access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) {
ret += git_config_from_file_with_options(fn, user_config,
data,
CONFIG_SCOPE_GLOBAL,
NULL);
if (nonzero_ret_on_global_config_error && !ret)
++global_config_success_count;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you apply the previous nit, then you'll want to match it here.

}

if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))
ret += git_config_from_file_with_options(fn, xdg_config, data,
CONFIG_SCOPE_GLOBAL, NULL);
if (nonzero_ret_on_global_config_error &&
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that you are changing behavior in this patch, you'll want to demonstrate that change in behavior with a test update. If you follow my recommendation to use test_must_fail in the first patch, this could be as simple as removing that indicator here.

!global_config_success_count)
--ret;

if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))
ret += git_config_from_file_with_options(fn, user_config, data,
CONFIG_SCOPE_GLOBAL, NULL);
free(xdg_config);
free(user_config);
}

if (!opts->ignore_repo && repo_config &&
!access_or_die(repo_config, R_OK, 0))
Expand All @@ -2074,8 +2097,6 @@ static int do_git_config_sequence(const struct config_options *opts,
die(_("unable to parse command-line config"));

free(system_config);
free(xdg_config);
free(user_config);
free(repo_config);
free(worktree_config);
return ret;
Expand Down Expand Up @@ -2105,15 +2126,19 @@ int config_with_options(config_fn_t fn, void *data,
*/
if (config_source && config_source->use_stdin) {
ret = git_config_from_stdin(fn, data, config_source->scope);
} else if (config_source && config_source->file) {
} else if (config_source && config_source->file &&
config_source->scope != CONFIG_SCOPE_GLOBAL) {
ret = git_config_from_file_with_options(fn, config_source->file,
data, config_source->scope,
NULL);
} else if (config_source && config_source->blob) {
ret = git_config_from_blob_ref(fn, repo, config_source->blob,
data, config_source->scope);
} else {
ret = do_git_config_sequence(opts, repo, fn, data);
ret = do_git_config_sequence(opts, repo, fn, data,
config_source ?
config_source->scope :
CONFIG_SCOPE_UNKNOWN);
}

if (inc.remote_urls) {
Expand Down
2 changes: 2 additions & 0 deletions config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ typedef int (*config_parser_event_fn_t)(enum config_event_t type,

struct config_options {
unsigned int respect_includes : 1;
unsigned int ignore_system : 1;
unsigned int ignore_global : 1;
unsigned int ignore_repo : 1;
unsigned int ignore_worktree : 1;
unsigned int ignore_cmdline : 1;
Expand Down
65 changes: 65 additions & 0 deletions t/t1300-config.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2367,6 +2367,71 @@ test_expect_success '--show-scope with --default' '
test_cmp expect actual
'

test_expect_success 'list with nonexistent global config' '
rm -rf "$HOME"/.gitconfig "$HOME"/.config/git/config &&
git config ${mode_prefix}list --show-scope
'

test_expect_success 'list --global with nonexistent global config' '
rm -rf "$HOME"/.gitconfig "$HOME"/.config/git/config &&
test_must_fail git config ${mode_prefix}list --global --show-scope
'

test_expect_success 'list --global with only home' '
rm -rf "$HOME"/.config/git/config &&

test_when_finished rm -f \"\$HOME\"/.gitconfig &&
cat >"$HOME"/.gitconfig <<-EOF &&
[home]
config = true
EOF

cat >expect <<-EOF &&
global home.config=true
EOF
git config ${mode_prefix}list --global --show-scope >output &&
test_cmp expect output
'

test_expect_success 'list --global with only xdg' '
rm -f "$HOME"/.gitconfig &&

test_when_finished rm -rf \"\$HOME\"/.config/git &&
mkdir -p "$HOME"/.config/git &&
cat >"$HOME"/.config/git/config <<-EOF &&
[xdg]
config = true
EOF

cat >expect <<-EOF &&
global xdg.config=true
EOF
git config ${mode_prefix}list --global --show-scope >output &&
test_cmp expect output
'

test_expect_success 'list --global with both home and xdg' '
test_when_finished rm -f \"\$HOME\"/.gitconfig &&
cat >"$HOME"/.gitconfig <<-EOF &&
[home]
config = true
EOF

test_when_finished rm -rf \"\$HOME\"/.config/git &&
mkdir -p "$HOME"/.config/git &&
cat >"$HOME"/.config/git/config <<-EOF &&
[xdg]
config = true
EOF

cat >expect <<-EOF &&
global xdg.config=true
global home.config=true
EOF
git config ${mode_prefix}list --global --show-scope >output &&
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--show-scope is valuable data to include so we see that both sources are included as global scope. Could you also add --show-origin to demonstrate that we can differentiate the two files? This will be particularly important for users who are trying to figure out why a behavior change occurred (if they had both files already but one was ignored by previous behavior).

test_cmp expect output
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of this patch, this test will fail, correct? It would be good to be clear about what step is failing here. Something like:

test_must_fail test_cmp expect output

This demonstrates that the test is documenting the expected behavior and how the current implementation fails to achieve that result (it doesn't fail in the git config command, but its output is wrong).

Later, you can remove the test_must_fail part to demonstrate that the test starts succeeding.

This is in contrast to using test_expect_failure since that only cares that something about the test failed, which is no longer preferred.

'

test_expect_success 'override global and system config' '
test_when_finished rm -f \"\$HOME\"/.gitconfig &&
cat >"$HOME"/.gitconfig <<-EOF &&
Expand Down
3 changes: 2 additions & 1 deletion t/t1306-xdg-files.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ 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 &&
echo user.name=read_config >expected &&
echo user.name=read_gitconfig >>expected &&
git config --global --list >actual &&
test_cmp expected actual
'
Expand Down
Loading