Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Offer to install Posh-Git in the installer #401

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Offer to install Posh-Git in the installer #401

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 59 additions & 2 deletions installer/install.iss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ Name: windowsterminal; Description: "(NEW!) Add a Git Bash Profile to Windows Te
#ifdef WITH_SCALAR
Name: scalar; Description: "(NEW!) Scalar (Git add-on to manage large-scale repositories)"; Types: default
#endif
Name: poshgit; Description: "(NEW!) Install Posh-Git from the PSGallery"


[Run]
Expand Down Expand Up @@ -1096,8 +1097,8 @@ begin
end;
#endif
RegQueryStringValue(HKEY_LOCAL_MACHINE,'Software\GitForWindows','CurrentVersion',PreviousGitForWindowsVersion);
// The Windows Terminal profile is new in v2.32.0
HasUnseenComponents:=IsUpgrade('2.32.0');
// The Posh-Git option is new in v2.35.0
HasUnseenComponents:=IsUpgrade('2.35.0');
if HasUnseenComponents then
AddToSet(CustomPagesWithUnseenOptions,wpSelectComponents);
#if APP_VERSION!='0-test'
Expand Down Expand Up @@ -2940,6 +2941,7 @@ end;
procedure CurStepChanged(CurStep:TSetupStep);
var
DllPath,FileName,Cmd,Msg,Ico:String;
ExitCode:DWORD;
BuiltIns,ImageNames,EnvPath:TArrayOfString;
Count,i:Longint;
RootKey:Integer;
Expand Down Expand Up @@ -3366,6 +3368,40 @@ begin
InstallWindowsTerminalFragment();
end;

{
Install Posh-Git from the PSGallery
}

if (IsComponentSelected('poshgit')) then begin
WizardForm.StatusLabel.Caption:='Installing Posh-Git from the PSGallery';
// Must use the sysnative version of PowerShell, otherwise will target the wrong profile
Cmd:='"'+ExpandConstant('{sysnative}\WindowsPowerShell\v1.0\powershell.exe')+'"'+
' -ExecutionPolicy Bypass -NoProfile -NoLogo -NonInteractive -WindowStyle Hidden -command "'+
'if (!(Get-PackageProvider -Name NuGet)) {'+
' Install-PackageProvider -Name NuGet -Force ' +
'} ' +
'$policy = (Get-PSRepository -Name PSGallery).InstallationPolicy; ' +
'if ($policy -ne """Trusted""") {' +
' Set-PSRepository -Name PSGallery -InstallationPolicy Trusted '+
'} ' +
'Uninstall-Package posh-git -Scope AllUsers -ErrorAction SilentlyContinue; ' +
'Install-Module -Repository PSGallery -Scope AllUsers posh-git; ' +
Copy link

Choose a reason for hiding this comment

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

If posh-git is already installed, this is a no-op.

WARNING: Version '0.7.3' of module 'posh-git' is already installed at 'C:\Program Files\WindowsPowerShell\Modules\posh-git\0.7.3'. To install version '1.0.0', run Install-Module and add the -Force parameter, this command will install version '1.0.0' side-by-side with version '0.7.3'.

-Force leaving old versions behind doesn't seem ideal.

We can detect an existing install with Get-Package posh-git -Scope AllUsers -ErrorAction SilentlyContinue. Would be nice to be able to Update-Module -Scope AllUsers, but it didn't learn -Scope until somewhat recently.

Maybe try to Uninstall-Package posh-git -Scope AllUsers -ErrorAction SilentlyContinue first?

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense.

Copy link

Choose a reason for hiding this comment

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

To test for posh-git already being installed, I think you want Get-Module posh-git -ListAvailable

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we need to test for that, or do we simply just uninstall? I am of two minds there. If a user uninstalled Posh-Git manually, they would probably want to be notified that Git still thinks it should be installed, right?

Copy link

Choose a reason for hiding this comment

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

I think if they ask to install a system posh-git, we clear out any existing system installs.

For the record, we're installing with Windows PowerShell but there are separate AllUsers install locations (with higher precedence) for PowerShell Core which we're leaving alone.

'$res=$?; ' +
'if ($policy -ne """Trusted""") {' +
' Set-PSRepository -Name PSGallery -InstallationPolicy $policy ' +
'} ' +
'if ($res) {' +
' Add-PoshGitToProfile -AllUsers; ' +
Copy link

Choose a reason for hiding this comment

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

This includes a check if posh-git is already imported in any of the current user's profiles, which would prevent the expected system-wide install here. It would seem reasonable for posh-git to be changed to only test the file to which the import would be added.

However, when testing this I also noticed that importing posh-git from two different locations (e.g. from the system install and then from a user install) can lead to unexpected behavior. So maybe better to err on the side of leaving all profiles alone if there's evidence of posh-git?

(No change required, here. Just thinking out loud about how posh-git should behave.)

Copy link

Choose a reason for hiding this comment

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

This includes a check if posh-git is already imported in any of the current user's profiles, which would prevent the expected system-wide install here. It would seem reasonable for posh-git to be changed to only test the file to which the import would be added.

I did not include this change in v1.1.0, but I do think it makes sense for adding to a specific profile to only check for an existing Import-Module in that file. We can't use -Force.

However, when testing this I also noticed that importing posh-git from two different locations (e.g. from the system install and then from a user install) can lead to unexpected behavior. So maybe better to err on the side of leaving all profiles alone if there's evidence of posh-git?

I believe the problem with importing different versions of posh-git is that we're is we're defining multiple versions of class PoshGitCellColor and such. It gets confused when the streams are crossed.

' $res=$? ' +
'} ' +
'if (!$res) {' +
' exit(1) ' +
'}"';
if not ExecWithCapture(Cmd,Msg,Msg,ExitCode) or (ExitCode<>0) then
LogError('Failed to install Posh-Git:'+#13+Msg);
end;


{
Optionally "skip" installing bundled SSH binaries conflicting with external OpenSSH:
}
Expand Down Expand Up @@ -3737,6 +3773,8 @@ var
FileName,PathOption:String;
EnvPath:TArrayOfString;
i:Longint;
Cmd,Msg:String;
ExitCode:DWORD;
begin
if CurUninstallStep<>usUninstall then begin
Exit;
Expand Down Expand Up @@ -3812,4 +3850,23 @@ begin
if not DeleteFile(FileName) then
LogError('Line {#__LINE__}: Unable to delete file "'+FileName+'". Please do it manually after logging off and on again.');
end;

{
Remove posh-git
}

if (IsComponentSelected('poshgit')) then begin
Cmd:='"'+ExpandConstant('{sysnative}\WindowsPowerShell\v1.0\powershell.exe')+'"'+
' -ExecutionPolicy Bypass -NoProfile -NoLogo -NonInteractive -WindowStyle Hidden -command "'+
'Remove-PoshGitFromProfile -AllUsers;' +
'$res=$?; ' +
'if (!(Uninstall-Package posh-git -Scope AllUsers -ErrorAction SilentlyContinue)) {' +
' $res=false; ' +
'} ' +
'if (!$res) {' +
' exit(1) ' +
'}"';
if not ExecWithCapture(Cmd,Msg,Msg,ExitCode) or (ExitCode<>0) then
LogError('Failed to uninstall Posh-Git:'+#13+Msg);
end;
end;