-
Notifications
You must be signed in to change notification settings - Fork 144
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.
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
base: master
Are you sure you want to change the base?
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.txt To 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 git-mentoring@googlegroups.com, 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: |
c480893
to
3f97f81
Compare
The `git config list --global` output includes `$HOME/.gitconfig`, but ignores `$XDG_CONFIG_HOME/git/config`. It should include both files. Modify tests to check for this expected behavior: A. `git config list --global` should include contents from both the home and XDG config locations (assuming they are readable), not just the former. Also, add tests to ensure that the subsequent patches do not introduce regressions on the behavior of `git config list`: B. The home config should take precedence over the XDG config. C. Without `--global`, it should not bail on unreadable/non-existent global config files. D. 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. Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
The output of `git config list --global` should include both `$HOME/.gitconfig` and `$XDG_CONFIG_HOME/git/config`, but it only reads from the former. We've assumed each config scope corresponds to a single config file location. Because of 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 source file. Since the global scope includes more than one file, we should use another method to read the global config. The output of `git config list --show-scope --show-origin` 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 that make it possible to ignore all but the global scope (i.e. ignore system, local, worktree, cmdline). Then, reuse this function for reading only the global scope. This was the suggested solution in the original bug report thread [1]. Note 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. Note 2: It's 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. Note 3: Keep populating `opts->source.file` in `builtin/config.c` because it is used as the destination 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. [1]: https://lore.kernel.org/git/kl6ly1oze7wb.fsf@chooglen-macbookpro.roam.corp.google.com. Reported-by: Jade Lovelace <lists@jade.fyi> Suggested-by: Glen Choo <glencbz@gmail.com> Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
The 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 introduced a regression in scenario B: running `git config list --global` would not fail when both global config files are unreadable. For example, `GIT_CONFIG_GLOBAL=does-not-exist git config list --global` would exit 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: The logic to determine the exit code does not actually sum the return codes of the underlying operations. Instead, it uses a single decrement operation. If this is undesirable, we can change it to sum the return codes of the underlying operations instead. Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com>
3f97f81
to
a44ef7c
Compare
/allow |
User delilahw is now allowed to use GitGitGadget. |
As reported in 1: both
$HOME/.gitconfig
and$XDG_CONFIG_HOME/git/config
are valid config locations for the global scope, 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
In reality, Git correctly applies the XDG config in its effective
configuration, but it just 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 behavior is that 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 and also in the
git config
documentation:The first patch introduces tests and regression checks. The second and
third patches implement the fix to include both config files when
--global
is specified.CC: Delilah Wu delilahwu@linux.microsoft.com