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

Changes from 1 commit
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
39 changes: 30 additions & 9 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 Down Expand Up @@ -2050,13 +2052,29 @@ static int do_git_config_sequence(const struct config_options *opts,
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 (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 (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 &&
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;

free(xdg_config);
free(user_config);
Expand Down Expand Up @@ -2117,7 +2135,10 @@ int config_with_options(config_fn_t fn, void *data,
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
Loading