Chocolatey Simplification + Uninstall #358

dahlbyk · 2017-01-06T06:40:13Z

Closes #239, inspired by need for #357 hack to fix #355.

Specific changes:

Chocolatey installer no longer adds code to $PROFILE that saves the pre–posh-git prompt
Chocolatey installer also strips that legacy code from $PROFILE (if you want to keep the old behavior, make sure PoshGitPrompt doesn't appear in those lines)
Added uninstaller that both deletes %ChocolateyBinRoot%\poshgit and removes lines we might have added to $PROFILE

We might consider reverting the second commit (and maybe the first) for now, to avoid a breaking change before 1.0 drops.

dahlbyk · 2017-01-06T06:43:35Z

/cc @mwrock as the orignial author of the prompt override logic (534d602).

mwrock

LGTM. I agree with the simplified approach. This strategy was originally written to preserve a posh-hg prompt. I had repos for both source control providers and I wanted each to preserve the other. I think tha tis VERY much an edge case today and I personally don't use mercurial anymore but it still holds a soft place in my heart.

rkeithhill

Looks good but look over the file encoding comments to see what you think.

rkeithhill · 2017-01-06T17:09:13Z

chocolatey/tools/chocolateyUninstall.ps1

+
+            $newProfile += $line
+        }
+        Set-Content -path $profile -value $newProfile -Force


Set-Content defaults to ASCII. Either we should detect the current encoding like in the other install.ps1 or maybe use -Encoding Utf8? But that will add the UTF-8 BOM. File encodings are such a PITA.

Also, should we backup the profile before we actually modify it - just in case?

Oh man yeah that BOM issue is a total pain. 👍 to backing up the old $PROFILE

Conveniently enough, install.ps1 already knows how to detect encoding. Reasonable to tackle that as a separate PR since this case is just copy/paste of chocolateyInstall.ps1?

rkeithhill · 2017-01-06T17:11:42Z

chocolatey/tools/chocolateyInstall.ps1

        }
-        # Save any previous Prompt logic
-        Insert-Script ([REF]$newProfile) $oldPromptOverride
        Set-Content -path $profile -value $newProfile -Force


See comment below about file encoding and creating a backup.

dahlbyk · 2017-01-16T07:28:39Z

I've brought this up-to-date after #361 and am working toward using Add-PoshGitToProfile and (new, encoding-aware) Remove-PoshGitFromProfile for all activities that touch the profile.

dahlbyk · 2017-01-16T16:56:47Z

Yup, that's why I forced the result to be an array @() and then checked the count (length) of the array. Casting to [bool] like that works but isn't particularly "powershelly". And we typically don't use a return when it isn't needed. :-)

Yeah, I should have just gone to bed. I was trying to troubleshoot a false positive and just couldn't figure it out. Turns out -match returns a boolean when matching a string and acts as a filter when used against an array/list.

PS> 'posh-git' -match 'posh-git'
True
PS> @('posh-git') -match 'posh-git'
posh-git

In retrospect, it's obvious: the false positive was from a one-line $PROFILE.CurrentUserAllHosts I just happened to have created. So the fix is to force to array before the -match: c8bd3b5. Added a test for this scenario, too.

rkeithhill · 2017-01-16T17:19:48Z

I should have just gone to bed

Heh, yeah that happens with me far too often. Sometimes when I look at my "late night code" the next day or two - wow! My wife keeps telling me - "you think you're twenty something but ... you're not!" :-)

dahlbyk · 2017-01-26T08:28:57Z

This is blocked until #376, which has absorbed this PR's non-Chocolatey changes, has been merged.

Pretty much everything has changed, so let's start over.

dahlbyk · 2017-01-27T18:29:52Z

This is blocked until #376, which has absorbed this PR's non-Chocolatey changes, has been merged.

Rebased on master and basically rewrote the entire branch with some refactoring, encoding awareness, etc. Haven't actually tested the Chocolatey package yet, but could use some feedback on the evolution.

rkeithhill

LGTM - just one question about determining "current" (or latest?) version.

rkeithhill · 2017-01-27T19:11:13Z

chocolatey/tools/chocolateyInstall.ps1

    Install-ChocolateyZipPackage 'poshgit' $poshGitInstall $poshgitPath
-    $pgitDir = Dir "$poshgitPath\*posh-git*\" | Sort-Object -Property LastWriteTime | Select -Last 1
+    $currentVersionPath = Get-ChildItem "$poshgitPath\*posh-git*\" | Sort-Object -Property LastWriteTime | Select-Object -Last 1


I wonder if it would be better to sort on the directory name if it includes a lexicographic sortable version number?

This is really just defensive programming. A few lines up we try to remove-item $poshgitPath, so this folder will almost always have a single subfolder.

dahlbyk · 2017-01-30T04:20:22Z

Finally getting around to testing out the updated packaging.

Clean install
Upgrade
Uninstall

WARNING: do not take a dependency on anything here, nor the v0.7.0-pre1 tag. I'm hacking and slashing all over the place.

rkeithhill · 2017-01-30T07:39:44Z

You've been busy. ;-)

dahlbyk · 2017-01-30T23:25:09Z

Added another "fix" for the latest error: #381 (comment)

It looks like Chocolatey 0.10.* might have changed availability of $PROFILE?

rkeithhill · 2017-01-30T23:35:41Z

Hmm, is Chocolatey implementing their own PowerShell host process (and not initializing $PROFILE)? Hard to believe Chocolatey would just whack the $PROFILE variable but unfortunately that variable is not readonly, so it is possible.

rkeithhill · 2017-01-30T23:38:26Z

src/Utils.ps1

+        Write-Verbose "`$profilePath          = '$profilePath'"
+        Write-Verbose "`$PROFILE              = '$PROFILE'"
+        Write-Verbose "CurrentUserCurrentHost = '${PROFILE.CurrentUserCurrentHost}'"
+        Write-Verbose "CurrentUserAllHosts    = '${PROFILE.CurrentUserAllHosts}'"


Here you want "... '$($Profile.CurrentUserAllHosts)'" ${} is handy when you want to separate the variable from the next text e.g. "${foo}${bar}".

Ditto for the other two below.

Good catch, not sure what I was thinking.

dahlbyk · 2017-01-31T03:11:10Z

Hmm, is Chocolatey implementing their own PowerShell host process (and not initializing $PROFILE)? Hard to believe Chocolatey would just whack the $PROFILE variable but unfortunately that variable is not readonly, so it is possible.

At this point I just want it to work. I may bug @ferventcoder later to figure out why $PROFILE is missing its properties.

Check out 4c8cbc6 for my workarounds. Everything finally seems to be working as expected with Chocolatey v0.10.3.

choco install poshgit --version=v0.7.0-pre4

rkeithhill · 2017-01-31T03:14:40Z

I hear ya on the "just want it to work" sentiment. :-) Otherwise, LGTM.

ferventcoder · 2017-01-31T22:08:03Z

$PROFILE is missing its properties.

Chocolatey indeed hosting our own PowerShell Host, since 0.9.10. I thought we implemented a way for $PROFILE to work similarly to how it did before, but it may not have much more than the one profile path as its settings. My understanding is that a profile path is dependent on the PowerShell Host and not guaranteed. At least from what I've read. Open to suggestions to improve the custom Choco Host though.

rkeithhill · 2017-01-31T22:39:02Z

@ferventcoder thanks for confirming. It makes more sense for us to handle this in the chocolatey install scripts. Any CurrentHost Profile scripts provided by Chocolatey would only apply when the install package is running in Chocolatey and not later when the current host is PowerShell console or ISE.

dahlbyk · 2017-02-01T09:34:32Z

$PROFILE is missing its properties.

Chocolatey indeed hosting our own PowerShell Host, since 0.9.10. I thought we implemented a way for $PROFILE to work similarly to how it did before, but it may not have much more than the one profile path as its settings. My understanding is that a profile path is dependent on the PowerShell Host and not guaranteed. At least from what I've read. Open to suggestions to improve the custom Choco Host though.

I think we're good now, but it wouldn't hurt for Chocolatey to learn about the extended $PROFILE properties: chocolatey/choco#1154.

ferventcoder · 2017-02-05T18:27:00Z

Not that you need it, but I just wanted to throw it out there that there is a chocolateyBeforeModify.ps1 (as of 0.9.10) that you can use before upgrade and before uninstall to ensure things are not running, etc.

dahlbyk mentioned this pull request Jan 6, 2017

Error in $profile after poshgit installed? #355

Closed

dahlbyk added this to the v0.7 milestone Jan 6, 2017

dahlbyk requested a review from rkeithhill January 6, 2017 06:42

mwrock approved these changes Jan 6, 2017

View reviewed changes

dahlbyk mentioned this pull request Jan 6, 2017

Chocolatey Deprecating A Function #226

Closed

rkeithhill previously approved these changes Jan 6, 2017

View reviewed changes

rkeithhill mentioned this pull request Jan 6, 2017

"Error An expression was expected after '('or on" whenever I open a powershell prompt #180

Closed

dahlbyk force-pushed the chocolatey-uninstall branch from b2b6c0e to f2a29a2 Compare January 16, 2017 07:32

dahlbyk mentioned this pull request Jan 26, 2017

Simplify install.ps1 and move core module files under src/ #376

Merged

rkeithhill mentioned this pull request Jan 27, 2017

Next Release Date? #353

Closed

dahlbyk force-pushed the chocolatey-uninstall branch 3 times, most recently from d902f42 to 1f94d29 Compare January 27, 2017 18:25

rkeithhill approved these changes Jan 27, 2017

View reviewed changes

dahlbyk force-pushed the chocolatey-uninstall branch from 1197f56 to 08c9613 Compare January 30, 2017 03:47

dahlbyk mentioned this pull request Jan 30, 2017

Can't install latest release #381

Closed

dahlbyk added 3 commits January 30, 2017 09:21

Remove Chocolatey prompt override

297e07a

Strip Chocolatey prompt override on install

48f61c9

Trivial cleanup

35c57a9

dahlbyk force-pushed the chocolatey-uninstall branch from fc68001 to 38fe768 Compare January 30, 2017 15:25

Refactor/simplify check for existing profile import

c1c9748

Add $PROFILE to verbose output

6bdac52

rkeithhill reviewed Jan 30, 2017

View reviewed changes

dahlbyk added 3 commits January 30, 2017 19:29

Fix poorly formed interpolation

77aaa9c

Bump to 0.7.0-pre4

5c8e048

Specify local version to install

9a1d91d

dahlbyk force-pushed the chocolatey-uninstall branch from c7d47c9 to 83ce339 Compare January 31, 2017 01:52

Fail if the version tag doesn't exist

8161c8f

dahlbyk force-pushed the chocolatey-uninstall branch from 83ce339 to 8161c8f Compare January 31, 2017 01:54

dahlbyk added 4 commits January 30, 2017 20:29

Confirm remote tag, with -Force option to create

c9c1b52

Add handling for non-standard $PROFILE

4c8cbc6

Force should always update remote tag

7620aaf

Create local tag, too

04c2415

dahlbyk mentioned this pull request Feb 1, 2017

Add current/all users/hosts properties to custom host's $PROFILE chocolatey/choco#1154

Open

This was referenced Feb 1, 2017

Handle Chocolatey Host $PROFILE #390

Merged

Roadmap to 1.0 #328

Open

dahlbyk added 2 commits February 3, 2017 08:57

Merge branch 'master' into chocolatey-uninstall

f8d9639

Bump to 0.7.0-pre5

8a4207d

dahlbyk merged commit 7b847ee into master Feb 6, 2017

dahlbyk deleted the chocolatey-uninstall branch February 6, 2017 16:59

dahlbyk mentioned this pull request Feb 6, 2017

Help! Chocolatey Upgrade Issues #272

Closed

dahlbyk mentioned this pull request May 14, 2018

Avoiding creating a C:\tools directory #473

Open

dscho mentioned this pull request Dec 9, 2021

Offer to install Posh-Git in the installer git-for-windows/build-extra#401

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chocolatey Simplification + Uninstall #358

Chocolatey Simplification + Uninstall #358

dahlbyk commented Jan 6, 2017

dahlbyk commented Jan 6, 2017

mwrock left a comment

rkeithhill left a comment

rkeithhill Jan 6, 2017

rkeithhill Jan 6, 2017

mwrock Jan 6, 2017

dahlbyk Jan 6, 2017

rkeithhill Jan 6, 2017

dahlbyk commented Jan 16, 2017 •

edited

Loading

dahlbyk commented Jan 16, 2017 •

edited

Loading

rkeithhill commented Jan 16, 2017

dahlbyk commented Jan 26, 2017

dahlbyk commented Jan 27, 2017

rkeithhill left a comment

rkeithhill Jan 27, 2017

dahlbyk Jan 27, 2017

dahlbyk commented Jan 30, 2017 •

edited

Loading

rkeithhill commented Jan 30, 2017

dahlbyk commented Jan 30, 2017

rkeithhill commented Jan 30, 2017

rkeithhill Jan 30, 2017

rkeithhill Jan 30, 2017

dahlbyk Jan 31, 2017

dahlbyk commented Jan 31, 2017 •

edited

Loading

rkeithhill commented Jan 31, 2017

ferventcoder commented Jan 31, 2017

rkeithhill commented Jan 31, 2017

dahlbyk commented Feb 1, 2017

ferventcoder commented Feb 5, 2017

Chocolatey Simplification + Uninstall #358

Chocolatey Simplification + Uninstall #358

Conversation

dahlbyk commented Jan 6, 2017

dahlbyk commented Jan 6, 2017

mwrock left a comment

Choose a reason for hiding this comment

rkeithhill 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

dahlbyk commented Jan 16, 2017 • edited Loading

dahlbyk commented Jan 16, 2017 • edited Loading

rkeithhill commented Jan 16, 2017

dahlbyk commented Jan 26, 2017

dahlbyk commented Jan 27, 2017

rkeithhill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dahlbyk commented Jan 30, 2017 • edited Loading

rkeithhill commented Jan 30, 2017

dahlbyk commented Jan 30, 2017

rkeithhill commented Jan 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dahlbyk commented Jan 31, 2017 • edited Loading

rkeithhill commented Jan 31, 2017

ferventcoder commented Jan 31, 2017

rkeithhill commented Jan 31, 2017

dahlbyk commented Feb 1, 2017

ferventcoder commented Feb 5, 2017

dahlbyk commented Jan 16, 2017 •

edited

Loading

dahlbyk commented Jan 16, 2017 •

edited

Loading

dahlbyk commented Jan 30, 2017 •

edited

Loading

dahlbyk commented Jan 31, 2017 •

edited

Loading