Skip to content
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

config: check both git/etc and git/mingw64/etc on windows #1194

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

pmrowla
Copy link
Contributor

@pmrowla pmrowla commented Jul 14, 2023

Current windows Git installations include separate git exe's in Git/cmd/git.exe and Git/mingw64/bin/git.exe. The mingw64 build will be in the PATH when the user is in git-bash, and the native cmd/git.exe build will be in the PATH in native windows shells (cmd.exe or powershell).

The system level gitconfig that gets used by either git.exe is located in Git/etc/gitconfig, but the existing windows gitconfig lookup behavior only searches Git/mingw64/etc/gitconfig

@pmrowla pmrowla requested a review from jelmer as a code owner July 14, 2023 06:13
git_dir, _bin_dir = os.path.split(path)
yield git_dir
parent_dir, basename = os.path.split(git_dir)
if basename == "mingw32" or basename == "mingw64":
yield parent_dir
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both Git/etc/... and Git/mingw64/etc/... exist in the windows Git installations I tried, but in all the cases I saw git was only using Git/etc/gitconfig (regardless of shell). But I'm not entirely sure that we can expect consistent behavior across all windows Git versions, so I went with the safe option of just yielding both Git/ and Git/mingw64/ whenever we are in the mingw/git-bash environment.

@jelmer jelmer merged commit 879fe6a into jelmer:master Jul 14, 2023
@pmrowla pmrowla deleted the win-git-bash branch July 14, 2023 09:37
@efiop
Copy link
Contributor

efiop commented Jul 14, 2023

@jelmer Could you trigger a release, please? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants