Skip to content

Conversation

@csware
Copy link
Contributor

@csware csware commented Apr 11, 2015

The https://git-for-windows.github.io package uses a different directory structure compared to msysgit and also providfes a 64-bit installer which is not handles correctly right now.

@csware csware changed the title Detect Git for windows Git root directory WIP: Detect Git for windows Git root directory Apr 11, 2015
@csware csware changed the title WIP: Detect Git for windows Git root directory Detect Git for windows Git root directory Apr 11, 2015
@nulltoken
Copy link
Member

/cc @dscho @shiftkey

@ethomson
Copy link
Member

So, uh, there's a couple of issues here.

Maybe it's time to consider an agreed upon global location for the configs. Right now we've all been intentionally using the g4w config, but if that's going to change then maybe we should change it with some plan for the future.

This is especially problematic with "portable Git". People who happen to ship that have their own global config and we cant interoperate.

@csware
Copy link
Contributor Author

csware commented Apr 11, 2015

The path detection works for msysgit, git for windows (also portable), github for windows and also cygwin (for cygwin, however, the template directory has another location).

@ethomson
Copy link
Member

How is that possible? libgit2 doesn't go looking for portable git installations...?

@nulltoken
Copy link
Member

@csware AppVeyor seems to tell that MSVC is tad unhappy with this PR. Could you please take a look at it?

@csware
Copy link
Contributor Author

csware commented Apr 11, 2015

@ethomson We're searching for git.exe and git.cmd on the %PATH%, so we already detect portable installations (as well as cygwin) before we try to detect msysgit and git-for-windows installations based on their uninstall information. Even if someone has multiple installations at the same time, we're using the one which comes on the %PATH% first (i.e., which windows finds first).

@csware
Copy link
Contributor Author

csware commented Apr 11, 2015

@nulltoken It compiles cleanly on my VS 2013... Working on it

@ethomson
Copy link
Member

Yeah, if it's in your PATH.

But if some vendor brings along a portable git installation and has some shiny icon to open it, but doesn't put it in the path... Then they're using portable git with one global git config and everything else with another.

This has been a problem for too long. Let's fix this.

@csware
Copy link
Contributor Author

csware commented Apr 11, 2015

@ethomson But that fix needs to be on top of this in order to support older installations...

@ethomson
Copy link
Member

There are no older installations. This PR targets compatibility with a beta. That's why I'm saying fix this, with everybody together now before G4W 2.0 ships their final release and it's too late and we all have more paths to care about.

@csware
Copy link
Contributor Author

csware commented Apr 11, 2015

As a short note: This affects the etc directory (for gitconfig and gitattributes) as well as (usr)/share/git-core/ for the templates directory.

@dscho
Copy link
Contributor

dscho commented Apr 11, 2015

@ethomson is there anything in g4w2 that I can change to make your life less painful?

@ethomson
Copy link
Member

@dscho I wonder if we can find some common place on the system to put the system config that we can all share that isn't the etc dir beneath one apps program files install dir.

I don't know what that would be though... Obviously I wouldn't want to do anything like the system drive root...

I think that %WINDIR% or whatever would be bad... What if we did a common name under program files for all the Git implementations? Program Files\Git\config ? That seems pretty weak though.

Maybe this is just not possible... But I feel like this would solve several problems at once if there was a good place to put this.

Orrrrrr.... Maybe a registry key that points out the location of the system wide locations?

I hate registry keys a lot but I hate them less than an ever increasing list of paths...

@csware
Copy link
Contributor Author

csware commented Apr 11, 2015

Suppose as this is a system config %PROGRAMDATA% or %CommonProgramFiles% (here we again have two folders, one for x86 and one special for x64) might be good places - problem is only if Git is installed for a local user (problematic regarding write permissions) and how to handle portable installations - there is no installer which can put default files to a common location.

A registry key might be the best option, but here we still have the problem with write permissions and x86 and x64 locations. - And for a portable version there is no installer which could set up the registry key or default config files (it also won't work for cygwin git).

@ethomson
Copy link
Member

Hmm. Yeah that's a very good point. :/

Is the registry idea crazy? That would at least prevent us from guessing. But sheesh, registry... :(

@danpetitt
Copy link

On 11 Apr 2015, at 16:04, Edward Thomson [email protected] wrote:

Hmm. Yeah that's a very good point. :/

Is the registry idea crazy? That would at least prevent us from guessing. But sheesh, registry... :(

Normally you would use %appdata% or environment variables which are cross platform

@ethomson
Copy link
Member

%APPDATA% is the user's configuration. This is about addressing the system-wide (user-independent) configuration file.

@danpetitt
Copy link

danpetitt commented Apr 11, 2015 via email

@dscho
Copy link
Contributor

dscho commented Apr 12, 2015

Is the registry idea crazy? That would at least prevent us from guessing. But sheesh, registry...

I think it is the correct spot. Any preferred key?

@dscho
Copy link
Contributor

dscho commented Apr 12, 2015

This is especially problematic with "portable Git". People who happen to ship that have their own global config and we cant interoperate.

Well, I would not worry about those. If there is a portable Git, I would expect it not to be interoperable with other Git installations on the same machine because the point of the portable Git is that it is not bound to the machine.

For GitHub for Windows, we can easily set the registry key, too (unless set already). I think. I'd have to ask them but they're pretty cool people (we sat together in the Starbucks, remember?).

@shiftkey
Copy link
Contributor

Catching up on this thread, let me know if any of the things I raise here are no longer accurate:

@ethomson

What if we did a common name under program files for all the Git implementations?

I'm mostly 👎 on this as it could depend on being elevated (shout out to group policy).

Is the registry idea crazy? That would at least prevent us from guessing. But sheesh, registry... :(

It's not that crazy - HKCU can be read/modified without requiring elevation, so I don't foresee it as a blocker for GitHub for Windows to support this. But if environment variables aren't an option then I guess this is more robust than checking a multitude of paths (and missing portable installs).

@dscho

For GitHub for Windows, we can easily set the registry key, too (unless set already). I think.

Yep, that's definitely something we can manage on our end.

@coderangerdan - %PROGRAMDATA% still requires elevation to modify, which leaves us with work to do to support portable installations like GitHub for Windows.

@danpetitt
Copy link

What if we did a common name under program files for all the Git implementations?

I'm mostly on this as it could depend on being elevated (shout out to group policy).

Is the registry idea crazy? That would at least prevent us from guessing. But sheesh, registry... :(

It's not that crazy - HKCU can be read/modified without requiring elevation, so I don't foresee it as a blocker for GitHub for Windows to support this. But if environment variables aren't an option then I guess this is more robust than checking a multitude of paths (and missing portable installs).

Registry HKCU is for current user and thus you may as well use %APPDATA% which is the more backup friendly location and imo the preferred way. Registry is urgh! I use it only when I have to which is to set file associations.

From what I can remember, there is nowhere that is global which doesn't require elevation as that is the whole point; anything which can affect more than current user will/should require elevation. Unless you just create your own folder somewhere and set your own permissions.

@ethomson
Copy link
Member

Hmm. So... let's say we do have a registry key for this... I feel like this is still merely a registry key that indicates where Git for Windows is installed, it's not really a registry key for where the system configuration would live.

Which is to say that if you uninstall Git for Windows, does that remove your system configuration file? If so, then that's not really a system config... it's an application configuration that other people also happen to read.

Surely there's some prior art here that I am not familiar with...

c:\windows\system32\etc? Blergh.

@csware
Copy link
Contributor Author

csware commented Apr 18, 2015

Btw. I was just made aware, that there also is https://sourceforge.net/projects/msys2/, git of the msys2 package...

@dscho
Copy link
Contributor

dscho commented Apr 18, 2015

c:\windows\system32\etc? Blergh.

Heh. What we really should do is add code to put the system-wide gitconfig itself into the registry ;-)

Seriously again, you are correct, we should use a central location for a system-wide configuration. I guess that we actually need to handle c:\windows\system32\etc\gitconfig as super system-wide configuration and for Git for Windows, handle /mingw??/etc/gitconfig in addition.

Btw. I was just made aware, that there also is https://sourceforge.net/projects/msys2/, git of the msys2 package...

Yep, but their git package is an MSys2-based one, i.e. it links to a light-weight version of Cygwin. I do not have the numbers handy, but switching to a non-MSys2 Git just blows the MSys2-based one right out of the water, speed-wise.

To be fair, they also have a non-MSys2-based Git package (mingw-w64-i686-git and mingw-w64-x86_64-git); the problem with those packages are that the Git contained in those packages does not pass the test suite at all. And if it does not pass the test suite, just imagine how many more issues it would display in every-day usage.

My hope, and my plan, is to substitute their mingw-w64-*-git packages with the packages we turned those into, over the past three months.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please, let's get the wording right once and for all, at least in here: msysgit is the development environment to create the MSYS1-based Git for Windows. msysgit is not intended for end-users, but for developers who want to work on Git for Windows itself.

So I'd propose to change the working as follows:

"git-for-windows" -> "MSYS2-based Git for Windows"
"msysgit" -> "MSYS1-based Git for Windows"

Also, it would be nice to break lines earlier to make the diff more readable on GitHub, and maybe even have proper punctuation (end sentences with a dot, start new sentences with an upper case letter).

Copy link
Member

Choose a reason for hiding this comment

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

I have a gif for this...

msysgit

Copy link
Contributor

Choose a reason for hiding this comment

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

LOL!

Copy link
Contributor

Choose a reason for hiding this comment

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

"git-for-windows" -> "MSYS2-based Git for Windows"
"msysgit" -> "MSYS1-based Git for Windows"

Let's be more precise:

git-for-windows -> Git for Windows 2.x
msysgit -> Git for Windows 1.x

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a gif for this...

Awesome! (Even if it is a .jpg here... 😜 )

Choose a reason for hiding this comment

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

Are these correct image?
history

cgywin_vs_msys

Copy link
Contributor

Choose a reason for hiding this comment

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

@YueLinHo I'll comment on the images although I believe they should not be part of this discussion.

All except the middle image look fine to me. The middle image seems to imply that you can run Cygwin binaries right away on Linux. That's not the case. You can run (many of) them with Wine on Linux, but that's also true for native Windows programs. In other words, if you intend to run a Windows binary on Linux, there's no benefit of having that binary linked against Cygwin / MSYS or not.

Choose a reason for hiding this comment

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

@sschuberth

Thank you so much. :-)

although I believe they should not be part of this discussion.

That's right.

Thanks again. ^_^

@sschuberth
Copy link
Contributor

@coderangerdan - %PROGRAMDATA% still requires elevation to modify

@shiftkey Really? Because this says

ProgramData specifies the path to the program-data folder (normally C:\ProgramData). Unlike the Program Files folder, this folder can be used by applications to store data for standard users, because it does not require elevated permissions.

And indeed a quick experiment showed that creating a folder inside %PROGRAMDATA% did not trigger the UAC prompt while creating on inside %PROGRAMFILES% did.

So %PROGRAMDATA% is my personal favorite solution. No registry please if we can avoid it.

For reference, also see the Where Should I Write Program Data Instead of Program Files? blog post.

@danpetitt
Copy link

@sschuberth

And indeed a quick experiment showed that creating a folder inside %PROGRAMDATA% did not trigger the UAC prompt while creating on inside %PROGRAMFILES% did.

So %PROGRAMDATA% is my personal favorite solution. No registry please if we can avoid it.

That was my understanding which is why I suggested it.

@dscho
Copy link
Contributor

dscho commented May 22, 2015

I strongly believe that this change needs to be in the upstream documentation, so that it is available to a wider audience.

I agree that it should be upstream eventually. But it makes more sense to hash out the remaining unclear points between us Windows types before sending it upstream.

After all, you do not want to have a version of your patch series accepted by (the Linux experts) upstream only to find out that literally all Windows projects disagree with your approach.

Now, I tried to implement an approach that would support all of the crazy edge cases I could think of, instead of preventing them (which switching from /etc/gitconfig to %PROGRAMDATA%\Git\config wholesale would have accomplished).

This implementation made it into no official Git for Windows yet, so the subject is still open.

As I said, though, it will take a convincing argument to change my mind about the /etc/gitconfig vs %PROGRAMDATA%\Git\config strategy I ended up implementing.

@kblees
Copy link

kblees commented May 22, 2015

  1. Do we want to have some shared configuration state between the various tools that is computer-wide?

Yes. This is for:

  • network settings (e.g. http proxy, smtp server)
  • OS-specific settings (e.g. pack.packsizelimit, core.ignorecase)
  • installed 3rd-party software (e.g. diff and merge tools, fonts to use in git-gui)
  1. If yes, is this the "system" configuration, or is it something else? If it is not the "system" configuration, what is its name?

It is the "system" configuration.

  1. Do we want to have a configuration space for tools themselves, that is computer-wide but only for that particular tool?

Yes, but I would call this "installation-wide" rather than "computer-wide" (you might have different installations of the same tool, e.g. to try out new versions).

This would be for:

  • installation-specific settings (e.g. help.format if the installation only includes a specific format)
  • build-specific settings (e.g. pack window- and thread limits for 32-bit builds)
  • vendor-specific settings (e.g. pre-configured aliases and remotes)
  1. If yes, is this the "system" configuration, or is it something else? If it is not the "system" configuration, what is its name?

I would call it "installation-specific". These settings are definied when packaging the software (i.e. before installing it), and are typically read-only after the software is installed. Therefore I don't think we need a git config --installation option for it.

@dscho
Copy link
Contributor

dscho commented May 22, 2015

If yes, is this the "system" configuration, or is it something else? If it is not the "system" configuration, what is its name?

It is the "system" configuration.

That would introduce a really big problem: what is /etc/gitconfig, then? And how would you change the documentation without breaking non-Windows expectations?

@kblees
Copy link

kblees commented May 22, 2015

Whereas the core.autocrlf setting most likely should be false for Cygwin, but not for Git for Windows.

You can still edit files with Notepad and then commit them with Cygwin-Git, so it should probably be true for Cygwin as well.

However, whether you want CRLF or LF-only in the repository would actually be a project-specific policy. I.e. a .gitconfig file that is part of the repository (and also perhaps hooks for policy checks). But this is an entirely separate issue.

@ethomson
Copy link
Member

However, whether you want CRLF or LF-only in the repository would actually be a project-specific policy.

Yes, I agree that this is best handled by .gitattributes. It would be really nice to start downplaying the prevalence of core.autocrlf settings. It seems particularly presumptuous to make this a system setting.

@kblees
Copy link

kblees commented May 23, 2015

I've updated git-for-windows/git#157. Concerning the still controversial points:

  • It uses %PROGRAMDATA%\Git\gitconfig. This can be changed to just ...\Git\config via make parameter, but would also change $(prefix)/etc/gitconfig to $(prefix)/etc/config.
  • $(prefix)/etc/gitconfig is read before %PROGRAMDATA%\Git\gitconfig (i.e. the latter can override settings of the former).
  • git config --system reads / writes %PROGRAMDATA%\Git\gitconfig (which I think was the predominant opinion on this thread). However, make sysconfdir=etc (with a relative path) builds a sandboxed version that only uses $(prefix)/etc/gitconfig (i.e. this restores the current behaviour that the "system-wide" config is in the installation directory).

All of this also works on Unix (replace %PROGRAMDATA%\Git\ above with /etc).

The updated git-config(1) looks like this (stripped of all the config options): https://drive.google.com/file/d/0BxXUoUg2r8ftVjFHS3Z4d2NpcTQ/view?usp=sharing

@kblees
Copy link

kblees commented May 24, 2015

If yes, is this the "system" configuration, or is it something else? If it is not the "system" configuration, what is its name?

It is the "system" configuration.

That would introduce a really big problem: what is /etc/gitconfig, then? And how would you change the documentation without breaking non-Windows expectations?

The documentation currently refers to both /etc/gitconfig and $(prefix)/etc/gitconfig as the "system-wide" configuration, sometimes even in the same sentence.

Where "$(prefix)" would be completely meaningless to ordinary users that have never seen an autoconf-style Makefile.

And users that finally dicover git's Makefile will find out that the default is prefix = $(HOME), so the "system-wide" configuration is actually user-specific!

You have to dig much deeper to discover that the ominous $(prefix) is only used if $(sysconfdir) is a relative path (which is the default unless prefix=/usr)...

Bottom line is that the current documentation is completely broken wrt. the "system-wide" config, and understanding it requires intimate knowledge of both the build system and the source code.

I tried to fix this by removing $(prefix) almost everywhere, and explaining what it means in the single place where it remains.

@dscho
Copy link
Contributor

dscho commented May 25, 2015

It uses %PROGRAMDATA%\Git\gitconfig.

I have a really strong opinion against repeating the git substring. It might appear that keeping the name gitconfig would simplify the code because now all /etc/* accesses could be redirected to %PROGRAMDATA%\Git. However, that would make too many assumptions about the latter location. It is in no way like /etc/. For one, it is Git-specific, which /etc/ is not. Further, we cannot assume any POSIX semantics, let alone the presence of a POSIX shell in %PROGRAMDATA%\Git, while /etc/ assumes exactly that. The truth is: /etc/ and %PROGRAMDATA%\Git are just two very different things.

$(prefix)/etc/gitconfig is read before %PROGRAMDATA%\Git\gitconfig (i.e. the latter can override settings of the former).

This is exactly the wrong way round. %PROGRAMDATA%\Git\config is supposed to be the central place for all Git software running on the same machine. As a consequence, those settings must be the most generic of all Git config files. The specific Git implementations might want to override or augment those settings, therefore their config files (such as Git for Windows' /mingw64/etc/gitconfig) must get a higher priority, i.e. be read later than the Windows-wide configuration.

For example, we can easily put the path to the ca-certificates into Git for Windows' own gitconfig: 1) we can use a POSIX path, and 2) as long s Git for Windows is installed, that path will be valid, and when Git for Windows is installed, both the ca-certificates as well as the configuration pointing there will be gone. Both points are not true with the Windows-wide configuration.

As to your documentation updates: thank you very much. This helps tremendously.

@sschuberth
Copy link
Contributor

For one, it is Git-specific, which /etc/ is not.

Why does that matter for any real world usage?

let alone the presence of a POSIX shell in %PROGRAMDATA%\Git, while /etc/ assumes exactly that.

/etc/ assumes that a POSIX shell is installed below /etc/?

@dscho
Copy link
Contributor

dscho commented May 26, 2015

For one, it is Git-specific, which /etc/ is not.

Why does that matter for any real world usage?

I matters from a maintenance perspective. The clearer the concepts are, the easier it is to maintain the different software packages.

let alone the presence of a POSIX shell in %PROGRAMDATA%\Git, while /etc/ assumes exactly that.

/etc/ assumes that a POSIX shell is installed below /etc/?

/etc/ is a POSIX path.

And no, /etc/ does not assume that a POSIX shell is installed below /etc/. Of course, I never said that, either.

However, the concept of /etc/ assumes that there is a shell, as many files inside /etc/ are shell scripts, activated by being marked as executable.

There are just entire worlds between /etc/ and %PROGRAMDATA%\Git.

@kblees
Copy link

kblees commented May 26, 2015

It uses %PROGRAMDATA%\Git\gitconfig.

I have a really strong opinion against repeating the git substring.

Although you suggested this yourself here: #3040 (comment)

It might appear that keeping the name gitconfig would simplify the code because now all /etc/* accesses could be redirected to %PROGRAMDATA%\Git.

Yes, that's the idea. E.g. if upstream git added a shared /etc/gitignore feature, it would automatically work without further changes on our side.

However, that would make too many assumptions about the latter location. It is in no way like /etc/. For one, it is Git-specific, which /etc/ is not.

My first version just replaced /etc with %PROGRAMDATA% (no \Git), and you didn't like that either...

Further, we cannot assume any POSIX semantics, let alone the presence of a POSIX shell in %PROGRAMDATA%\Git, while /etc/ assumes exactly that. The truth is: /etc/ and %PROGRAMDATA%\Git are just two very different things.

I don't see why the presence of a POSIX shell would be relevant, as neither gitconfig nor gitattributes are shell scripts.

Using gitconfig instead of just config has additional advantages:

  • find -name gitconfig can be used to find system-wide config files, independent of the platform (i.e. also works on Unix).
  • Documentation: The general rule "replace /etc with %PRGRAMDATA%\Git" is reasonably simple that I think its sufficient to document this in a few central places (one ATM). Doing the same thing with a rule that is not based on path boundaries (i.e. "replace /etc/git* with %PRGRAMDATA%\Git\*") makes me uneasy. I think we would have to explicitly mention the Windows specific path much more often.

$(prefix)/etc/gitconfig is read before %PROGRAMDATA%\Git\gitconfig (i.e. the latter can override settings of the former).

This is exactly the wrong way round. %PROGRAMDATA%\Git\config is supposed to be the central place for all Git software running on the same machine. As a consequence, those settings must be the most generic of all Git config files. The specific Git implementations might want to override or augment those settings, therefore their config files (such as Git for Windows' /mingw64/etc/gitconfig) must get a higher priority, i.e. be read later than the Windows-wide configuration.

The content of /mingw64/etc/gitconfig is defined by us when we build the installer, and it will be shared by all machines that install Git for Windows. I.e. these settings are much more generic than the Windows-wide config on any single machine.

For example, we can easily put the path to the ca-certificates into Git for Windows' own gitconfig:

We can still do this even if Git for Windows' own gitconfig has the lowest priority.

However, an administrator should be able to override the ca-certificates in the Windows-wide config, e.g. by specifying an absolute Windows path to a ca-bundle that also includes company-private ca-certificates. This would not be possible if Git for Windows' gitconfig had higher priority, i.e. such ca-bundle files would have to be configured per-user or per-repo.

@dscho
Copy link
Contributor

dscho commented May 27, 2015

E.g. if upstream git added a shared /etc/gitignore feature, it would automatically work without further changes on our side.

And if upstream decided to reuse the /etc/init.d/ infrastructure, we would face a problem. Why? Because we would have blatantly ignored the golden rule of software design: implementation details should not inform on design decisions.

My first version just replaced /etc with %PROGRAMDATA% (no \Git), and you didn't like that either...

It is not that I did not like it.

Repeat after me: etc/ has POSIX semantics. etc/ is very specifically POSIX. %PROGRAMDATA% has completely different meaning and semantics. Mixing these two concepts does not make things easier, it just would make things messy and unmaintainable. We must keep clear concepts.

I don't see why the presence of a POSIX shell would be relevant, as neither gitconfig nor gitattributes are shell scripts.

Already the presence of POSIX shell scripts in etc/ should have told you all you need to know: etc/ is conceptually a completely different beast from %PROGRAMDATA% and from %PROGRAMDATA\Git. Completely different. etc/ is not even a Windows concept, for crying out loud. It is different! It is an alien! It is specific to POSIX!

The content of /mingw64/etc/gitconfig is defined by us when we build the installer

Incorrect. We ship a template that is defined when I build the installer.

And then the installer adjusts it based on the user input at installation time.

And why do we do this? Because Git for Windows' etc/gitconfig is specific to each particular Git for Windows installation.

If you have two different Git for Windows versions installed, they both have separate, and potentially completely different etc/gitconfig files. And that is exactly how it should be.

The general rule "replace /etc with %PRGRAMDATA%\Git" is reasonably simple

Oh, come on. By this rationale, we should use rm -rf / when uninstalling Git for Windows because it is even simpler than what you suggest. It is the unintended consequences that make both approaches not only simpler, but also wrong.

Do not let implementation details inform on design decisions!

As I have repeated ad nauseam now: etc/ and %PROGRAMDATA%\Git are two very different things, from a design perspective. You just cannot consider them equivalent.

Do I have to list all the conceptual differences again?

For example, we can easily put the path to the ca-certificates into Git for Windows' own gitconfig:

We can still do this even if Git for Windows' own gitconfig has the lowest priority.

Sure we can. But should we?

The question here is a question of design.

Just think about it: should $HOME/.gitconfig override /etc/gitconfig? This is a very good question, and once you understand the rationale that the former should have a higher priority than the latter (because the per-user scope is more specific than the system-wide scope), you cannot help but agree that Git for Windows' etc/gitconfig should have a higher priority than %PROGRAMDATA%\Git\config (because Git for Windows' scope is more specific than Windows' scope).

I really hope that you agree with this argument, because I have no idea what else would convince you (and this entire discussion only managed to make my understanding of the issue clearer, nothing of what I read came even close to refute my argument).

It uses %PROGRAMDATA%\Git\gitconfig.

I have a really strong opinion against repeating the git substring.

Although you suggested this yourself here: #3040 (comment)

In fact, I argued exactly the opposite: "By that rationale [i.e. using the name gitconfig because the documentation talks about $prefix/etc/gitconfig], we would have to put git.exe into %PROGRAMDATA%\Git\bin\ because you equate ${prefix} with %PROGRAMDATA%\Git now. Are you sure you want to pursue that line of reasoning?" In other words, this was an argument against repeating the "git" substring. Additionally, it was an argument demonstrating how non-sensical it would be to mistake ${prefix} for the same as %PROGRAMDATA%\Git (just like etc/ is an entirely different concept than %PROGRAMDATA%\Git).

In any case, could we please get away from finger pointing ("you said that, I said that") and concentrate on the matter of the discussion again? The matter being that there still seems to be some misunderstanding floating about: etc/ is not the same as %PROGRAMDATA%\Git. They are conceptually, scope-wise and practically different. Very much so.

It is a good idea, of course, to support setting Git attributes in %PROGRAMDATA%\Git, for all Git programs running on the same machine.

That does not at all mean, though, that we should mistake %PROGRAMDATA%\Git for Git for Windows' new etc/ directory. That would just be wrong. I offered quite a bit of evidence to back that up already.

Likewise, wanting support for Git attributes to be configured inside %PROGRAMDATA%\Git does not at all imply that we have to call the file gitattributes. It is a good thing, of course, if the character sequence "git" appears in the path of the configuration files. Once. E.g. /etc/gitconfig, $HOME/.gitconfig, .git/config, /etc/gitattributes, .git/info/attributes. Please note that the character sequence "git" shows up in those paths exactly once, respectively, and please note further that all of these basenames are different, even when they share roles (but differ in scope). In all of these cases, the design dictated the implementation, not the other way round.

Summary:

  • We have to decide on a case-by-case basis which type of configuration we want to support, and which file name to use for the configuration, in %PROGRAMDATA%\Git.
  • The configuration in %PROGRAMDATA%\Git is the broadest possible Git configuration per machine. It therefore must take the lowest precedence, e.g. every Git config file should be able to override what %PROGRAMDATA%\Git\config specifies.

@kblees
Copy link

kblees commented May 27, 2015

Repeat after me: etc/ has POSIX semantics. etc/ is very specifically POSIX.

Neither /etc/ nor etc/ are mentioned by POSIX whatsowever [1]

%PROGRAMDATA% has completely different meaning and semantics.

The FHS defines /etc as follows [2]: "The /etc hierarchy contains configuration files. A "configuration file" is a local file used to control the operation of a program; it must be static and cannot be an executable binary."

%PROGRAMDATA% is the "Default Location for Shared Application data and settings" [3]

So %PROGRAMDATA% is broader than /etc in that it also includes what would be /var on Unix. With respect to configuration files, or "settings", %PROGRAMDATA% and /etc are equivalent.

The matter being that there still seems to be some misunderstanding floating about: etc/ is not the same as %PROGRAMDATA%\Git. They are conceptually, scope-wise and practically different. Very much so.

I fully agree.

etc/ is relative to the installation directory (i.e. $(prefix)/etc/).

/etc/ is the host-wide configuration directory (i.e. equivalent to %PROGRAMDATA%[\Git] on Windows).

Note that Git's system_path() function has always distinguished between absolute and relative paths in this way, on all platforms. My PR doesn't change that, it just plugs into the existing absolute path logic, and enables the absolute sysconfdir=/etc setting on Windows (which wasn't possible before, as /etc is not an absolute Windows path).

[1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap10.html
[2] http://www.pathname.com/fhs/pub/fhs-2.3.html#ETCHOSTSPECIFICSYSTEMCONFIGURATION
[3] https://www.microsoft.com/en-us/download/details.aspx?id=22322

@kblees
Copy link

kblees commented May 27, 2015

This is I believe the core of the problem, and we can stop the discussion right here if we cannot sort this out:

... Git for Windows' etc/gitconfig should have a higher priority than %PROGRAMDATA%\Git\config (because Git for Windows' scope is more specific than Windows' scope).

We ship a template that is defined when I build the installer.

And then the installer adjusts it based on the user input at installation time.

And why do we do this? Because Git for Windows' etc/gitconfig is specific to each particular Git for Windows installation.

If you have two different Git for Windows versions installed, they both have separate, and potentially completely different etc/gitconfig files. And that is exactly how it should be.

The issue discussed here is that libgit2 would like to share / reuse the Git for Windows configuration.

If install-time choices such as core.autocrlf end up in Git for Windows' installation-specific config file, and if these settings also override the windows-wide settings, we are back to square one. IOW setting core.autocrlf in the windows-wide config file will have no effect and libgit2 will still have to search for Git for Windows installation directories to achieve the desired goal.

This is very sad to say, but unless you change your position especially on load order / priority, I'm afraid we will not find a solution. I suggest we don't make this any worse and close the issue as "won't fix" (or rather: Git for Windows won't support windows-wide config files in the desired way, and libgit2 should continue to search for existing Git for Windows installations).

@dscho
Copy link
Contributor

dscho commented May 27, 2015

The thing is: Windows-wide configuration should go to %PROGRAMDATA%\Git. And Git for Windows should be able to override it.

So no, you won't convince me that Git for Windows' own settings should be overridden by the much broader scoped %PROGRAMDATA%\Git. Not unless you have a very good argument (which you hid from me so far).

My current plan is to cherry-pick from your Pull Request the parts that I like (which is almost everything except the wholesale /etc/ -> %PROGRAMDATA%\Git redirection, which is just simply wrong). Then you won't have to agree that broader scope means less precedence, first, second, and always, even if it would be sad to see.

@dscho
Copy link
Contributor

dscho commented May 27, 2015

@kblees Okay, I try once more, because I really believe it is important that you understand why the approach you chose is not the best way.

Just keep in mind that Git for Windows is not the same as GitHub for Windows, but both are based off of the same code base.

Now, GitHub for Windows ships with a different credential helper than Git for Windows. This is legitimate.

If your wish came true, i.e. that %PROGRAMDATA%\Git\config had a higher precedence than the individual etc/gitconfig files, this would be simply impossible.

But it should be possible, of course. And it is possible if the config files with the narrower scope (i.e. Git for Windows', and GitHub for Windows' etc/gitconfig) take higher precedence, as I am still convinced is the better design.

Now, does this sound like a convincing case or what?

@kblees
Copy link

kblees commented May 27, 2015

Now, GitHub for Windows ships with a different credential helper than Git for Windows. This is legitimate.

If your wish came true, i.e. that %PROGRAMDATA%\Git\config had a higher precedence than the individual etc/gitconfig files, this would be simply impossible.

As the credential helper is installation-specific, it should only be configured in etc/gitconfig, not in %PROGRAMDATA%\Git\config. Thus, precendence is irrelevant.

Precedence only matters for options that may reasonably be set in both config files, right?

E.g. if the install-time choice for core.autocrlf ends up in etc/gitconfig, and etc/gitconfig overrides %PROGRAMDATA%\Git\config, then setting core.autocrlf in %PROGRAMDATA%\Git\config will have no effect. And if libgit2 or JGit wanted to respect the install-time choice, they would have to search the installation directories rather than just checking the windows-wide config.

Now, if the installer was smarter about where to put install-time choices (e.g. core.autocrlf -> %PROGRAMDATA%\Git\config), precendence wouldn't be such a big issue.

But the combination "install-time choices go to etc/gitconfig and etc/gitconfig overrides %PROGRAMDATA%\Git\config" makes no sense to me at all.

@kblees
Copy link

kblees commented May 27, 2015

In any case, it would still be possible to override installation-specific etc/gitconfig settings in the per-user and per-repo config files, including the credential helper? That is, each individual user will have to make sure to only set config options that are compatible with all Git versions they intend to use.

It is quite odd that a local system administrator (i.e. master of the %PROGRAMDATA%\Git\config file) should be deprived of this possibility, even though she is in a much better position to know which options will be compatible with the installed Git versions.

Or do you actually need a feature to force an option to a specific value? I.e. some kind of etc/final-gitconfig that is loaded last?

@dscho
Copy link
Contributor

dscho commented May 27, 2015

As the credential helper is installation-specific, it should only be configured in etc/gitconfig, not in %PROGRAMDATA%\Git\config. Thus, precendence is irrelevant

Actually, the precedence is totally relevant: this example shows that the precedence I prefer makes more sense.

E.g. if the install-time choice for core.autocrlf ends up in etc/gitconfig, and etc/gitconfig overrides %PROGRAMDATA%\Git\config, then setting core.autocrlf in %PROGRAMDATA%\Git\config will have no effect.

Now, this is also a good example: a sensible Windows-wide default for core.autocrlf is input or true, while Cygwin's Git should totally set core.autocrlf to false. And the correct precedence order allows that.

But the combination "install-time choices go to etc/gitconfig and etc/gitconfig overrides %PROGRAMDATA%\Git\config" makes no sense to me at all.

Well, then you can do something about it. If you want to let the Git for Windows installer change the Windows-wide settings, do it, but also make sure that you have convincing arguments to defend that change.

it would still be possible to override installation-specific etc/gitconfig settings in the per-user and per-repo config files, including the credential helper?

Of course! The specificity of scope, from broad to narrow is:

  1. %PROGRAMDATA%\Git\config
  2. (for Git for Windows) etc/gitconfig
  3. %HOME%\.gitconfig
  4. %GIT_DIR%\.git\config

And that is exactly the reverse order of precedence. Beautiful, isn't it?

(And yes, in the Windows context I just ignored that there is also support for $XDG_CONFIG_HOME)

That is, each individual user will have to make sure to only set config options that are compatible with all Git versions they intend to use.

Now, why exactly should that be odd? It always has been the case! Of course your dotfiles must be valid for whatever software you intend to run...

It is quite odd that a local system administrator (i.e. master of the %PROGRAMDATA%\Git\config file) should be deprived of this possibility, even though she is in a much better position to know which options will be compatible with the installed Git versions.

No, that is exactly the wrong way round. Look, I ran a custom Git in my home directory for ages. It supported the receive.denyCurrentBranch = updateInstead long before upstream Git did (because I am a lazy bastard and I only submitted the patches last fall).

This is a totally legitimate use case.

However, if the system administrator could override my Git config, as you seem to suggest, I would never have been able to override the system-wide settings.

Which is why the precedence of the Git config files is in reverse order of the broadness of scope. I.e. /etc/gitconfig < $HOME/.gitconfig < $GIT_DIR/.git/config.

And %PROGRAMDATA%\Git\config has a very logical place in that sequence: it fits snugly to the left of /etc/gitconfig because that /etc/gitconfig actually lives inside a specific application folder inside C:\Program Files, i.e. its scope is substantially narrower than the one of %PROGRAMDATA%\Git\config which is bound to no specific application at all.

@kblees
Copy link

kblees commented May 27, 2015

@dscho It seems that any further discussion is pointless, as you already did your own thing and closed my PR (as you did with @sschuberth 's issue that git config --system doesn't use the windows-wide config).

Bottom line for libgit2 and JGit is that you will still have to search for installation-specific config files, because settings in the windows-wide config file will be overridden by the Git for Windows installer and anything added via git config --system. Cygwin-git and MSys2-git will not support the windows-wide config, and there will be no windows-wide gitattributes either.

@dscho
Copy link
Contributor

dscho commented May 27, 2015

@kblees if you have any convincing arguments against the current precedence order, please do not hesitate to tell me. So far, I have only seen arguments that I could, and did, refute easily, to the point that those arguments even disagreed with the current state of Git's very own design.

Bottom line for libgit2 and JGit is that you will still have to search for installation-specific config files, because settings in the windows-wide config file will be overridden by the Git for Windows installer and anything added via git config --system.

Now, easy. The Git for Windows installer actually already sets most of the options in %PROGRAMDATA%\Git\config. So libgit2 and JGit can find them, change them, and all is groovy.

As I planned it.

As to git config --system, yes, you are correct, this command errs on the side of caution and only affects Git for Windows. Big deal. If anyone really wants to change the global default, and thinks they really know what they're doing, they can use git config -f %PROGRAMDATA%\Git\config, as does our installer for settings such as core.autocrlf. Easy enough.

And probably the percentage of users who actually change their system configuration themselves, manually, requires a good, modern microscope to be noticeable.

Cygwin-git and MSys2-git will not support the windows-wide config,

Sure they will, if there are people willing to make that work. I plan to submit the patch series upstream anyway, including your two very nice cleanup patches. Then it should not take all that much effort for Cygwin or MSys2 to pick up the support for the Windows-wide configuration.

and there will be no windows-wide gitattributes either.

The fact that I did not accept the wholesale /etc/ -> %PROGRAMDATA%\Git redirection does not imply that I am against having %PROGRAMDATA%\Git\attributes. The opposite is true, as can be read in at least one comment above: I like the idea.

But it will take a less heavy-handed patch to support that, and of course it will even more so take contributors who make that support work in libgit2, JGit, Cygwin Git, etc.

@kblees
Copy link

kblees commented Jun 1, 2015

So far, I have only seen arguments that I could, and did, refute easily, to the point that those arguments even disagreed with the current state of Git's very own design.

Wow, really? I have to admit that this bold statement really left me kind of speechless.

In the faint hope that we may still find a sound solution, let me try once more with just one of the basic rules for software development on Windows [1]:

  • The %PROGRAMFILES% directory is for static, read-only files of an application (executables and support files), similar to the /usr directory on Unix. Except for installation programs, nobody is supposed to write there, ever. Apart from permissions and UAC protection, a very practical reason for this is that upgrading the application should not lose any data.
  • The %PORGRAMDATA% directory (former %ALLUSERSPROFILE%) is for application data and settings that are shared by all users, similar to the /etc and /var directories on Unix.
  • The %USERPROFILE% (or even better: %APPDATA% or %LOCALAPPDATA%) directory is for user-specific application data and settings, similar to $HOME on Unix.

20 years ago it may have been state of the art to store config files in the installation directory. But its 2015 and we're not talking about "Git for DOS" here.

While we'll probably never certify Git for Windows with the Windows Logo program, we should nevertheless follow the basic rules of conduct - to deserve the "for Windows" label, and to be taken seriously as a mature, professional software product.

If I tell my software deployment guys that after each upgrade, they need to patch %PROGRAMFILES%/Git/.../gitconfig (or .../ca-bundle.crt), because these files override the machine-wide configuration in %PROGRAMDATA%, they'll just laugh at me and kick me out the door.

Git's design on Unix certainly does not require admins to mess around with files in /usr. So can we please agree that writing to the %PROGRAMFILES% directory is off limits? And in consequence, that the config file in %PROGRAMDATA% needs to have higher precedence and should also be used by git config --system?

I ran a custom Git in my home directory for ages....However, if the system administrator could override my Git config, as you seem to suggest

You should realize that Git built for your HOME directory will completely ignore /etc/gitconfig.

You should also realize that your current solution does not allow to build such a sandboxed Git version on Windows, because you blatantly ignored Git's design to make things configurable via Makefile settings.

Bottom line for libgit2 and JGit is that you will still have to search for installation-specific config files, because settings in the windows-wide config file will be overridden by the Git for Windows installer and anything added via git config --system.

Now, easy. The Git for Windows installer actually already sets most of the options in %PROGRAMDATA%\Git\config. So libgit2 and JGit can find them, change them, and all is groovy.

You claimed the exact opposite above, multiple times. Even though I made it abundantly clear (also multiple times) that this in conjunction with your proposed precedence order was the main issue.

May I kindly ask you to verify your arguments before sidetracking the discussion with false claims? It could save us all a lot of time.

Cygwin-git and MSys2-git will not support the windows-wide config,

Sure they will, if there are people willing to make that work.

You've just torn a perfectly working solution to pieces, deliberately breaking Cygwin/Msys2 support, gitattributes and git config --system.

So why should anyone take up that work, again?

[1] This is technical requirement #2 in the Windows 7 Client Software Logo program (https://www.microsoft.com/en-us/download/details.aspx?id=3859).

@csware
Copy link
Contributor Author

csware commented Jun 19, 2015

@dscho: I'm a bit confused. Today I tried the 2nd release candidate and now, there is no gitconfig file in any etc directory any more - there only is a config file in the %PROGRAMDATA%\Git directory which contains all settings which were in etc/gitconfig in git for windows based on msys1. - Based on the discussion on the here, there should still be a etc/gitconfig file containing the installation settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

might this cause trouble if there is a file called /etc/config on *nix based systems?!

Choose a reason for hiding this comment

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

thanks

@csware
Copy link
Contributor Author

csware commented Jun 19, 2015

Well, talking again about a portable git distribution. Image a portable git package is on the %PATH% and libgit2 is used to find a system path. Then I would expect libgit2 to also use the settings which are defined in the portable git package as these are used when I call git.exe on the cli (e.g., for autocrlf) which might also differ from the %PROGRAMDATA%\Git\config settings. So, I think I prefer my original PR, where I tried to find the etc folder of a git for windows installation - this local gitconfig file (if it exists; see my previous comment #3040 (comment)) should be used in preference to the %PROGRAMDATA%\Git\config file...

@ethomson
Copy link
Member

ethomson commented Nov 2, 2015

Closed via #3475

Thanks for getting the ball rolling on this change!

@ethomson ethomson closed this Nov 2, 2015
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.

10 participants