config: read both home and xdg files for --global#1938
Conversation
Welcome to GitGitGadgetHi @delilahw, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests. Please make sure that either:
You can CC potential reviewers by adding a footer to the PR description with the following syntax: NOTE: DO NOT copy/paste your CC list from a previous GGG PR's description, Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:
It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code. Contributing the patchesBefore you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form Both the person who commented An alternative is the channel Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment If you want to see what email(s) would be sent for a After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail). If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the curl -g --user "<EMailAddress>:<Password>" \ --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txtTo iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description): To send a new iteration, just add another PR comment with the contents: Need help?New contributors who want advice are encouraged to join [email protected], where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join. You may also be able to find help in real time in the developer IRC channel, |
There are issues in commit 3a615ee: |
delilahwforce-pushed the lilah/fix-config-list-global-home-and-xdg/patchset branch 4 times, most recently from 3f97f81 to a44ef7cCompare
dscho commented Jun 28, 2025
/allow |
User delilahw is now allowed to use GitGitGadget. |
derrickstolee left a comment
derrickstolee left a comment There was a problem hiding this comment.
Your code looks pretty good. I haven't validated it locally, but I only have a few stylistic things to recommend and some extra testing to try. Feel free to submit when you're ready.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
delilahwforce-pushed the lilah/fix-config-list-global-home-and-xdg/patchset branch 3 times, most recently from aed07f9 to 5b253d6Compare
delilahw commented Sep 15, 2025
naauurrr the |
delilahwforce-pushed the lilah/fix-config-list-global-home-and-xdg/patchset branch from 5b253d6 to 6d205b9Compare
delilahwforce-pushed the lilah/fix-config-list-global-home-and-xdg/patchset branch 2 times, most recently from 8fd6c76 to 85c6914Comparedelilahw commented Sep 25, 2025
/preview |
Preview email sent as [email protected] |
Git prefers forward slashes as directory separators across all platforms. On Windows, the backslash is the native directory separator, but all Windows versions supported by Git also accept the forward slash in all but rare circumstances. Our tests expect forward slashes. Git generates relative paths with forward slashes. Forward slashes are more convenient to use in shell scripts. For these reasons, we enforced forward slashes in `interpolate_path()` in 5ca6b7b (config --show-origin: report paths with forward slashes, 2016-03-23). However, other code paths may generate paths containing backslashes. For example, `config --show-origin` prints the XDG config path with mixed slashes on Windows: $ git config --list --show-origin file:C:/Program Files/Git/etc/gitconfig system.foo=bar file:"C:\\Users\\delilah/.config/git/config" xdg.foo=bar file:C:/Users/delilah/.gitconfig home.foo=bar file:.git/config local.foo=bar Let's enforce forward slashes in all code paths that directly or indirectly call `cleanup_path()` by modifying it to use `convert_slashes()` on Windows. Since `convert_slashes()` modifies the path in-place, change the argument and return type of `cleanup_path()` from `const char *` to `char *`. All existing callers of `cleanup_path()` pass `char *` anyways, so this change is compatible. The next patch, config: test home and xdg files in `list --global`, will assert that the XDG config path uses forward slashes. Suggested-by: Johannes Schindelin <[email protected]> Signed-off-by: Delilah Ashley Wu <[email protected]> Reviewed-by: Johannes Schindelin <[email protected]>
The `git config list --global` output includes `$HOME/.gitconfig` (home config), but ignores `$XDG_CONFIG_HOME/git/config` (XDG config). It should include both files. Modify tests to check the following and expect a failure: - `git config list --global` should include contents from both the home and XDG config locations (assuming they are readable), not just the former. - `--show-origin` should print correct paths to both config files, assuming they exist. Also, add tests to ensure subsequent patches do not introduce regressions to `git config list`. Specifically, check that: - The home config should take precedence over the XDG config. - Without `--global`, it should not bail on unreadable/non-existent global config files. - With `--global`, it should bail when both `$HOME/.gitconfig` and `$XDG_CONFIG_HOME/git/config` are unreadable. It should not bail if at least one of them is readable. The next patch, config: read global scope via config_sequence, will implement a fix to include both config files when `--global` is specified. Reported-by: Jade Lovelace <[email protected]> Helped-by: Derrick Stolee <[email protected]> Signed-off-by: Delilah Ashley Wu <[email protected]> Reviewed-by: Johannes Schindelin <[email protected]>
The output of `git config list --global` should include both the home (`$HOME/.gitconfig`) and XDG (`$XDG_CONFIG_HOME/git/config`) configs, but it only reads from the former. We assumed each config scope corresponds to a single config file. Under this assumption, `git config list --global` reads the global config by calling `git_config_from_file_with_options(...,"~/.gitconfig", ...)`. This function usage restricts us to a single config file. Because the global scope includes two files, we should read the configs via another method. The output of `git config list --show-scope --show-origin` (without `--global`) correctly includes both the home and XDG config files. So there's existing code that respects both locations, namely the `do_git_config_sequence()` function which reads from all scopes. Introduce flags to make it possible to ignore all but the global scope (i.e. ignore system, local, worktree, and cmdline). Then, reuse the function to read only the global scope when `--global` is specified. This was the suggested solution in the bug report: https://lore.kernel.org/git/[email protected]. Then, modify the tests to check that `git config list --global` includes both home and XDG configs. This patch introduces a regression. If both global config files are unreadable, then `git config list --global` should exit non-zero. This is no longer the case, so mark the corresponding test as a "TODO known breakage" and address the issue in the next patch, config: keep bailing on unreadable global files. Implementation notes: 1. The `ignore_global` flag is not set anywhere, so the `if (!opts->ignore_global)` condition is always met. We can remove this flag if desired. 2. I've assumed that `config_source->scope == CONFIG_SCOPE_GLOBAL` iff `--global` is specified. This comparison determines whether to call `do_git_config_sequence()` for the global scope, or to keep calling `git_config_from_file_with_options()` for other scopes. 3. Keep populating `opts->source.file` in `builtin/config.c` because it is used as the destination config file for write operations. The proposed changes could convolute the code because there is no single source of truth for the config file locations in the global scope. Add a comment to help clarify this. Please let me know if it's unclear. Reported-by: Jade Lovelace <[email protected]> Suggested-by: Glen Choo <[email protected]> Helped-by: Derrick Stolee <[email protected]> Signed-off-by: Delilah Ashley Wu <[email protected]> Reviewed-by: Johannes Schindelin <[email protected]>
The expected behaviour for `git config list` is: A. Without `--global`, it should not bail on unreadable/non-existent global config files. B. With `--global`, it should bail when both `$HOME/.gitconfig` and `$XDG_CONFIG_HOME/git/config` are unreadable. It should not bail when one or more of them is readable. The previous patch, config: read global scope via config_sequence, introduced a regression in scenario B. When both global config files are unreadable, running `git config list --global` would not fail. For example, `GIT_CONFIG_GLOBAL=does-not-exist git config list --global` exits with status code 0. Assuming that `config_source->scope == CONFIG_SCOPE_GLOBAL` iff the `--global` argument is specified, use this to determine whether to bail. When reading only the global scope and both config files are unreadable, then adjust the return code to be non-zero. Note: When bailing, the exit code is not determined by sum of the return codes of the underlying operations. Instead, the exit code is modified via a single decrement. If this is undesirable, we can change it to sum the return codes of the underlying operations instead. Lastly, modify the tests to remove the known breakage/regression. The tests for scenario B will now pass. Helped-by: Derrick Stolee <[email protected]> Signed-off-by: Delilah Ashley Wu <[email protected]> Reviewed-by: Johannes Schindelin <[email protected]>
delilahwforce-pushed the lilah/fix-config-list-global-home-and-xdg/patchset branch from 85c6914 to 6119ceeComparedelilahw commented Sep 25, 2025
/preview |
Preview email sent as [email protected] |
delilahw commented Oct 10, 2025
/preview |
Preview email sent as [email protected] |
This patch series was integrated into seen via git@c398ad3. |
This patch series was integrated into seen via git@406c8e7. |
On the Git mailing list, Delilah Ashley Wu wrote (reply to this): On Fri, Oct 10, 2025 at 03:27:24AM +0200, Kristoffer Haugsbakk wrote: > On Fri, Oct 10, 2025, at 03:14, Delilah Ashley Wu via GitGitGadget wrote:> > As reported in [1]: `$HOME/.gitconfig` and `$XDG_CONFIG_HOME/git/config` are> > both valid global config locations, but `git config list --global` only> > includes the former in its output.>> Note only if both files exist. Thanks for the clarification, I'll be sure to note this in my v2 cover letter and commit messages. Delilah =) |
On the Git mailing list, Delilah Ashley Wu wrote (reply to this): On Wed, Nov 19, 2025 at 06:44:25AM -0800, Junio C Hamano wrote: > "Delilah Ashley Wu via GitGitGadget" <[email protected]>> writes:>> > As reported in [1]: `$HOME/.gitconfig` and `$XDG_CONFIG_HOME/git/config` are> > both valid global config locations, but `git config list --global` only> > includes the former in its output.>> ... while "git config list" includes both, which is an inconsistency> without good reason. Good point! Will add above to the v2 cover letter. > What is the reason behind [RFC] in the title? Are there things that> are iffy yourself in the patches that reviewers want to pay special> attention to? There wasn't any reason; I accidentally left the GitHub PR in draft mode when I submitted it. I'll drop the [RFC] in v2. And thanks for the review! You covered the points that I also felt iffy about, e.g. introducing a regression in the middle of the patch series. I'll address your feedback in v2. Delilah :) |
This patch series was integrated into seen via git@81c37de. |
There was a status update in the "Cooking" section about the branch "git config --list --global", unlike "git config --list", did not consult both of the two possible per-user sources of the configuration files, i.e. $HOME/.gitconfig and the XDG one, which has been corrected. Comments? source: <[email protected]> |
This patch series was integrated into seen via git@6cba3bd. |
This patch series was integrated into seen via git@32cdc67. |
This patch series was integrated into seen via git@f1e75dc. |
There was a status update in the "Cooking" section about the branch "git config --list --global", unlike "git config --list", did not consult both of the two possible per-user sources of the configuration files, i.e. $HOME/.gitconfig and the XDG one, which has been corrected. Expecting a reroll. cf. <20251122020047.GB3947@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> source: <[email protected]> |
This patch series was integrated into seen via git@5fb9ba7. |
This patch series was integrated into seen via git@f5fc135. |
There was a status update in the "Cooking" section about the branch "git config --list --global", unlike "git config --list", did not consult both of the two possible per-user sources of the configuration files, i.e. $HOME/.gitconfig and the XDG one, which has been corrected. Expecting a reroll. cf. <20251122020047.GB3947@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> source: <[email protected]> |
This patch series was integrated into seen via git@fff66a4. |
This patch series was integrated into seen via git@be85718. |
There was a status update in the "Cooking" section about the branch "git config --list --global", unlike "git config --list", did not consult both of the two possible per-user sources of the configuration files, i.e. $HOME/.gitconfig and the XDG one, which has been corrected. Expecting a reroll. cf. <20251122020047.GB3947@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> source: <[email protected]> |
This patch series was integrated into seen via git@c598315. |
This patch series was integrated into seen via git@e38e605. |
There was a status update in the "Cooking" section about the branch "git config --list --global", unlike "git config --list", did not consult both of the two possible per-user sources of the configuration files, i.e. $HOME/.gitconfig and the XDG one, which has been corrected. Expecting a reroll. cf. <20251122020047.GB3947@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> source: <[email protected]> |
This patch series was integrated into seen via git@e0e409a. |
This patch series was integrated into seen via git@08a89bb. |
There was a status update in the "Cooking" section about the branch "git config --list --global", unlike "git config --list", did not consult both of the two possible per-user sources of the configuration files, i.e. $HOME/.gitconfig and the XDG one, which has been corrected. Expecting a reroll. cf. <20251122020047.GB3947@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> source: <[email protected]> |
This patch series was integrated into seen via git@3e3843b. |
This patch series was integrated into seen via git@02f6c1f. |
This patch series was integrated into seen via git@d622ec5. |
This patch series was integrated into seen via git@9f9c058. |
There was a status update in the "Cooking" section about the branch "git config --list --global", unlike "git config --list", did not consult both of the two possible per-user sources of the configuration files, i.e. $HOME/.gitconfig and the XDG one, which has been corrected. Expecting a reroll. cf. <20251122020047.GB3947@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> source: <[email protected]> |
This patch series was integrated into seen via git@191d10a. |
This patch series was integrated into seen via git@aa02058. |
This patch series was integrated into seen via git@9c93da1. |
3 participants
Hi!
As reported in [1]: `$HOME/.gitconfig` and `$XDG_CONFIG_HOME/git/config` are both valid global config locations, but `git config list --global` only includes the former in its output.
Suppose we have this config in `$HOME/.gitconfig`:
And this config in `$XDG_CONFIG_HOME/git/config`:
Then, to reproduce the issue that `--global` only shows the home config:
Git correctly applies the XDG config in its effective configuration, but it doesn't show up when `--global` is specified. We can confirm this by checking the output without the `--global` flag:
The expected behaviour is both configs should be shown when `--global` is specified, so we'd expect its output to look the same as above. This was confirmed in [2], which quoted the `git config` documentation:
The first patch fixes forward slash normalisation on Windows paths. The second patch introduces tests and regression checks. The third and fourth patches implement the fix to include both config files when `--global` is specified. Johannes has kindly pre-reviewed this patch series via GitHub on GitGitGadget #1938 [3]. You'll notice some force-pushes after the review, but I only changed commit messages.
[1]: https://lore.kernel.org/git/CAFA9we-QLQRzJdGMMCPatmfrk1oHeiUu9msMRXXk1MLE5HRxBQ@mail.gmail.com/
[2]: https://lore.kernel.org/git/[email protected]/
[3]: #1938
Thank you all for your time!
Delilah
cc: Delilah Ashley Wu [email protected]
cc: Derrick Stolee [email protected]
cc: Johannes Schindelin [email protected]
cc: Patrick Steinhardt [email protected]
cc: "Kristoffer Haugsbakk" [email protected]