RFC: system-wide config file in %PROGRAMDATA% #157

kblees · 2015-05-21T15:45:13Z

This fixes issue #154, as an alternative to @dscho's PR #104.

Notable differences:

git config --system reads / writes %PROGRAMDATA%\Git\gitconfig
$(prefix)/etc/gitconfig is read before %PROGRAMDATA%\Git\gitconfig
the fallback mechanism works on all platforms, including Linux and all Windows build targets
also works for %PROGRAMDATA%\Git\gitattributes (and any future config files that go to the sysconfdir Makefile variable)
doesn't need SHGetFolderPath API. IIRC, delay-loading DLLs was a huge performance saver in MSys-1.dll, so I'd rather not require loading shell32.dll in the startup code of practically every git command (although I don't think we currently do delay-loading, but this may be as simple as adding a compiler switch)

dscho · 2015-05-21T17:16:16Z

I am still convinced that there are settings which make sense only in the context of Git for Windows, but not, say, for libgit2-based software. Therefore, I am opposed to simply replacing the /etc/gitconfig location with %PROGRAMDATA%\Git\config.

There is the additional problem that this completely violates the portable application principle. Granted, you did not introduce it, but the proposed change would really cement that violation, i.e. now all PortableGit-* violates the contract to be self-contained and not look outside their own directories for configuration.

In any case, it should be %PROGRAMDATA%\Git\attributes, not %PROGRAMDATA\gitattributes.

Factor out near identical per-file logic. Its also unclear that 'ret += git_config_from_file()' actually decrements ret on error. It adds the return value of error() (or const_error() for GCC), which is five levels down the call hierarchy. Signed-off-by: Karsten Blees <[email protected]>

'git config' (--add / --unset etc.) automatically creates missing config files. However, it fails with a misleading error message "could not lock config file" if the parent directory doesn't exist. Also create missing parent directories. Signed-off-by: Karsten Blees <[email protected]>

If Git is configured to use the absolute '/etc/gitconfig' for system-wide settings, it may still be useful to have installation-specific settings such as 'help.format' (which depends on the installed set of help pages). This is particularly relevant on Windows, where a variety of Git flavors with different feature sets may be installed in parallel. If ETC_GITCONFIG is an absolute path (typically '/etc/gitconfig'), load installation-specific defaults (from '$(prefix)/$(ETC_GITCONFIG)') before loading the system-wide configuration. As installation-specific settings will typically be defined when creating the software package / installer (i.e. *before* git is installed), we don't need an extra 'git config --installation' option. Update the documentation accordingly. This removes '$(prefix)' from most places (which would be meaningless to normal users anyway). Signed-off-by: Karsten Blees <[email protected]>

Git versions on Windows typically store system-wide config files within their installation directory. Other Git implementations (e.g. libgit2, JGit etc.) may wish to reuse the system-wide config, but finding the installation directory of an already installed Git version is problematic. On Windows, the %PROGRAMDATA% environment variable specifies the root directory for system-wide application data (%ALLUSERSPROFILE% for WinXP). This would be the equivalent of '/etc' (and '/var'), except that naming guidelines recommend organizing data in per-application sub-directories. When looking up a system_path() on Windows, replace the absolute path prefix '/etc/' with '%PROGRAMDATA%/Git/'. For MinGW and MSVC builds, activate system-wide configurations by changing the $(sysconfdir) Makefile variable to an absolute path ('/etc'). Cygwin and MSys2 builds get this automatically when building with 'prefix=/usr'. Update the documentation for /etc/gitconfig and /etc/gitattributes. IOW: All Git versions on Windows (Cygwin, MinGW, MSVC and MSys2) will load configuration settings from '$(prefix)/etc/gitconfig' followed by '%PROGRAMDATA%/Git/gitconfig'. Signed-off-by: Karsten Blees <[email protected]>

kblees · 2015-05-23T20:32:42Z

I've updated the PR and the description to match the current version.

dscho · 2015-05-25T13:54:08Z

Hmm. I still do not like the repeated "git" in %PROGRAMDATA%\Git\gitconfig, not even one bit.

dscho · 2015-05-25T16:07:11Z

This changes the priority of the Windows-wide and the system config files. Therefore, it is no longer possible to override a Windows-wide setting via /mingw64/etc/gitconfig. However, that was exactly what I tried to accomplish with my Pull Request.

Just to be absolutely clear: I still have two very strong objections to this Pull Request, even if most of it looks very good:

I am certain that repeating git in %PROGRAMDATA%\Git\gitconfig would be a mistake. It violates the DRY principle, and actually points to an even larger issue: %PROGRAMDATA%\Git can definitely not play the same role for Git for Windows as /etc/ can because /etc/ is owned by MSys2, while %PROGRAMDATA% is a place to share configurations with other Git implementations. Other Git might be unaware of POSIX conventions, use different certificate formats, handle less locales, files that would be uninstalled with Git for Windows should not be referenced in the Windows-wide config, etc. Therefore I would like to encourage you to reconsider redirecting all /etc/ traffic. We need to be very deliberate for what files we look in %PROGRAMDATA%\Git.
Conceptually, the Windows-wide configuration can only be less specific than any configuration could that Git for Windows ships: The former is a much more publicly visible place than the latter. Therefore, the precedence order must be such that Git for Windows' gitconfig can override the Windows-wide configuration, not the other way round.

dscho · 2015-05-26T09:03:09Z

For the record: this Pull Request is the reason I delayed the Git for Windows version I wanted to release yesterday.

kblees · 2015-05-26T15:46:03Z

%PROGRAMDATA%\Git can definitely not play the same role for Git for Windows as /etc/ can because /etc/ is owned by MSys2

I think this is a misunderstanding: the absolute path /etc/ was never used before. Until now, etc/ was always relative (to the location of the executable).

Of course you cannot put Git for Windows or MSys2 specific settings into /etc/ (aka %PROGRAMDATA%\Git), but you can put them into etc/ (aka $(prefix)/etc/).

Therefore I would like to encourage you to reconsider redirecting all /etc/ traffic. We need to be very deliberate for what files we look in %PROGRAMDATA%\Git.

/etc/ is not even a valid absolute path on Windows (it is relative to the current drive letter), therefore IMO we must translate it.

Additionally, it would be a pity if the new %PROGRAMDATA%\Git location didn't work for /etc/gitattributes as well. Because if it didn't, you would still have to copy shared gitattributes files to all Git installations (requiring write access to %PROGRAMFILES%), and uninstalling / upgrading Git would probably delete the file (depending on how smart the installer is).

kblees · 2015-05-26T22:04:55Z

More thoughts on ca-bundle.crt:

Any Git for Windows installer will include the $(prefix)/ssl/certs/ca-bundle.crt file that was "current" when the installer was built.
The $(prefix)/ssl/certs/ca-bundle.crt file is supplied by the mingw-w64-ca-certificates package (https://github.com/git-for-windows/MINGW-packages/tree/master/mingw-w64-ca-certificates).
The mingw-w64-ca-certificates package is based on Debian's ca-certificates package, latest version 2014-10-19 (https://packages.debian.org/en/sid/ca-certificates). The package description contains this:

Please note that Debian can neither confirm nor deny whether the certificate authorities whose certificates are included in this package have in any way been audited for trustworthiness or RFC 3647 compliance. Full responsibility to assess them belongs to the local system administrator.

The Debian package is based on Mozilla's certdata.txt file, latest version 2015-04-20 (http://mxr.mozilla.org/mozilla-central/source/security/nss/lib/ckfw/builtins/certdata.txt).

Bottom line is this: The Git for Windows project cannot guarantee that the shipped ca-bundle.crt file is up-to-date. Therefore, if a local system administrator configures an alternate version in the system-wide (or "windows-wide") configuration, the Git for Windows specific gitconfig file must not overrule this.

Note: cURL on Windows should actually find the curl-ca-bundle.crt file automatically (see curl(1)):

The windows version of curl will automatically look for a CA certs file named 'curl-ca-bundle.crt', either in the same directory as curl.exe, or in the Current Working Directory, or in any folder along your PATH.

This is how it worked in the old Git for Windows. However, the lookup mechanism is broken in the Mingw64 version because of this patch: https://github.com/git-for-windows/MINGW-packages/blob/master/mingw-w64-curl/0001-curl-relocation.patch

This is why we have to specify the path in gitconfig in the first place.

dscho · 2015-05-27T07:20:02Z

I think this is a misunderstanding: the absolute path /etc/ was never used before. Until now, etc/ was always relative (to the location of the executable).

For my argument that etc/ is

a POSIX convention, implying the availability of a shell, and
in contrast to %PROGRAMDATA%\Git not specific to Git at all,
it is completely irrelevant whether I talked about an absolute /etc/ or a relative etc/.

/etc/ (aka %PROGRAMDATA%\Git)

Please don't. It is a serious misunderstanding of scope and purpose to consider etc/ and %PROGRAMDATA%\Git equivalent, as I have pointed out multiple times by now.

/etc/ is not even a valid absolute path on Windows (it is relative to the current drive letter), therefore IMO we must translate it.

That is a red herring. MSys2 already translates it to a Windows path. But that does not change the role of etc/.

Additionally, it would be a pity if the new %PROGRAMDATA%\Git location didn't work for /etc/gitattributes as well. Because if it didn't, you would still have to copy shared gitattributes files to all Git installations (requiring write access to %PROGRAMFILES%), and uninstalling / upgrading Git would probably delete the file (depending on how smart the installer is).

I agree that it would be a good thing to support attributes in %PROGRAMDATA%\Git.

However, please let's just agree that repeating git in %PROGRAMDATA%\Git\gitattributes would be just wrong, just like %PROGRAMDATA\Git\gitconfig would be an unwise choice. After all, we also call the repository-specific versions .git/config and .git/info/attributes, not .git/gitconfig and .git/info/gitattributes!

But I fear that we get hung up on almost irrelevant details, while the bigger issue still looms: you still want to equate etc/ and %PROGRAMDATA\Git which is just not a bad idea.

if a local system administrator configures an alternate version in the system-wide (or "windows-wide") configuration, the Git for Windows specific gitconfig file must not overrule this.

Wrong. If a system administrator installs other certificates, she has to update the Git for Windows gitconfig accordingly (either deleting that setting, or adjusting it).

cURL on Windows should actually find the curl-ca-bundle.crt file automatically (see curl(1)):

The windows version of curl will automatically look for a CA certs file named 'curl-ca-bundle.crt', either in the same directory as curl.exe, or in the Current Working Directory, or in any folder along your PATH.

This is how it worked in the old Git for Windows. However, the lookup mechanism is broken in the Mingw64 version because of this patch: https://github.com/git-for-windows/MINGW-packages/blob/master/mingw-w64-curl/0001-curl-relocation.patch

So maybe we can fix that lookup mechanism? I have no idea how it is broken, do you have any more information to share?

We're getting seriously side-tracked here, though.

So back to the original question of this Pull Request: will you adjust the precedence of etc/gitconfig relative to %PROGRAMDATA%\Git in the branch associated with this Pull Request?

dscho · 2015-05-27T07:20:40Z

will you adjust the precedence of etc/gitconfig relative to %PROGRAMDATA%\Git in the branch associated with this Pull Request?

Oh, and: will you revert the wholesale redirection of /etc/?

dscho · 2015-05-27T19:15:07Z

As "threatened", I cherry-picked the really nice changes, and kept the precedence in a logically defensible order: 57190aee^...57190aee^2

…tus-cache-untracked-hint status: deserialize with -uno does not print correct hint

Includes these pull requests: #1 #6 #10 #11 git-for-windows#157 git-for-windows#212 git-for-windows#260 git-for-windows#270 Signed-off-by: Derrick Stolee <[email protected]>

Includes these pull requests: git-for-windows#1 git-for-windows#6 git-for-windows#10 git-for-windows#11 git-for-windows#157 git-for-windows#212 git-for-windows#260 git-for-windows#270 Signed-off-by: Derrick Stolee <[email protected]>

kblees mentioned this pull request May 21, 2015

Detect Git for windows Git root directory libgit2/libgit2#3040

Closed

kblees added 5 commits May 23, 2015 02:19

Merge branch 'kb/windows-wide-config'

0de57fc

kblees force-pushed the kb/windows-wide-config-gfw branch from 2bcfa09 to 0de57fc Compare May 23, 2015 20:12

dscho closed this May 27, 2015

kblees mentioned this pull request Jun 20, 2015

Where is system-wide gitattributes in git 2.4.4? #215

Closed

jeffhostetler added a commit to jeffhostetler/git that referenced this pull request Jul 9, 2019

Merge pull request git-for-windows#157 from jeffhostetler/vfs-222-sta…

9b1adf3

…tus-cache-untracked-hint status: deserialize with -uno does not print correct hint

jeffhostetler added a commit to jeffhostetler/git that referenced this pull request Aug 29, 2019

Merge pull request git-for-windows#157 from jeffhostetler/vfs-222-sta…

bd42007

…tus-cache-untracked-hint status: deserialize with -uno does not print correct hint

garimasi514 pushed a commit to garimasi514/git that referenced this pull request Jan 6, 2020

Merge pull request git-for-windows#157 from jeffhostetler/vfs-222-sta…

abdcc15

…tus-cache-untracked-hint status: deserialize with -uno does not print correct hint

jeffhostetler added a commit to jeffhostetler/git that referenced this pull request Apr 11, 2020

Merge pull request git-for-windows#157 from jeffhostetler/vfs-222-sta…

cd2a787

…tus-cache-untracked-hint status: deserialize with -uno does not print correct hint

jeffhostetler added a commit to jeffhostetler/git that referenced this pull request Apr 13, 2020

Merge pull request git-for-windows#157 from jeffhostetler/vfs-222-sta…

12d0cc0

…tus-cache-untracked-hint status: deserialize with -uno does not print correct hint

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: system-wide config file in %PROGRAMDATA% #157

RFC: system-wide config file in %PROGRAMDATA% #157

kblees commented May 21, 2015

dscho commented May 21, 2015

kblees commented May 23, 2015

dscho commented May 25, 2015

dscho commented May 25, 2015

dscho commented May 26, 2015

kblees commented May 26, 2015

kblees commented May 26, 2015

dscho commented May 27, 2015

dscho commented May 27, 2015

dscho commented May 27, 2015

RFC: system-wide config file in %PROGRAMDATA% #157

RFC: system-wide config file in %PROGRAMDATA% #157

Conversation

kblees commented May 21, 2015

dscho commented May 21, 2015

kblees commented May 23, 2015

dscho commented May 25, 2015

dscho commented May 25, 2015

dscho commented May 26, 2015

kblees commented May 26, 2015

kblees commented May 26, 2015

dscho commented May 27, 2015

dscho commented May 27, 2015

dscho commented May 27, 2015