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 |
54d8dc3 to
d9ad011
Compare
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 On Windows, these inconsistent slashes occur because the `$HOME` and `$XDG_CONFIG_HOME` environment variables may contain backslashes when `xdg_config_home_for()` interpolates them into one of two templates: `$HOME/.config/<subdir>/<filename>` or `$XDG_CONFIG_HOME/<subdir>/<filename>`. 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 calling `convert_slashes()` on the interpolated path on Windows. Avoid modifying the strings returned from `getenv()`, which would lead to undefined behaviour. Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
Teach `do_git_config_sequence()` to optionally report an error if none of the configuration files in the sequence were successfully processed. Introduce a flag to enable this new behaviour and leave it disabled by default to avoid breaking existing callers. 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 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
commands to read both files and be consistent with the documentation and
the behaviour of the unscoped `git config list` and `git config --get`
commands [1][2], which read from both files (in addition to
system-wide and repository-specific entries, of course).
We assume each scope corresponds to a single configuration file. So,
during `--global` operations, Git selects a single path and passes it to
`git_config_from_file_with_options(path)`. Since global configuration
comes from two files, we should read the configuration files with
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.
Modify tests to check that both configuration files are respected during
`--global` read operations and add tests to ensure we do not introduce
regressions. Specifically:
- The home config should take precedence over the XDG config.
- `git config list` should not fail on non-existent global config.
- `git config list --global` should fail when both global config files
are non-existent. It should not fail if at least one of them exists.
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>
d9ad011 to
33d79b9
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