-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Detect Git for windows Git root directory #3040
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
Conversation
|
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. |
|
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). |
|
How is that possible? libgit2 doesn't go looking for portable git installations...? |
|
@csware AppVeyor seems to tell that MSVC is tad unhappy with this PR. Could you please take a look at it? |
|
@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). |
|
@nulltoken It compiles cleanly on my VS 2013... Working on it |
|
Yeah, if it's in your 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. |
|
@ethomson But that fix needs to be on top of this in order to support older installations... |
|
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. |
|
As a short note: This affects the |
|
@ethomson is there anything in g4w2 that I can change to make your life less painful? |
|
@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 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 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... |
|
Suppose as this is a system config 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). |
|
Hmm. Yeah that's a very good point. :/ Is the registry idea crazy? That would at least prevent us from guessing. But sheesh, registry... :( |
|
|
|
|
%APPDATA% is the user's configuration. This is about addressing the system-wide (user-independent) configuration file
Sorry I misunderstood; you should then write to ProgramData for global app settings. Lots of other places would give you restricted write access issues. You can use the following things to get this location:
Env variable: ProgramData
Env variable: ALLUSERSPROFILE
Code: Environment.SpecialFolder.CommonApplicationData
|
I think it is the correct spot. Any preferred key? |
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?). |
|
Catching up on this thread, let me know if any of the things I raise here are no longer accurate:
I'm mostly 👎 on this as it could depend on being elevated (shout out to group policy).
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).
Yep, that's definitely something we can manage on our end. @coderangerdan - |
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. |
|
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...
|
|
Btw. I was just made aware, that there also is https://sourceforge.net/projects/msys2/, git of the msys2 package... |
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
Yep, but their To be fair, they also have a non-MSys2-based Git package ( My hope, and my plan, is to substitute their |
src/win32/findfile.c
Outdated
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOL!
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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... 😜 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much. :-)
although I believe they should not be part of this discussion.
That's right.
Thanks again. ^_^
@shiftkey Really? Because this says
And indeed a quick experiment showed that creating a folder inside So For reference, also see the Where Should I Write Program Data Instead of Program Files? blog post. |
That was my understanding which is why I suggested it. |
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 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 |
Yes. This is for:
It is the "system" configuration.
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:
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 |
That would introduce a really big problem: what is |
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 |
Yes, I agree that this is best handled by |
|
I've updated git-for-windows/git#157. Concerning the still controversial points:
All of this also works on Unix (replace The updated git-config(1) looks like this (stripped of all the config options): https://drive.google.com/file/d/0BxXUoUg2r8ftVjFHS3Z4d2NpcTQ/view?usp=sharing |
The documentation currently refers to both 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 You have to dig much deeper to discover that the ominous 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 |
I have a really strong opinion against repeating the
This is exactly the wrong way round. For example, we can easily put the path to the ca-certificates into Git for Windows' own As to your documentation updates: thank you very much. This helps tremendously. |
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.
And no, However, the concept of There are just entire worlds between |
Although you suggested this yourself here: #3040 (comment)
Yes, that's the idea. E.g. if upstream git added a shared
My first version just replaced
I don't see why the presence of a POSIX shell would be relevant, as neither gitconfig nor gitattributes are shell scripts. Using
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.
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. |
And if upstream decided to reuse the
It is not that I did not like it. Repeat after me:
Already the presence of POSIX shell scripts in
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' If you have two different Git for Windows versions installed, they both have separate, and potentially completely different
Oh, come on. By this rationale, we should use Do not let implementation details inform on design decisions! As I have repeated ad nauseam now: Do I have to list all the conceptual differences again?
Sure we can. But should we? The question here is a question of design. Just think about it: should 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).
In fact, I argued exactly the opposite: "By that rationale [i.e. using the name 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: It is a good idea, of course, to support setting Git attributes in That does not at all mean, though, that we should mistake Likewise, wanting support for Git attributes to be configured inside Summary:
|
Neither
The FHS defines
So
I fully agree.
Note that Git's [1] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap10.html |
|
This is I believe the core of the problem, and we can stop the discussion right here if we cannot sort this out:
The issue discussed here is that libgit2 would like to share / reuse the Git for Windows configuration. If install-time choices such as 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). |
|
The thing is: Windows-wide configuration should go to So no, you won't convince me that Git for Windows' own settings should be overridden by the much broader scoped My current plan is to cherry-pick from your Pull Request the parts that I like (which is almost everything except the wholesale |
|
@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 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' Now, does this sound like a convincing case or what? |
As the credential helper is installation-specific, it should only be configured in Precedence only matters for options that may reasonably be set in both config files, right? E.g. if the install-time choice for Now, if the installer was smarter about where to put install-time choices (e.g. But the combination "install-time choices go to |
|
In any case, it would still be possible to override installation-specific It is quite odd that a local system administrator (i.e. master of the Or do you actually need a feature to force an option to a specific value? I.e. some kind of |
Actually, the precedence is totally relevant: this example shows that the precedence I prefer makes more sense.
Now, this is also a good example: a sensible Windows-wide default for
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.
Of course! The specificity of scope, from broad to narrow is:
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
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...
No, that is exactly the wrong way round. Look, I ran a custom Git in my home directory for ages. It supported the 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. And |
|
@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 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 |
|
@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.
Now, easy. The Git for Windows installer actually already sets most of the options in As I planned it. As to And probably the percentage of users who actually change their system configuration themselves, manually, requires a good, modern microscope to be noticeable.
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.
The fact that I did not accept the wholesale 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. |
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]:
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 Git's design on Unix certainly does not require admins to mess around with files in
You should realize that Git built for your HOME directory will completely ignore 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.
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.
You've just torn a perfectly working solution to pieces, deliberately breaking Cygwin/Msys2 support, gitattributes and 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). |
|
@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. |
There was a problem hiding this comment.
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?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
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... |
|
Closed via #3475 Thanks for getting the ball rolling on this change! |



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.