search

git-for-windows / git

A fork of Git containing Windows-specific patches.
http://gitforwindows.org/
Other
8.28k stars 2.51k forks source link

Provide installer option to install per-user even if user is admin #4758

Open soredake opened 8 months ago

soredake commented 8 months ago

Setup

$ git --version --build-options

git version 2.43.0.windows.1
cpu: x86_64
built from commit: 4b968f3ea3b32a7bc50846bab49f3f381841d297
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
$ cmd.exe /c ver

Microsoft Windows [Version 10.0.22631.2861]
# One of the following:
> type "C:\Program Files\Git\etc\install-options.txt"
> type "C:\Program Files (x86)\Git\etc\install-options.txt"
> type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt"
> type "$env:USERPROFILE\AppData\Local\Programs\Git\etc\install-options.txt"
$ cat /etc/install-options.txt

Editor Option: VIM
Custom Editor Path:
Default Branch Option:
Path Option: Cmd
SSH Option: ExternalOpenSSH
Tortoise Option: false
CURL Option: OpenSSL
CRLF Option: CRLFAlways
Bash Terminal Option: MinTTY
Git Pull Behavior Option: Merge
Use Credential Manager: Enabled
Performance Tweaks FSCache: Enabled
Enable Symlinks: Disabled
Enable Pseudo Console Support: Disabled
Enable FSMonitor: Disabled

Details

PowerShell-Core

  winget install --no-upgrade --scope=user -h --accept-package-agreements --accept-source-agreements Git.Git --custom '"/COMPONENTS=`"icons,assoc,assoc_sh,,,,gitlfs,icons\quicklaunch`" /o:SSHOption=ExternalOpenSSH"'

I should not see UAC prompt and git should be installed to "$env:USERPROFILE\AppData\Local\Programs\Git", not to "C:\Program Files\Git"

I see UAC prompt in user scope and git is installed "C:\Program Files\Git" instead of "$env:USERPROFILE\AppData\Local\Programs\Git"

insert URL here

soredake commented 8 months ago

Related: https://github.com/git-for-windows/git/discussions/4399#discussioncomment-5877325 https://github.com/microsoft/winget-cli/issues/3240

dscho commented 7 months ago

Git for Windows uses InnoSetup to build the installer. There is an option to specify the required privileges, PrivilegesRequired, which is used by the installer (we even use the undocumented none value which was deprecated a long time ago, in favor of lowest, which would however not work for us because it would no longer allow installing in admin mode).

Having said that, I missed that InnoSetup 6.0.0 introduced a new option to allow overriding the install mode: PrivilegesRequiredOverridesAllowed. This might make it possible to offer a user-wide installation mode.

However, it won't be as simple as merely adding that option to the install.iss file: There are most likely parts of the code that expect to be installed into the system-wide location C:\Program Files\Git and that will need to be vetted carefully.

fadein commented 4 months ago

@dscho, I went ahead and rebuilt the installer according to your suggestion above... and it just worked. GfW is installed under my user's %AppData% and works beautifully. I have re-installed it many times with various combinations of options but have yet to find a single issue.

The nearest thing to a problem that I have encountered is that the Git Bash shortcut which Windows Explorer "helpfully" creates when you pin the app to the task bar referred to the path C:\Program Files\Git. This file was made by Windows under the path %AppData%\Roaming\Microsoft\Internet Explorer\QuickLaunch\TemproraryShortcuts and was not removed when I uninstalled GfW (which makes sense because the GfW installer did not create it). After I re-installed GfW to the new path this shortcut still pointed to the old path.

I scanned through build-extra/installer/install.iss but cannot find any hard coded instances of C:\Program Files. As far as I can tell, this change could be released. Can you tell me some things I could look out for that you are concerned could be broken by this change?

As for the patch, I just replaced the entire #ifdef at lines 81-85 with these two lines:

PrivilegesRequired=lowest
PrivilegesRequiredOverridesAllowed=dialog

It appeared to me that #ifdef was not working as expected, but I don't understand the significance of the OUTPUT_TO_TEMP symbol.

dscho commented 4 months ago

The nearest thing to a problem that I have encountered is that the Git Bash shortcut which Windows Explorer "helpfully" creates when you pin the app to the task bar referred to the path C:\Program Files\Git.

Hmm. @fadein this would most likely be due to some sort of caching, as the information used for pinning refers to "@@EXEPATH@@\\git-bash.exe where @@EXEPATH@@ gets interpolated to the location of git-bash.exe.

This file was made by Windows under the path %AppData%\Roaming\Microsoft\Internet Explorer\QuickLaunch\TemproraryShortcuts and was not removed when I uninstalled GfW (which makes sense because the GfW installer did not create it).

It would still be nice to remove it, we have a dedicated section for removing such shortcuts in the installer definition.

By the way, this is exactly what I was referring to careful vetting, going through the (admittedly long, almost 4k lines) install.iss file to see what might need to be adjusted.

It appeared to me that #ifdef was not working as expected, but I don't understand the significance of the OUTPUT_TO_TEMP symbol.

The OUTPUT_TO_TEMP constant is defined in config.iss, which is written by release.sh.

As for the patch, I just replaced the entire #ifdef at lines 81-85 with these two lines:

PrivilegesRequired=lowest
PrivilegesRequiredOverridesAllowed=dialog

It would probably need to be changed to something like this:

PrivilegesRequired=lowest
#ifndef OUTPUT_TO_TEMP
PrivilegesRequiredOverridesAllowed=dialog
#endif

I do wonder, though, whether that incurs a change of behavior for the use case I actually want to support: installing into C:\Program Files\Git. I could imagine that the proposed change will now add yet another dialog for the user to click through?

Besides, there is one rather big fall-out that we would suffer from this change: If we install into %LOCALAPPDATA%, naturally Git's config will also be located there. That breaks the contract with MinGit, which is shipping with a carefully modified system gitconfig that wants to inherit the configuration of the installed Git for Windows, if any:

[include]
    ; include Git for Windows' system config in order
    ; to inherit settings like \`core.autocrlf\`
    path = C:/Program Files (x86)/Git/etc/gitconfig
    path = C:/Program Files/Git/etc/gitconfig

This is crucial because that config contains settings that change Git's behavior, such as core.autoCRLF, http.sslBackend, filter.lfs.required, etc. If the regular Git for Windows is used to access repositories with these settings, and MinGit (used in 3rd-party applications) would be used without these settings, it could wreak quite a bit of havoc.

We could of course extend that [include] list by appending, say, path = ~/AppData/Local/Programs/Git/etc/gitconfig. That change would then percolate into the next Git for Windows, and more importantly, MinGit release. Which would be eventually picked up by downstream users such as Visual Studio and GitHub Desktop. Once those downstream users have integrated a MinGit version with this change, experience tells us that it will still take quite some time for users of those programs to update. Up until that time, if we were to make that installer change, we would break those setups.

There are millions of Visual Studio/GitHub Desktop users out there that would be negatively impacted if we rushed this change in the installer. This fact dictates that we need to tread very wisely here. The best course of action would be to make that MinGit change, verify that it actually works as intended, publish it with Git for Windows v2.46 scheduled for July 29th, 2024, and then wait an appropriate amount of time before integrating that install.iss change. Or maybe not wait, exactly, but use that time wisely to continue thinking about potentially undesirable consequences of making this change.