Byron / gitoxide

An idiomatic, lean, fast & safe pure Rust implementation of Git
Apache License 2.0
8.84k stars 301 forks source link

Don't read "installation" config from `GIT_CONFIG` #1583

Closed EliahKagan closed 3 weeks ago

EliahKagan commented 3 weeks ago

This makes it so GIT_HIGHEST_SCOPE_CONFIG_PATH never uses the value of the GIT_CONFIG environment variable. Previously it used it because git config uses it, treating it as though its value were an operand to --file. But no other git commands use it.

It was previously used in most situations when present, since it causes git config -l to list nothing else of any scope (even if very high scoped configuration is available). The main and possibly only significant situation where it was not used was the EXEPATH optimization on Windows (which in practice only occurs in Git Bash environments). I think this inconsistency--since it never has anything to do with whether a user would want GIT_CONFIG to have an effect--is one of the most significant reasons to omit it.

However, when reviewing this, I recommend considering if possible uses where a user would set GIT_CONFIG to view configuration from an arbitrary file with commands like gix config--even though it would not work from Git Bash--might be something people are doing. I have prefixed the commit message fix: but maybe I should have written fix!: for this reason? I am not sure. But as far as I know, this is not altering any behavior to become inconsistent with what was previously documented or otherwise stated.

There is more information in the commit messages, especially the one prefixed fix:, eb72d31a6. This also adds tests related to GIT_CONFIG, in the two commits that precede that, 340ff371d and f8e38d057.

I have also unset some other environment variables (that would only occasionally be set in the first place) besides GIT_CONFIG. This is in the first commit, 01a412feb, and it is for a different reason: it is to ensure that even if some version of git behaves weirdly, or if my understanding of its intended behavior is incorrect, then we still get the full effect of setting GIT_DIR. The commit message has more information about that. I mostly included that here rather than in a separate PR to avoid a merge conflict (or decreased code readability if the remove_env calls were to be merged in an unintuitive order).