config: read both home and xdg files for --global - #2196
Conversation
|
There is an issue in commit 4c0da9e:
|
|
NB: i won't be submitting this via gitgitgadget anymore because I'd like the subject to contain |
If you wanted to use GitGitGadget and have a v2, you would need to reopen the original PR and force-push there, then |
thanks for the suggestion @dscho !! i did something weird with my branch and it wouldn't let me reopen the original PR after force pushing, which is why i opened this new PR. but i've setup |
33d79b9 to
1a89c55
Compare
|
There is an issue in commit 4f4ffa4:
|
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 5ca6b7b (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. Signed-off-by: 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 an `int require_successful_config` flag. Add tests to check for regressions: `git config list --global` should fail if both the home and XDG configuration files are nonexistent. It should succeed if at least one of them were read. We only care about the command's exit code, not its output. The next patch, config: read global scope via config_sequence, changes how `git config list --global` reads the global configuration. It uses this new flag to prevent a regression by ensuring the command continues to fail when both (all) global config files are nonexistent. Signed-off-by: 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` read from
both files (in addition to system-wide and repository-specific entries).
We should address this inconsistency by reading both files during
`--global` operations.
The implementation assumes that each configuration scope corresponds to
a single file. So during `--global` operations, Git selects a single
path and passes it to `git_config_from_file_with_options(path)`. Since
the global scope sources two files, we should read the configuration
files using another method.
Running `git config list --show-scope --show-origin` (without
`--global`) reads both the home and XDG files. So there's 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.
Also, modify tests to check that both configuration files are respected
during `--global` read operations.
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.
- 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>
1a89c55 to
813ac6b
Compare
Hi,
Here is my reroll.
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 adds a flag for error handling when reading configuration files. The third patch implements the fix to include both config files when `--global` is specified.
Changes in v2:
[1]: https://lore.kernel.org/git/CAFA9we-QLQRzJdGMMCPatmfrk1oHeiUu9msMRXXk1MLE5HRxBQ@mail.gmail.com/
[2]: https://lore.kernel.org/git/xmqqmt5lezi3.fsf@gitster.g/
[3]: #1938
Thank you all for your time!
Delilah
cc: Delilah Ashley Wu delilahwu@microsoft.com
cc: Derrick Stolee stolee@gmail.com
cc: Johannes Schindelin johannes.schindelin@gmx.de
cc: Junio C Hamano gitster@pobox.com
cc: Patrick Steinhardt ps@pks.im
cc: Kristoffer Haugsbakk kristofferhaugsbakk@fastmail.com