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

Make git config --system work like you think it should on Windows #2358

Conversation

dscho
Copy link
Member

@dscho dscho commented Oct 17, 2019

I cannot count how many times the existence of the Git config in ProgramData caused confusion, most recently in #2348.

These days and age, it seems to be in fashion to double down on bad ideas rather than admitting mistakes, but I would like to believe that it is better to see a bad decision for what it is and revert it. It took me long enough!

So rather than trying to explain the concept of C:\ProgramData\Git\config and keeping Git for Windows' system config out of everybody's sight, yet having a prominent way with git config --system to access the latter while not having any intuitive way for the former, it is past time to reconcile both already, into a location that is both intuitive and clarifies the ownership: this is Git for Windows' system config, other Git implementations are free to use it, but not to modify it.

This PR is a small part of what is necessary to fix the mess, the installer will also have to be updated, and there needs to be a big notice in the release notes.

My plan is to merge this PR tomorrow, in time for Git for Windows v2.24.0-rc0.

This moves the system config into a more logical location: the `mingw64`
part of `C:\Program Files\Git\mingw64\etc\gitconfig` never made sense,
as it is a mere implementation detail. Let's skip the `mingw64` part and
move this to `C:\Program Files\Git\etc\gitconfig`.

Side note: in the rare (and not recommended) case a user chooses to
install 32-bit Git for Windows on a 64-bit system, the path will of
course be `C:\Program Files (x86)\Git\etc\gitconfig`.

The next step after this patch is to scrap support for
`C:\ProgramData\Git\config`, which never really made sense to users, and
which is inconsistent with non-Windows versions of Git, anyway.

Background: During the Git for Windows v1.x days, the system config was
located at `C:\Program Files (x86)\Git\etc\gitconfig`. With Git for
Windows v2.x, it moved to `C:\Program Files\Git\mingw64\gitconfig` (or
`C:\Program Files (x86)\Git\mingw32\gitconfig`).

Obviously, this new location was not stable (because of the "mingw64"
part) and this maintainer thought that it was a splendid time to
introduce a "super-system" config, with a constant location, that would
be shared by all Git implementations. Hence `C:\ProgramData\Git\config`
was born.

What we *should* have done instead is to move the location to the
top-level `etc` directory instead of the `mingw64\etc` one (or
`mingw32\etc` for 32-bit versions).

Likewise, we should have treated the system `gitattributes` in a similar
manner.

This patch makes it so.

Obviously, we are cautious to do this only for the known install
locations `/mingw64` and `/mingw32`; If anybody wants to override that
while building their version of Git (e.g. via `make prefix=$HOME`), we
leave the default location of the system config and gitattributes alone.

Signed-off-by: Johannes Schindelin <[email protected]>
In the previous commit, we moved the location of the system config to
`C:\Program Files\Git\etc\gitconfig`, to give users a reliable location
for the system config.

Previously, we had introduced `C:\ProgramData\Git\config` as such a
location.

However, the purpose of `C:\ProgramData\Git\config` was always for the
installed Git for Windows to share its configured options. Every upgrade
will modify the settings in that file according to what the user
specified when running Git for Windows' installer. So it is not really a
file whose ownership is shared between all Git implementations, contrary
to what the location suggests.

Also, it was totally confusing to many users that there was a "system"
config and then yet another "sort of system" config that did not even
have a corresponding `git config` option such as `--system`.

Further, Portable Git should probably never look at
`C:\ProgramData\Git\config` to begin with: this makes it rather
non-Portable ;-)

Finally, support for `C:\ProgramData\Git\config` never really caught on:
only libgit2 implemented it, but even JGit (which might be used by more
users than libgit2-based applications by grace of backing Android
Studio) did not.

Therefore, let's finally admit that it was a mistake to introduce
`C:\ProgramData\Git\config` and remove support for it.

This reverts commit bcbbb4f.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member Author

dscho commented Oct 17, 2019

I canceled the Azure Pipeline because of the pesky perforce problem that made the macOS jobs fail (again!). A tentative fix is here: #2359. Once merged, I will re-run the Azure Pipeline.

@dscho
Copy link
Member Author

dscho commented Oct 17, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ethomson
Copy link
Member

Hmm. The goal of the ProgramData file was that it was actually supposed to be a shared configuration file. So that you didn't have two applications working in the same git repository with two different ideas of core.autocrlf and the like.

What I would like to see is to actually have a truly system-wide configuration file. I agree that having four configuration locations, but only on Windows, is confusing. But I think that this regresses the already-tenuous compatibility between Git for Windows, Portable Git, and the other implementations.

@dscho
Copy link
Member Author

dscho commented Oct 17, 2019

What I would like to see is to actually have a truly system-wide configuration file.

@ethomson If you have a splendid idea where that should go, I want to hear it: the recent security fix for the C:\ProgramData\Git\config support indicated rather strongly that it was not a good spot to pick.

Also, I am really happy to scrap support for C:\ProgramData\Git\config so that Portable Git finally gets truly portable again. That was pointed out to me multiple times, and I finally acted.

So I could imagine that we might end up with another location instead of C:\ProgramData\Git\config, but that would still need to not be heeded by Portable Git, therefore I think that this PR is still addressing an important issue in the right way. My plan for MinGit is to use [include]path=C:\Program Files\Git\etc\gitconfig in its system config, and if we end up wanting a shared config (which JGit seems totally fine not to buy into), we should use the same strategy in Git for Windows so that Portable Git remains unaffected.

@ethomson
Copy link
Member

Also, I am really happy to scrap support for C:\ProgramData\Git\config so that Portable Git finally gets truly portable again. That was pointed out to me multiple times, and I finally acted.

I'm sorry, I don't understand the motivation here. People want Portable Git to not read the system configuration file? What's the rationale?

That's just for curiosity's sake. I feel like Portable Git being unique and not reading the system configuration location is fine, but there are myriad mechanisms that we can use to accomplish that, and it's really rather orthogonal to this discussion. (Meaning, you could remove its support for the ProgramData location today.)

My plan for MinGit is to use [include]path=C:\Program Files\Git\etc\gitconfig in its system config

I feel like we could alternately just agree on a system-wide configuration location and not settle for includeing the system-wide configuration location in a thing that's called the "system config" but isn't actually system-wide.

I know that this is Weird For Us since Git for Windows is really a distribution of a POSIX-like environment, not just a single application, but I think that instead of looking at that as the environment that we have to live in, we should look at what our users want. And I don't think that they want to understand that different clients have to read different "system config" files because they're different distributions of POSIX-like environments.

Including the common one is a clever solution, but I feel like we could just agree on where the common one goes in the first place and forget about $(prefix)/etc entirely.

@ethomson
Copy link
Member

Having said that, I suppose that I really don't care that much since libgit2, at least, goes looking for your etc directory to read that configuration file.

I am concerned about what this means for libgit2's backward compatibility. I suppose it's your installer that might deal with the new configuration files? What do you have planned here?

What if I have stuff in my ProgramData config and install the newest version of Git for Windows? Is that ignored? Migrated to a different file?

@dscho
Copy link
Member Author

dscho commented Oct 17, 2019

People want Portable Git to not read the system configuration file? What's the rationale?

@ethomson people who use Portable Git want to read the system config in that Portable Git, and only that one.

Reading from outside of that Portable Git makes it -- by definition -- non-Portable.

I am concerned about what this means for libgit2's backward compatibility. I suppose it's your installer that might deal with the new configuration files? What do you have planned here?

What if I have stuff in my ProgramData config and install the newest version of Git for Windows? Is that ignored? Migrated to a different file?

My plan is to teach the Git for Windows installer to look for current settings (but only the ones that you can set via the installer) in the system config, and if it is not found in the expected place, go and look at ProgramData (if there is one).

If you have settings in ProgramData, you have to migrate them manually.

Since I expect the number of people who even know about ProgramData to be at most in the double-digit numbers, I don't think this is too serious of a backwards-incompatibility.

@PhilipOakley
Copy link

Hi dscho, It looks good to me. As noted the extra config was getting confusing. Plus the extra bit of documentation only being in the G-f-W version meant that I (and Google) had missed it.

Not sure how to get upstream docs to carry G-f-W notes... [maybe a line in the upstream "For additional Git for Windows notes, see https..."]

@dscho
Copy link
Member Author

dscho commented Oct 17, 2019

Not sure how to get upstream docs to carry G-f-W notes... [maybe a line in the upstream "For additional Git for Windows notes, see https..."]

Well, part of the allure of this PR is that we don't have to Git for Windows-specific notes in https://git-scm.com/docs/git-config...

dscho added a commit to dscho/build-extra that referenced this pull request Oct 17, 2019
In git-for-windows/git#2358, we adjusted the
location of the system config and gitattributes file.

This patch adjusts the post-install script of the `git-extra` package
(which is used to edit some files that cannot otherwise be overridden in
a regular MSYS2 setup) to expect the system config in the new place.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/build-extra that referenced this pull request Oct 17, 2019
In git-for-windows/git#2358, we reconciled the
two system configs into a single one.

Even if we now build an installer bundling a Git executable that is
still ProgramData-aware, it does not hurt to already drop bothering
about editing/initializing it: the system config in `C:\Program Files`
would override the ProgramData one anyway.

Note: for the time being, we still initialize the defaults for the
settings that can be configured in Git for Windows' installer from
_both_ ProgramData _and_ the previous installation's system config.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/build-extra that referenced this pull request Oct 17, 2019
In git-for-windows/git#2358, we fixed the design
where Git for Windows would first read a "public" system config that was
not even in `C:\Program Files\Git` and then would read a "private"
system config. It now only reads _one_ system config, and that is in an
intuitive location: `C:\Program Files\Git\etc\gitconfig`.

Let's teach the script that generates the installer to make sure to ask
`git.exe` what its idea of the system config location is, and then use
that.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/build-extra that referenced this pull request Oct 17, 2019
In git-for-windows/git#2358, we fixed the design
where Git for Windows would first read a "public" system config that was
not even in `C:\Program Files\Git` and then would read a "private"
system config. It now only reads _one_ system config, and that is in an
intuitive location: `C:\Program Files\Git\etc\gitconfig`.

Let's teach the script that generates the Portable Git to make sure to
ask `git.exe` what its idea of the system config location is, and then
use that.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/build-extra that referenced this pull request Oct 17, 2019
In git-for-windows/git#2358, we fixed the design
where Git for Windows would first read a "public" system config that was
not even in `C:\Program Files\Git` and then would read a "private"
system config. It now only reads _one_ system config, and that is in an
intuitive location: `C:\Program Files\Git\etc\gitconfig`.

Let's teach the script that generates the MinGit subset of Git for
Windows to make sure to ask `git.exe` what its idea of the system config
location is, and then use that.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho merged commit c25f209 into git-for-windows:master Oct 18, 2019
@dscho
Copy link
Member Author

dscho commented Oct 18, 2019

I merged this so that I can now start to rebase to Git v2.24.0, which will take quite a while because of all the builtin add -c fallout...

@dscho dscho deleted the reconcile-system-config-and-programdata-config branch October 18, 2019 07:28
@dscho dscho added this to the v2.23.0(2) milestone Oct 18, 2019
dscho added a commit that referenced this pull request Oct 20, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
dscho added a commit that referenced this pull request Oct 21, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
git-for-windows-ci pushed a commit that referenced this pull request Oct 21, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
dscho added a commit that referenced this pull request Oct 23, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
dscho added a commit that referenced this pull request Oct 25, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
dscho added a commit that referenced this pull request Oct 30, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
dscho added a commit that referenced this pull request Nov 2, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
@dscho dscho mentioned this pull request Nov 4, 2019
dscho added a commit that referenced this pull request Nov 6, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
@ghost
Copy link

ghost commented Nov 7, 2019

Thanks for this. I have made edits in these files and yep this surely helps with having a clear picture and avoid conflicts. I use another msys2 installation for compiling and I was just having an issue with GIT popping up when I ran a command to configure a compilation prep file, it was so weird as two actions ran simulatneously, git reported a syntax issue and the "configure" ran and actually completed it work. Turned out I had a bloated msys2 install with a lot of git packages as well as GFW added to the PATH which I removed since it's not needed for Git Bash anyway in my case.
There's already a need to rename or delete "link.exe" in the default msys2 installation to be able to compile FFmpeg on windows with MSVC for example. These things only look small but they can cost hours and hours of troubleshooting, not to mention the nerves.

dscho added a commit that referenced this pull request Nov 16, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
dscho added a commit that referenced this pull request Nov 25, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
dscho added a commit that referenced this pull request Nov 26, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
@whut
Copy link

whut commented Nov 26, 2019

Looks like the rebase.autosquash=true, that was previously in C:\ProgramData\Git\config was not migrated to new location. Was that a deliberate decision?

@dscho
Copy link
Member Author

dscho commented Nov 26, 2019

Looks like the rebase.autosquash=true, that was previously in C:\ProgramData\Git\config was not migrated to new location. Was that a deliberate decision?

No, it was not a deliberate decision. To the contrary, it should still be there, see https://github.com/git-for-windows/build-extra/blob/master/git-extra/gitconfig

@whut could you open a new ticket for this, filling out all the placeholders so that I know where to chase the bug?

@dscho
Copy link
Member Author

dscho commented Nov 26, 2019

FWIW I even see this setting in the current version of Git for Windows' SDK.

dscho added a commit that referenced this pull request Dec 7, 2019
…amdata-config

Make `git config --system` work like you think it should on Windows
@JojOatXGME
Copy link

JojOatXGME commented Dec 14, 2020

I also noticed some months ago that git rebase -i did no-longer use --autosquash by default after updating my Git installation on Windows. I did not bother investigating the issue at first but I recently noticed that some users were “blaming” JetBrains for changing the behavior of IDEA. See IDEA-209497. I guess they run into the same issue.

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.

6 participants