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`:

[home]
    config = true

And this config in `$XDG_CONFIG_HOME/git/config`:

[xdg]
    config = true

Then, to reproduce the issue that `--global` only shows the home config:

$ git config list --global --show-scope --show-origin global file:/Users/delilah/.gitconfig home.config=true

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:

$ git config list --show-scope --show-origin global file:/Users/delilah/.config/git/config xdg.config=true global file:/Users/delilah/.gitconfig home.config=true

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/xmqqmt5lezi3.fsf@gitster.g/ [3]: https://github.com/gitgitgadget/git/pull/1938/

Thank you all for your time! Delilah

Delilah Ashley Wu (4):
  cleanup_path: force forward slashes on Windows
  config: test home and xdg files in `list --global`
  config: read global scope via config_sequence
  config: keep bailing on unreadable global files

 builtin/config.c     | 12 ++++++++
 config.c             | 54 ++++++++++++++++++++++++++----------
 config.h             |  2 ++
 path.c               | 10 +++++--
 t/t1300-config.sh    | 65 ++++++++++++++++++++++++++++++++++++++++++++
 t/t1306-xdg-files.sh |  3 +-
 6 files changed, 128 insertions(+), 18 deletions(-)

base-commit: ca2559c1d630eb4f04cdee2328aaf1c768907a9e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1938%2Fdelilahw%2Flilah%2Ffix-config-list-global-home-and-xdg%2Fpatchset-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1938/delilahw/lilah/fix-config-list-global-home-and-xdg/patchset-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1938

From: Delilah Ashley Wu <delilahwu@microsoft.com>

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 5ca6b7bb47b (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 <johannes.schindelin@gmx.de>
Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 path.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

From: Delilah Ashley Wu <delilahwu@microsoft.com>

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 <lists@jade.fyi>
Helped-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t1300-config.sh    | 65 ++++++++++++++++++++++++++++++++++++++++++++
 t/t1306-xdg-files.sh |  5 ++--
 2 files changed, 68 insertions(+), 2 deletions(-)

From: Delilah Ashley Wu <delilahwu@microsoft.com>

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/kl6ly1oze7wb.fsf@chooglen-macbookpro.roam.corp.google.com.

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 <lists@jade.fyi>
Suggested-by: Glen Choo <glencbz@gmail.com>
Helped-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/config.c     | 12 ++++++++++++
 config.c             | 26 +++++++++++++++-----------
 config.h             |  2 ++
 t/t1300-config.sh    |  6 +++---
 t/t1306-xdg-files.sh |  2 +-
 5 files changed, 33 insertions(+), 15 deletions(-)

From: Delilah Ashley Wu <delilahwu@microsoft.com>

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 <stolee@gmail.com>
Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 config.c          | 40 +++++++++++++++++++++++++++++++---------
 t/t1300-config.sh |  4 ++--
 2 files changed, 33 insertions(+), 11 deletions(-)

On Fri, Oct 10, 2025, at 03:14, Delilah Ashley Wu via GitGitGadget wrote:

Note only if both files exist.

Hi,

On Fri, 10 Oct 2025, Delilah Ashley Wu via GitGitGadget wrote:

For the record, my "Reviewed-by:" still stands, if lack of reviews should be the reason why this patch series has not even entered the `seen` branch.

Ciao, Johannes

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Thanks for pinging.

"Why is it not in 'next'" is a legitimate question. I think that is because the topic has no discussion on the list in the thread.

"Why is it in 'seen'" is a question with no answer. As I often say, 'seen' is merely what I happened to have seen and found it promising but is not ready for 'next', and people should not read anything more into it.

I didn't look at it primarily because nobody, not even one on a handful of experienced contributors whose opinions are well regarded in the community on the CC: list, responded to the thread at all. Before the message I am responding to, that is ;-)

I wanted to see how well people receive the motivation behind the proposed change, as I vaguely recalled that not using both at the same time was deliberate to help those who migrate from historical location to XDG layout, but did not have time and energy to do the digging myself to become knowledgeable again to give any comment worth reading.

"Delilah Ashley Wu via GitGitGadget" <gitgitgadget@gmail.com> writes:

... while "git config list" includes both, which is an inconsistency without good reason.

Thanks for addressing this issue. I haven't had a chance to look at these patches yet, but both analysis and Glen's outline for the best approach presented in the thread [1] do look very sensible.

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?

Thanks.

"Delilah Ashley Wu via GitGitGadget" <gitgitgadget@gmail.com> writes:

Not just compatible ;-). If there is a caller that wants cleanup_path() not to munge what it passes, this change will introduce a bug for them. Have you made sure that none of these callers mind that backslashes are converted into forward slashes?

The path to the leaf-level blobs is always slash separated in the index, a tree object sorts an entry that points at a subtree as if its path component has terminating slash, etc., and only when these paths are externalized, they are converted to filesystem dependent hierarchy separator (by system call like creat(2) even on platforms like Windows whose filesystem uses backslashes as the pathname separator). Canonicalizing end-user supplied path early at a central place does make sense.

Hmph, the need for cast is a bit annoying, but more importantly, why don't we have to worry about leading ".\\\\" instead of ".////"? Shouldn't we be stripping backslashes the same way on Windows?

In other words, why do it here, not _before_ the loop that says "If the path begins with dot (i.e. the thing is relative to the current directory) followed by a directory separator, remove it together with any extra directory separators that come immediately after it"?

"Delilah Ashley Wu via GitGitGadget" <gitgitgadget@gmail.com> writes:

Please be gentle to future readers of "git log" and help them with a bit more explanation on the "should" here. E.g.,

    should include both files, to be consistent with the output from
    `git config list` (not limited to `--global`) that lists entries
    from both files (in addition to system-wide and repository-specific
    entries, of course).

or something.

Testing these two combinations is a good thing, but "expect a failure"? There doesn't seem to be any test that is marked as "test_expect_failure" in this patch. Confused?

    Side note: we generally do not want test_expect_failure tests in
    one patch, followed by a code fix with changes to tests that
    flip s/test_expect_failure/test_expect_success/' in another
    patch, though.  The reason is primarily that such a two-patch
    series makes it harder to review the step that has the fix, by
    hiding the body of the test whose earlier failure gets fixed by
    the code change.

Good.

Do we expect an empty output, or are we happy as long as "git config" does not segfault, even if it spews anything? I guess that at this late point in the test we have per-repository or system-wide configuration files with something in them to test, so there would be some output but we do not care? If that is the case, not checking the output, like this patch does, is the right thing.

OK. Do we require --show-scope to fail this, or do we fail with and without --show-scope as long as --global is in effect? If the latter, test both ...

	rm -f "$HOME/.gitconfig" "$HOME/.config/git/config" &&
	test_must_fail git config ${mode_prefix}list --global &&
	test_must_fail git config ${mode_prefix}list --global --show-scope

... like this, perhaps? Also, don't overuse '-r' with 'rm' (applies other tests in this patch) when you know what you are removing should not be a directory.

Lose "r" from "-rf" or lose "/config".

OK.

OK.

Do not write a test this way. If you want to document an existing and unfixed breakage, instead of saying "we do want to see what is in this expect file, but we know output does not unfortunately match it", which is how the above test expresses it, start the whole thing with "test_expect_failure" (instead of "test_expect_success"), and have the body of the test express what you really want to see. I.e. the last steps should say

	git config ${mode_prefix}list --global --show-scope --show-origin >actual &&
	test_cmp expect actual

But an earier side note applies. If "git config list --global" gets corrected, this test will see update to turn "! test_cmp" into "test_cmp" (or "test_expect_success" to "test_expect_failure"), and such a patch that comes with the code fix will not show what is being tested and forcing the reviewer to go back to the previous step to see what the change is really about. A test that demonstrates and protects the behaviour corrected by the code change is best added in the same patch as the code change.

I cannot quite tell from only half the test, but I suspect that this shares exactly the same problem with the last one in the other file I commented above?

Thanks.

"Delilah Ashley Wu via GitGitGadget" <gitgitgadget@gmail.com> writes:

", but" -> "to match the information given by the command without --global, but".

That is rather unfortunate, as we do try hard to avoid deliberate regressions in our history. The reason why this step cannot be done without first introducing a regression is...?

If the reason is "it would make a single patch too big", perhaps we can do it in two steps, one preliminary "git_config_sequence() learns an extra barf-if-no-input parameter that causes it to return error if no files in the specified sequence exists" step, followed by this change that starts using git_config_sequence() to handle "--global", which uses that new flag to ensure that there won't be a regression?

Very nicely done.

Thanks.

On Fri, Oct 10, 2025 at 03:27:24AM +0200, Kristoffer Haugsbakk wrote:

Thanks for the clarification, I'll be sure to note this in my v2 cover letter and commit messages.

Delilah =)

On Wed, Nov 19, 2025 at 06:44:25AM -0800, Junio C Hamano wrote:

Good point! Will add above to the v2 cover letter.

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

Delilah Ashley Wu <delilahwu@linux.microsoft.com> writes:

After this and [*] the discussion stopped and the topic has been dormant since then for full two months. I'd drop the topic from 'seen' soonish but that does not mean an improved version of this patch is unwelcome.

Thanks.

[References]
 * https://lore.kernel.org/git/20251122020047.GB3947@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net/

Hi all, thanks for your patience. Here's my reroll.

As reported in [1], `$HOME/.gitconfig` and `$XDG_CONFIG_HOME/git/config` are both valid global configuration locations. However, when both files exist, `git config list --global` only reads from the former location whereas `git config list` (without `--global`) reads from both. The same issue was reported for `git config get` in [2]. This inconsistency has no good justification and contradicts the documented behaviour.

Suppose that `$HOME/.gitconfig` contains:
    [home]
        config = true

and `$XDG_CONFIG_HOME/git/config` contains:
    [xdg]
        config = true

Then, listing with `--global` shows only the home config:
    $ git config list --global --show-scope --show-origin
    global  file:/Users/delilah/.gitconfig    home.config=true

and getting the XDG configuration entry with `--global` will fail:
    $ git config get --global xdg.config; echo $?
    1

Git still reads the XDG config as part of its effective configuration,
as shown by listing the configuration without `--global`:
    $ git config list --show-scope --show-origin
    global  file:/Users/delilah/.config/git/config    xdg.config=true
    global  file:/Users/delilah/.gitconfig            home.config=true

The documentation, quoted in [1] and [2], states that `--global` should read from both files, so its output should be the same as above. Here's the relevant excerpt:

To be consistent with the documentation and the behaviour without `--global`, we should read both configuration files when `--global` is passed. We do this in a few steps:

 - Patch 1 fixes slash normalisation on Windows paths. This is used for
   `--show-origin` assertions in patch 3 tests.
 - Patch 2 modifies error handling when reading configuration files.
   This is used to prevent a regression in patch 3.
 - Patch 3 reads both configuration files when `--global` is specified.

[1]: https://lore.kernel.org/git/CAFA9we-QLQRzJdGMMCPatmfrk1oHeiUu9msMRXXk1MLE5HRxBQ@mail.gmail.com/ [2]: https://lore.kernel.org/git/CAAdFe9yhBk-WecVzCTsjQ-4Z3AZAbpP+w+B076ouM3qX6d1WAg@mail.gmail.com/

Thanks again for your time! Delilah

---
Changes in v2:
 - Squash test-only patches into their corresponding implementation
   patches.
 - Reorder patches to prevent a regression from being introduced and
   then fixed in a later patch.
 - Narrow the scope of slash conversion to `xdg_config_home_for()` and
   avoid modifying `cleanup_path()`, which could've broken callers that
   do not expect normalised slashes.
 - Clarify that some tests only check the return code of a `git config`
   command; we do not care about the output.
 - Link to v1: https://patch.msgid.link/pull.1938.git.1760058849.gitgitgadget@gmail.com/

---
Delilah Ashley Wu (3):
      path: use forward slashes in XDG config on Windows
      config: let sequence require a successful file
      config: read global scope via config_sequence

 builtin/config.c     |  11 +++++
 config.c             |  76 +++++++++++++++++++++++-----------
 config.h             |   2 +
 path.c               |  16 ++++---
 t/t1300-config.sh    | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t1306-xdg-files.sh |   5 ++-
 6 files changed, 194 insertions(+), 31 deletions(-)

Range-diff versus v1:

1: d9525d954e < -: ---------- config: read both home and xdg files for --global 2: c24ed49bac < -: ---------- cleanup_path: force forward slashes on Windows 3: 51293ee827 < -: ---------- config: test home and xdg files in `list --global` 4: 26f3c46598 < -: ---------- config: read global scope via config_sequence 5: b6ab7bfd67 < -: ---------- config: keep bailing on unreadable global files -: ---------- > 1: 2fa37d8aa7 path: use forward slashes in XDG config on Windows -: ---------- > 2: d90c9ae69f config: let sequence require a successful file -: ---------- > 3: a3b5599c8d config: read global scope via config_sequence

--- base-commit: 2c78326f810173a4f3aefd8021f1e07575412481 change-id: 20260808-fix-config-list-global-home-and-xdg-9bcaac093a1b

From: Delilah Ashley Wu <delilahwu@microsoft.com>

Teach `do_git_config_sequence()` to optionally report an error if no configuration files in the sequence were successfully processed. Gate this new behaviour with a flag and keep it disabled for now.

Add tests to record existing behaviour and prevent regressions in the next patch, "config: read global scope via config_sequence", which adds a code path that enables the flag. When no global configuration file exists, `git config list` succeeds whereas `git config list --global` fails. The command output is irrelevant, so only check the exit code.

Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
---
 config.c          | 57 ++++++++++++++++++++++++++++++++++++++-----------------
 t/t1300-config.sh | 12 ++++++++++++
 2 files changed, 52 insertions(+), 17 deletions(-)

From: Delilah Ashley Wu <delilahwu@microsoft.com>

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 displays 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 5ca6b7bb47b (config --show-origin: report paths with forward slashes, 2016-03-23). However, other code paths may construct 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

These mixed slashes occur because the `$HOME` and `$XDG_CONFIG_HOME` environment variables usually contain backslashes on Windows, and `xdg_config_home_for()` interpolates them into templates that use hardcoded forward slashes.

Since callers of `xdg_config_home_for()` handle mixed slashes correctly, it is reasonable to assume that they can handle paths with only forward slashes. Let's enforce forward slashes in `xdg_config_home_for()` by using `convert_slashes()` on Windows.

Also, there are no tests for the XDG path with `--show-origin`. Add a test for slash conversion and a confidence check for the default path.

Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
---
 path.c            | 16 ++++++++++------
 t/t1300-config.sh | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 6 deletions(-)

From: Delilah Ashley Wu <delilahwu@microsoft.com>

When both `$HOME/.gitconfig` and `$XDG_CONFIG_HOME/git/config` exist, `git config list --global` and `git config get --global` read the home configuration file but ignore the XDG file. Bug reporters expected these `--global` scoped commands to read both files [1][2], which would be consistent with the documentation and the behaviour of the unscoped variants. For example, `git config list` and `git config get` (without `--global`) read from both files (in addition to system-wide and repository-specific entries). We should address this inconsistency by respecting both files during `--global` read operations.

The implementation assumes that each configuration scope corresponds to a single file. So during `--global` read operations, Git selects one file path to pass to `git_config_from_file_with_options(file)`. Because the global scope can come from more than one file, we should use another method to read the global configuration.

Since `git config list --show-scope --show-origin` reads both the home and XDG files, there must be existing code that respects both locations, namely `do_git_config_sequence()` which reads from all scopes. Introduce flags 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 [3] in the original bug report [1].

Modify tests to check that both configuration files are respected during
`--global` read operations. Also, add additional tests to supplement the
regression tests from the previous patch, "config: let sequence require
a successful file". The expected behaviour of `git config list` is:
  - 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
    when one or more of them is readable.

Implementation notes:
  - The `ignore_global` flag is not set anywhere, so the
    `if (!opts->ignore_global)` condition is always met. Include the
    flag for completeness, but we can remove it if desired.

  - 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 clarify this.

[1] https://lore.kernel.org/git/CAFA9we-QLQRzJdGMMCPatmfrk1oHeiUu9msMRXXk1MLE5HRxBQ@mail.gmail.com/ [2] https://lore.kernel.org/git/CAAdFe9yhBk-WecVzCTsjQ-4Z3AZAbpP+w+B076ouM3qX6d1WAg@mail.gmail.com/ [3] https://lore.kernel.org/git/kl6ly1oze7wb.fsf@chooglen-macbookpro.roam.corp.google.com

Reported-by: Jade Lovelace <lists@jade.fyi>
Reported-by: Nils Fahldieck <nils@fahldieck.de>
Suggested-by: Glen Choo <glencbz@gmail.com>
Helped-by: Derrick Stolee <stolee@gmail.com>
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
---
 builtin/config.c     | 11 ++++++++
 config.c             | 37 +++++++++++++++------------
 config.h             |  2 ++
 t/t1300-config.sh    | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t1306-xdg-files.sh |  5 +++-
 5 files changed, 109 insertions(+), 17 deletions(-)

On Sun, Aug 23, 2026 at 3:31 AM Delilah Ashley Wu <delilahwu@linux.microsoft.com> wrote:

I have a related question: which of the global file(s) does

    git config --global --edit

edit? Which one(s) should it edit?

Chris

Chris Torek <chris.torek@gmail.com> writes:

I _know_ that having git-config read per-user configuration from both places was a deliberate design choice to help those who choose to migrate away from ~/.gitconfig to the XDG layout, while making sure we do not disrupt those who choose not to migrate.

For the write-out path of "git config --global set var val", we also chose accordingly, knowing that the majority of users back then had their per-user configuration in ~/.gitconfig and some, but not necessarily all, wanted to migrate to the XDG layout, while avoiding writing the same thing twice to different places. Therefore, "git config --global --edit" should follow the choice in the same spirit as the existing write-out code path (and no, I do not think we want to open two files in users' editors).

As to the primary focus of this topic, I think "git config --global" for the read path was not designed as carefully as the write-out code path or the general "git config" sequence when we introduced optional support for the XDG layout. Any discrepancy between "git config" when reading per-user values (to be overridden further by per-repository settings) and what "git config --global" reads from per-user files is very likely not due to any deliberate design choice, but merely bugs caused by a slip of the mind.

Delilah Ashley Wu <delilahwu@linux.microsoft.com> writes:

Is this "force forwared slashes to Windows users" a required part of XDG/HOME global fix? If not, please leave it out of the topic.

Even if it is a good idea to always force forward slashes to Windows users (I have no strong opinions on the topic), and if it is very unlikely to break existing Windows users (I do not have any clue if that would be the case or not, as I do not do Windows), we would want to make sure if we can get the same effect without sprinkling "#ifdef" in the platform agnostic part of the codebase like "path.c" file.

Where would the slash in "ret" that is passed to convert_slashes() function come from? If they come from environment variables like XDG_CONFIG_HOME and HOME, that is end-user's preference and we have no business forcing them which forms of slashes to use. Does it come from "subdir" or "filename" parameters? It might be the job for the callers to standardize slashes in the value they send in, but as far as I can see, these do not have anything other than hardcoded constants that use no slashes (most of them) or one forward slash ("systemd/user").

Again, I do not see it explained why this change has to be part of this series in the proposed log message, so...?

Delilah Ashley Wu <delilahwu@linux.microsoft.com> writes:

It is not exactly 'irrelevant' as that is how the user learns what caused the command to fail, e.g. "fatal: unable to read config file <path>".

What you meant was that you are not interested in the exact message, you only want to make sure it fails because of the missing file, and you thought that it is a good way to do so to check the exit code.

Perhaps "attempt" -> "try" or something more clever can be used to make sure we won't have to type so many characters. "try_config()" should be decriptive enough for the purpose, for example.

File scope static helper functions do not have to be and should not be named with so many words. Shorter names would also help to keep your lines under ~70 column limit.

If we are allowed to use system config, system_config is defined, and we can read the system config, we try to grab values from it, and record the fact that we did so successfully.

We grab paths to two files, as before.

If xdg config is to be used (note: GIT_CONFIG_GLOBAL environment can disable the use of it) and xdg file is available, we read and record just like we saw is done for the system config above.

Ditto fo user config.

And the local one.

And the per-worktree one.

If we are asked to ensure that we successfully read at least one place and we didn't, we assign -1 to ret but we do so ONLY when we haven't seen any other errors (i.e., existing non-zero ret is preserved, which may not be -1). OK.

I am not convinced 100% that we need "success_count", either, until we see how it is used in the later steps. But from the way the try_config() thing is used, I find it dubious that it now returns void. It should just keep returning the error code as before, and the caller should just keep accumulcating as the original code used to. I.e.,

		ret += try_config(fn, frotz_config, data,
				  CONFIG_SCOPE_FROTZ, NULL,
                                  &success);

Delilah Ashley Wu <delilahwu@linux.microsoft.com> writes:

We used to use ignore_repo, ignore_worktree, and ignore_cmdline members in the config_options, but to ignore system configuration, we relied on git_config_system() that checks GIT_CONFIG_NOSYSTEM environment variable, and there was no way to ignore per-user configuration. From that point of view, I find it sensible to make config_options the primary way to configure which parts of the configuration sequence is disabled.

But then we should go one step further, shouldn't we? Either teach git_config_system() to take config_options struct and pay attention to .ignore_system member in it, or get rid of git_config_system() and have the current users of that function take config_options and pay attention to its .ignore_system member, so that we do not have to write an ugly conditional like this one:

It is a bit misleading that this conditional is always taken. No caller will tell this function to skip the per-user configuration.

+100 column wide columns?  Please don't.

This sequence is a bit hard to read. Instead of piggybacking on the existing call to do the READL sequencing, add a new "else if" clause to deal specifically with the global case to the cascade would make the result easier to follow, I suspect. Something like this fix-up on top of this patch, perhaps.

 config.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Thanks for the reviews! I'm still working through the feedback for v3.

On Wed, Aug 26, 2026 at 10:58:57AM +1000, Junio C Hamano wrote:

Sorry, I forgot to explain in the log message that this change is supposed to supplement the `--show-origin` tests added in patch 3 (config: read global scope via config_sequence). Without it, the `--show-origin` would output a path with mixed slashes on Windows:

    file:"C:\\Users\\delilah/.config/git/config"    xdg.foo=bar

The tests expect paths containing only forward slashes. So patch 3 modifies `t1300-config.sh` assertions to look like this:

    echo "file:$HOME/.config/git/config    xdg.config=xdg" >expect
    git config list --global --show-origin >actual
    test_cmp expect actual

where `$HOME` has been normalised to contain forward slashes only, as seen at `t1300-config.sh:2179`, which was introduced in 45bf329 (t1300: fix the new --show-origin tests on Windows):

    HOME="$(pwd)" # convert to Windows path

I followed an existing usage of `#ifdef GIT_WINDOWS_NATIVE` and `convert_slashes()` in `path.c`, but if it's no longer allowed in the platform agnostic part, we could do the slash conversion elsewhere. This assumes we want to keep converting the slashes, but we should reconsider from your points raised below.

The slash in `ret` would come from the environment variables, so perhaps we should not modify the slashes in them at all. Instead, I could drop this patch and change the tests in patch 3 to export a `XDG_CONFIG_HOME` value containing only forward slashes. This would satisfy the assumption that paths in the tests will contain only forward slashes. What do you think?

Thanks! Delilah =)