Setup moves git to the end of PATH

[the-ress]

• I was not able to find an open or closed issue matching what I'm seeing

Setup

• Which version of Git for Windows are you using? Is it 32-bit or 64-bit?

$ git --version --build-options

git version 2.20.1.windows.1
cpu: x86_64
built from commit: 7c9fbc07db0e2939b36095df45864b8cda19b64f
sizeof-long: 4
sizeof-size_t: 8

• Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?

$ cmd.exe /c ver

Microsoft Windows [Version 10.0.17134.441]

• What options did you set as part of the installation? Or did you choose the
  defaults?

# 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"
$ cat /etc/install-options.txt

Editor Option: VisualStudioCode
Custom Editor Path:
Path Option: Cmd
SSH Option: OpenSSH
CURL Option: OpenSSL
CRLF Option: CRLFCommitAsIs
Bash Terminal Option: ConHost
Performance Tweaks FSCache: Enabled
Use Credential Manager: Enabled
Enable Symlinks: Disabled

• Any other interesting things about your environment that might be related
  to the issue you're seeing?

My PATH includes c:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TeamFoundation\Team Explorer (to run tf.exe).

Details

• Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

CMD, PowerShell, Total Commander

• What commands did you run to trigger this issue? If you can provide a
  Minimal, Complete, and Verifiable example
  this will help us understand the issue.

1. Install git-for-windows
2. Move C:\Program Files\Git\cmd up in the PATH environment variable
3. Upgrade git-for-windows to a newer version

• What did you expect to occur after running these commands?

C:\Program Files\Git\cmd should stay where I moved it.

• What actually happened instead?

C:\Program Files\Git\cmd is moved to the end of the list.

This breaks running git from Total Commander because it opens the Git folder in c:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TeamFoundation\Team Explorer instead.

[dscho]

c:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TeamFoundation\Team Explorer

How did this end up in the PATH? It should not be there. Besides, git.exe does not reside in that directory for me, you would need c:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TeamFoundation\Team Explorer\Git\cmd in the PATH...

Back to your question: you would like Git to remember the position in the PATH during upgrades.

That is technically challenging, as we uninstall previous versions fully during an upgrade, i.e. the PATH variable would be edited by removing the Git entry.

Quite honestly, I am not sure how to address this problem. Ideas?

[the-ress]

How did this end up in the PATH?

I added that manually for reasons unrelated to git. However it contains Git subfolder.
When I run git fetch in Total Commander, it opens the Git folder in Explorer instead of running git.exe if C:\Program Files\Git\cmd is below c:\Program Files (x86)\[...]\TeamFoundation\Team Explorer.

[the-ress]

You could pass a command line parameter to the uninstaller and then read its value in CurUninstallStepChanged.

Or you could take a snapshot of PATH before the uninstaller runs.

[dscho]

You could pass a command line parameter to the uninstaller and then read its value in CurUninstallStepChanged.

Or you could take a snapshot of PATH before the uninstaller runs.

That assumes that the installer will always add the same entry to PATH.

But that is not true: when the user chooses the default options, <Git>\cmd is added. When the user chooses the "all Unix tools" option in an upgrade, the installer wants to add <Git>\mingw64\bin and <Git>\usr\bin instead.

In order to address this feature request correctly, we have to handle this scenario. I don't want to end up with a hack that causes more trouble than it solves.

[mfriedrich74]

This is the old topic of an git upgrade installer, where the git uninstaller needs to be told to keep settings intact. Additionally I have the feeling git installer should not touch the PATH variable if wants to add its path and it's already there. Best regards, Mike

…

On Tue, Dec 18, 2018, 1:48 PM Johannes Schindelin ***@***.***> wrote: You could pass a command line parameter to the uninstaller <https://github.com/git-for-windows/build-extra/blob/b3e79252af5be8d7df15187692439bd97bfb7d9f/installer/install.iss#L1980> and then read its value <https://stackoverflow.com/a/35096481/99237> in CurUninstallStepChanged <https://github.com/git-for-windows/build-extra/blob/b3e79252af5be8d7df15187692439bd97bfb7d9f/installer/install.iss#L2800> . Or you could take a snapshot of PATH before the uninstaller runs. That assumes that the installer will always add the same entry to PATH. But that is not true: when the user chooses the default options, <Git>\cmd is added. When the user chooses the "all Unix tools" option in an upgrade, the installer wants to add <Git>\mingw64\bin and <Git>\usr\bin instead. In order to address this feature request correctly, we *have* to handle this scenario. I don't want to end up with a hack that causes more trouble than it solves. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1990 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHJ-yIVA6Ldkep9KutFEuWNuBplLQbyaks5u6Th8gaJpZM4ZYBn3> .

[dscho]

On Thu, 20 Dec 2018, Mike Friedrich wrote: This is the old topic of an git upgrade installer, where the git uninstaller needs to be told to keep settings intact. Additionally I have the feeling git installer should not touch the PATH variable if wants to add its path and it's already there.

The possible options (in particular *after* learning what InnoSetup allows us to do) would be splendid argument to fight over, maybe even an epic one, or even a fierce flame war. Alas, fierce flame wars, as much fun as they might ever have been, have yet to fix even a single thing.

[dscho]

Or you could take a snapshot of PATH before the uninstaller runs.

@the-ress this sounds like your best option.

Are you still interested in this change? If so, I encourage you to

1. install Git for Windows' SDK
2. sdk cd build-extra
3. edit installer/install.iss in a way related to git-for-windows/build-extra@646b6e2:
   1. store the value of PATH before resetting it during uninstall in, say, EnvPathBeforeUpgrade (which would need to be declared next to UninstallString, of type TArrayOfString)
   2. find the insertion point (if any) before appending (now actually inserting) Git's components into the PATH, probably by introducing a new function like this:

      function FindFirstDifference(a,b:TArrayOfString): Integer;
      var
          i,len:Integer;
      begin
          len:=GetArrayLength(a);
          if (len>GetArrayLength(b)) then
              len:=GetArrayLength(b);
          for i:=0 to len-1 do
              if (a[i]<>b[i]) then begin
                  Result:=i;
                  Exit
              end;
          Result:=GetArrayLength(a);
      end;

      and then using it via something like i:=FindFirstDifference(EnvPath,EnvPathBeforeUpgrade);
   3. If inserting instead of appending, shift the succeeding array elements out of the way after growing the array, i.e. something like this:

      // grow env array
      j:=1;
      if RdbPath[GP_CmdTools].Checked then
          j:=j+2;
      SetArrayLength(EnvPath,GetArrayLength(EnvPath)+j);
      
      // shift succeeding elements if necessary
      if (i<GetArrayLength(EnvPath)-j)) then
          for k:=GetArrayLength(EnvPath)-1 down to (i+j) do
              EnvPath[k]:=EnvPath[k-j];

4. test, by creating a test installer (./installer/release.sh 0-test) and using it
5. once everything works as intended, commit, push and open a Pull Request.

[dscho]

@Abdulmanahtalib is this your way of pointing me to tickets you think can be closed?

[dscho]

@the-ress so... do you want this or not? I think I laid out pretty clearly how you can get this feature, and there is not really a whole lot of effort required from your side to complete this project.

[c33s]

voting for reopen. would still love to see this bug fixed. i is quite annoying that after each update of git my system is broken because of git paths moved to the end of the variable.