Offer to install Posh-Git in the installer #401

dscho · 2021-12-06T00:20:46Z

Posh-Git is a very neat PowerShell module that serves as a PowerShell equivalent to Git's Bash prompt and tab completion.

Let's offer it as an optional component; When selected, it will be installed from the PSGallery for all users and will then also be added to the profile.

This addresses git-for-windows/git#1384

/cc @dahlbyk

dahlbyk

Pulling this over from git-for-windows/git#1384 (comment):

Should we uninstall Posh-Git when uninstalling Git for Windows?

Importing posh-git without a git command available logs a warning. Not fatal, but not ideal; better to be able to uninstall.

Can you tell if Git for Windows was installed with the poshgit component? Ideally we'd leave the module alone if it was installed some other way.

If so, how do I remove it from the users' profile, programmatically?

Relevant code exists as of dahlbyk/posh-git#358; we can clean that up and export Remove-PoshGitFromProfile -AllUsers.

dahlbyk · 2021-12-09T04:32:59Z

installer/install.iss

+        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 -NoLogo -NonInteractive -WindowStyle Hidden -command "'+


Might add -NoProfile, too?

dahlbyk · 2021-12-09T05:33:37Z

installer/install.iss

+            'if ($policy -ne """Trusted""") {' +
+            ' Set-PSRepository -Name PSGallery -InstallationPolicy Trusted '+
+            '} ' +
+            'Install-Module -Repository PSGallery -Scope AllUsers posh-git; ' +


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?

Makes sense.

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

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?

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.

dahlbyk · 2021-12-09T06:07:26Z

installer/install.iss

+            ' Set-PSRepository -Name PSGallery -InstallationPolicy $policy ' +
+            '} ' +
+            'if ($res) {' +
+            ' Add-PoshGitToProfile -AllUsers; ' +


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.)

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.

dahlbyk · 2021-12-09T06:29:57Z

cc @rkeithhill @lzybkr

dscho · 2021-12-09T12:31:49Z

Can you tell if Git for Windows was installed with the poshgit component?

Yes. We simply ask IsComponentInstalled('poshgit') (see the corresponding uninstall step for the auto-updater).

If so, how do I remove it from the users' profile, programmatically?

Relevant code exists as of dahlbyk/posh-git#358; we can clean that up and export Remove-PoshGitFromProfile -AllUsers.

That would be nice! Do you think you can do it? I ask because 1) I am almost a PowerShell illiterate, and 2) I am currently overwhelmed with working on an unwelcome blocker.

dscho · 2021-12-09T12:41:40Z

@dahlbyk I updated the commit (anticipating suppport for Remove-PoshGitFromProfile -AllUsers). Could I trouble you to review https://github.com/git-for-windows/build-extra/compare/489804d4184902ef9949eacc12823bd7f3d565f2..ec4db6d37c257d4e61d1cf877d76c8f9888bd53e?

dscho · 2021-12-09T12:51:20Z

Relevant code exists as of dahlbyk/posh-git#358; we can clean that up and export Remove-PoshGitFromProfile -AllUsers.

That would be nice! Do you think you can do it?

Actually, let me give it a shot (while waiting for the MSYS2 runtime to compile).

This is needed to be able to install/uninstall Posh-Git via Git for Windows' installer/uninstaller. See git-for-windows/build-extra#401 for details. Signed-off-by: Johannes Schindelin <[email protected]>

dahlbyk · 2021-12-09T15:03:27Z

installer/install.iss

+            'Uninstall-Package posh-git -Scope AllUsers -ErrorAction SilentlyContinue; ' +
+            '$res=$?; ' +
+            'if ($res) {' +
+            ' Remove-PoshGitFromProfile -AllUsers; ' +


Since this function will be provided by the module, should we uninstall after it has been removed from profile?

But of course 😁

Posh-Git is a very neat PowerShell module that serves as a PowerShell equivalent to Git's Bash prompt and tab completion. Let's offer it as an optional component; When selected, it will be installed from the PSGallery for all users and will then also be added to the profile. When selected, the uninstaller will also uninstall `posh-git`. This addresses git-for-windows/git#1384 Signed-off-by: Johannes Schindelin <[email protected]>

This is needed to be able to install/uninstall Posh-Git via Git for Windows' installer/uninstaller. See git-for-windows/build-extra#401 for details. Signed-off-by: Johannes Schindelin <[email protected]>

dscho · 2021-12-10T12:51:04Z

Relevant code exists as of dahlbyk/posh-git#358; we can clean that up and export Remove-PoshGitFromProfile -AllUsers.

That would be nice! Do you think you can do it?

Actually, let me give it a shot (while waiting for the MSYS2 runtime to compile).

@dahlbyk here you are: dahlbyk/posh-git#877

BTW I also wanted to verify that it passes the test suite, but it failed. The test suite failed also without my changes, though, so I opened dahlbyk/posh-git#878 which I hope you find the time to look into?

dscho · 2021-12-10T12:53:22Z

@dahlbyk here you are: dahlbyk/posh-git#877

Oh, and of course that PR (as well as a Posh-Git release after merging it) is a blocker for this here PR.

dscho · 2022-01-10T12:28:19Z

Git for Windows v2.35.0-rc0 is due either tonight or tomorrow morning. I had hoped to get Posh-Git into that release, but it is cutting it too close now even if we can get the project to offer Posh-Git via Git for Windows moving forward again.

dahlbyk · 2022-01-10T15:33:07Z

Git for Windows v2.35.0-rc0 is due either tonight or tomorrow morning. I had hoped to get Posh-Git into that release, but it is cutting it too close now even if we can get the project to offer Posh-Git via Git for Windows moving forward again.

Agree. Still ramping up after disconnected entirely over break, but hope to get back into this soon.

dscho · 2022-01-17T15:48:02Z

-rc2 is due Wednesday. We've time to get this integrated until then, or after 2.35.0.

dahlbyk · 2022-01-17T20:15:56Z

-rc2 is due Wednesday. We've time to get this integrated until then, or after 2.35.0.

If you don't see progress on this tonight, let's assume we're waiting for vNext.

dscho · 2022-01-20T13:36:52Z

Even if it is too late for yet another Git for Windows version, let's push this forward so that we do not miss the next train, too. First stop: tell me how I can fix the CI runs (which all fail right now), then take a sweep at the open PRs/issues (most importantly, dahlbyk/posh-git#877 and dahlbyk/posh-git#885).

dscho · 2022-01-25T13:06:25Z

Git for Windows v2.35.0 was released yesterday. @dahlbyk I would like to push for Posh-Git to have a working GitHub workflow, the to accept the two PRs I opened, and then to have a new release so that we can go forward with this here PR.

dahlbyk · 2022-01-25T16:38:07Z

Git for Windows v2.35.0 was released yesterday. @dahlbyk I would like to push for Posh-Git to have a working GitHub workflow, the to accept the two PRs I opened, and then to have a new release so that we can go forward with this here PR.

💯 🔜

dahlbyk

So now that you waited months for Remove-PoshGitFromProfile, I'm more and more convinced we should just leave all profiles alone and just find a way to communicate that the user can Add-PoshGitToProfile from any PowerShell host to auto-import the system-installed module.

Profile scripts don't work without changing Execution Policy; we could have the installer offer to change that for folks (need to give options), but I wouldn't want to do it silently
Installing to CurrentHost means any host except the default (e.g. in VS Code) won't have posh-git, so the user will need to know about Add-PoshGitToProfile to turn it on there
- Or we could use AllHosts, but there may be hosts that don't work well with posh-git and you can't opt out
PowerShell Core uses entirely different $PROFILEs, so we could either install there too or we're back to needing education about Add-PoshGitToProfile

In other words, this could well turn into a whole new page of the installer devoted to configuring the PowerShell environment:

Execution Policy?
Current or All Users?
Windows PowerShell: Default Host, All Hosts, or none?
PowerShell Core (if pwsh.exe): Default Host, All Hosts, or none?

Or we could just figure out somewhere for the installer to link to Learn More. 🤷‍♂️

dahlbyk · 2022-03-31T16:52:52Z

installer/install.iss

+            'if ($policy -ne """Trusted""") {' +
+            ' Set-PSRepository -Name PSGallery -InstallationPolicy Trusted '+
+            '} ' +
+            'Install-Module -Repository PSGallery -Scope AllUsers posh-git; ' +


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.

dahlbyk · 2022-03-31T17:35:38Z

installer/install.iss

+            ' Set-PSRepository -Name PSGallery -InstallationPolicy $policy ' +
+            '} ' +
+            'if ($res) {' +
+            ' Add-PoshGitToProfile -AllUsers; ' +


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.

dscho · 2022-04-29T13:21:28Z

I'm more and more convinced we should just leave all profiles alone and just find a way to communicate that the user can Add-PoshGitToProfile from any PowerShell host to auto-import the system-installed module.

I'm not sure that I buy this argument. The problem I see here is that a user installed Git for Windows, clicked the option to install posh-git, then uninstalls Git for Windows (believing that it would clean up posh-git, too), and is then presented with a broken PowerShell profile (that may or may not result in a totally messed-up prompt).

dahlbyk · 2022-04-29T17:09:44Z

I'm more and more convinced we should just leave all profiles alone and just find a way to communicate that the user can Add-PoshGitToProfile from any PowerShell host to auto-import the system-installed module.

I'm not sure that I buy this argument. The problem I see here is that a user installed Git for Windows, clicked the option to install posh-git, then uninstalls Git for Windows (believing that it would clean up posh-git, too), and is then presented with a broken PowerShell profile (that may or may not result in a totally messed-up prompt).

I'm not sure we can avoid this. Even setting aside PS Core, a different user could install the system posh-git from their user-specific profile and we can't reasonably fix that on their behalf.

The simplest we can make this:

Add to to All Users profile and only remove from All Users, if present
Current Host is probably safer than All Hosts
Try to add/remove for pwsh profile in addition to powershell, if command exists

Conflicting installs and other profiles are on the user to manage.

How easy would it be to make adding to profile an option, either binary or pick All/Current User/Host? Would the uninstall remember what was chosen?

dscho · 2022-07-14T10:05:39Z

The simplest we can make this:

* Add to to All Users profile and only remove from All Users, if present

* Current Host is probably safer than All Hosts

* Try to add/remove for `pwsh` profile in addition to `powershell`, if command exists

Sounds good. I'm a bit too out of my depth to make that happen, though. (In addition, I have too much else on my plate.)

How easy would it be to make adding to profile an option, either binary or pick All/Current User/Host?

That could be a checkbox. Here and here are valuable pages about this. It would look somewhat like this:

Would the uninstall remember what was chosen?

I would assume so. If it did not remember, we could add specific code to remember.

dahlbyk · 2022-07-14T19:11:20Z

How easy would it be to make adding to profile an option, either binary or pick All/Current User/Host?

That could be a checkbox. Here and here are valuable pages about this. It would look somewhat like this:

The exclusive component flag could let us model both All/Current Host and All/Current User. 🤔

☐ Install Posh-Git from PSGallery
- ☐ Install for Current User exclusive
- ☐ Install for All Users exclusive dontinheritcheck
- ☐ Add to Profile? (see Add-PoshGitToProfile) disablenouninstallwarning
  - ☐ Default Host exclusive
  - ☐ All Hosts exclusive dontinheritcheck

dscho · 2022-07-15T12:37:57Z

Sounds good, as long as posh-git delivers the relevant mode of Remove-PoshGitFromProfile.

This is needed to be able to install/uninstall Posh-Git via Git for Windows' installer/uninstaller. See git-for-windows/build-extra#401 for details. Signed-off-by: Johannes Schindelin <[email protected]>

dscho mentioned this pull request Dec 7, 2021

Enhancement: bundle posh-git? git-for-windows/git#1384

Closed

dahlbyk reviewed Dec 9, 2021

View reviewed changes

dscho force-pushed the optionally-install-posh-git branch from 489804d to ec4db6d Compare December 9, 2021 12:40

dscho force-pushed the optionally-install-posh-git branch from ec4db6d to 23fae10 Compare December 9, 2021 13:31

dahlbyk reviewed Dec 9, 2021

View reviewed changes

dscho force-pushed the optionally-install-posh-git branch from 23fae10 to 126917c Compare December 9, 2021 22:43

dscho mentioned this pull request Dec 10, 2021

Add the Remove-PoshGitFromProfile function dahlbyk/posh-git#877

Merged

dahlbyk reviewed Mar 31, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Offer to install Posh-Git in the installer #401

Offer to install Posh-Git in the installer #401

dscho commented Dec 6, 2021

dahlbyk left a comment

dahlbyk Dec 9, 2021

dscho Dec 9, 2021

dahlbyk Dec 9, 2021

dscho Dec 9, 2021

rkeithhill Dec 9, 2021

dscho Dec 10, 2021

dahlbyk Mar 31, 2022

dahlbyk Dec 9, 2021

dahlbyk Mar 31, 2022

dahlbyk commented Dec 9, 2021

dscho commented Dec 9, 2021

dscho commented Dec 9, 2021

dscho commented Dec 9, 2021

dahlbyk Dec 9, 2021

dscho Dec 9, 2021

dscho commented Dec 10, 2021

dscho commented Dec 10, 2021

dscho commented Jan 10, 2022

dahlbyk commented Jan 10, 2022

dscho commented Jan 17, 2022

dahlbyk commented Jan 17, 2022

dscho commented Jan 20, 2022 •

edited

Loading

dscho commented Jan 25, 2022

dahlbyk commented Jan 25, 2022

dahlbyk left a comment

dahlbyk Mar 31, 2022

dahlbyk Mar 31, 2022

dscho commented Apr 29, 2022

dahlbyk commented Apr 29, 2022

dscho commented Jul 14, 2022

dahlbyk commented Jul 14, 2022

dscho commented Jul 15, 2022

Offer to install Posh-Git in the installer #401

Are you sure you want to change the base?

Offer to install Posh-Git in the installer #401

Conversation

dscho commented Dec 6, 2021

dahlbyk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dahlbyk commented Dec 9, 2021

dscho commented Dec 9, 2021

dscho commented Dec 9, 2021

dscho commented Dec 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dscho commented Dec 10, 2021

dscho commented Dec 10, 2021

dscho commented Jan 10, 2022

dahlbyk commented Jan 10, 2022

dscho commented Jan 17, 2022

dahlbyk commented Jan 17, 2022

dscho commented Jan 20, 2022 • edited Loading

dscho commented Jan 25, 2022

dahlbyk commented Jan 25, 2022

dahlbyk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dscho commented Apr 29, 2022

dahlbyk commented Apr 29, 2022

dscho commented Jul 14, 2022

dahlbyk commented Jul 14, 2022

dscho commented Jul 15, 2022

dscho commented Jan 20, 2022 •

edited

Loading