(#310) Uninstall-ChocolateyPath #1019

DarwinJS · 2016-10-15T11:53:25Z

I would like feedback on whether to incorporate any of these features:

DONE: Awarn that the path also exists in the opposite scope (User or Machine) - but don't process the removal from the non-requested scope
DONE: allow a special scope option 'All' which removes the path from both scopes (User and Machine) in one pass

Closes #310

ferventcoder · 2016-10-15T15:37:50Z

src/chocolatey.resources/helpers/functions/Uninstall-ChocolateyPath.ps1

@@ -0,0 +1,117 @@
+# Copyright 2011 - Present RealDimensions Software, LLC & original authors/contributors from https://github.com/chocolatey/chocolatey


Not that you would have known this, but this won't carry the second part of the copyright as this didn't come from the old code base.

ferventcoder · 2016-10-15T15:38:50Z

src/chocolatey.resources/helpers/functions/Uninstall-ChocolateyPath.ps1

+  Write-Output "Only evaluating and updating path scope `"$pathType`", path will not be assessed nor removed for other scope, so path may exist in other scope as well."
+  $originalPathToUninstall = $PathToUninstall
+  #First half on handling trailing slash properly - remove it from requested path:
+  $PathToUninstall = $PathToUninstall.trimend('\')


What is trimend and is it available in PowerShell v2+ (and thus .NET Framework 2.0+)?

ferventcoder · 2016-10-15T15:56:56Z

src/chocolatey.resources/helpers/functions/Uninstall-ChocolateyPath.ps1

+  [parameter(ValueFromRemainingArguments = $true)][Object[]] $ignoredArguments
+)
+  Write-Debug "Running 'Uninstall-ChocolateyPath' with PathToUninstall:`'$PathToUninstall`'";
+  Write-Output "Only evaluating and updating path scope `"$pathType`", path will not be assessed nor removed for other scope, so path may exist in other scope as well."


I know it is widely believed that Write-Host is wrong, and it is under most circumstances. When it comes to logging, we prefer Write-Host in the Chocolatey installer module. It doesn't cause issues when you want to return actual things (Write-Output will return things to the pipeline). So please convert these to Write-Host.

Yes, I am familiar with http://www.jsnover.com/blog/2013/12/07/write-host-considered-harmful/:

The blog post appears to be about scripts (or modules) that could be used by other tools. This module is used for Chocolatey, so it is the end consumer of its own module.

Write-Output ends up in the pipeline (aka is part of the return value from the function). This can have (and has had) disastrous results, so we prefer Write-Host. I prefer real life examples to rhetoric, so 44306ef

We control the hosts this runs under.

We simply want logging messages.

Consider Write-Output == Log.Info() and Write-Host == Log.Info() for the Chocolatey framework.
Yes, I realize this means we should have probably set up a logging utility. We could go on all day long about how we should have brought in an actual logging facility like log4net, but honestly all of that work to provide what Write-Host already provides seems non-beneficial.

ferventcoder · 2016-10-15T16:30:48Z

allow a special scope option 'All' which removes the path from both scopes (User and Machine) in one pass

I like it!

ferventcoder · 2016-10-15T16:31:19Z

warn that the path also exists in the opposite scope (User or Machine) - but don't process the removal from the non-requested scope

Also like it. And you could also provide that there is an "ALL" option as part of the warning.

DarwinJS · 2016-10-17T12:55:54Z

OK - the combination of: [1] Always logging which scopes the path appears in (even if not the one being changed) and [2] Supporting the recursion of the function to switch to admin rights if possible and [3] allow use of -pathToUninstall 'All' - resulted in a very different code structure.

ferventcoder · 2016-10-17T17:20:18Z

src/chocolatey.resources/helpers/functions/Uninstall-ChocolateyPath.ps1

+      }
+      Else
+      {
+        Throw "Something is wrong as I did not get admin rights on the recursive call and was about to go into a recursive loop.  (Recursive loops make the Chocolatey gods dizzy.)"


Leave Chocolatey gods references out. It sends the wrong message to some folks. And I should remove the one that is in there about pushing to non-secure urls from Chocolatey. I know it is fun, but some folks take offense to this.

Also, no messages in the first person.

ferventcoder · 2016-10-17T17:20:56Z

src/chocolatey.resources/helpers/functions/Uninstall-ChocolateyPath.ps1

+`-PathType 'Machine'`.
+
+This is used when the application/tool is not being linked by Chocolatey
+(not in the lib folder).


I will also need to update this to indicate first supported version of Chocolatey this is in.

Modified to Check for -pathToUninstall in all scopes and to support scope option 'All' Handles multiple occurances of path, promiscuous handling of trailing slash (just does the right thing whether or not pathToUninstall or embedded path do or do not have trailing slash)

DarwinJS · 2016-10-18T04:35:29Z

Ok - choco gods reference removed and corrected error in first person.

I am having a hard time discovering whether .trimend is available in PSH 2 via research.

I have temporarily lost access to my Windows 7 VMs because they were in Hyper-V and after Windows 10 Anniversary update I cannot have both Virtual Box AND Hyper-V INSTALLED at the same time. Before Anniversary Update both could be installed, but not active at once. I don't have any VMs with PSH 2 on Virtual Box yet and it would be a bit of fusing to reinstall hyper-V and then deinstall when done.

DarwinJS · 2016-10-18T04:56:33Z

I was able to test to be sure that ".trimend" is available as a string method on PSH 2.0

jberezanski · 2016-10-19T19:42:10Z

FWIW, System.String.TrimEnd(char[]) is available at least since .NET 1.1 (that's as far back as MSDN docs go).

DarwinJS · 2016-10-19T20:39:42Z

@jberezanski - thanks for the research on that!

dragon788 · 2016-10-31T22:51:12Z

Looking forward to this, was just taking a whack at implementing this within an individual package similar to one on the linked issues that DarwinJS had mentioned. Nothing drives me nuts more than clutter in the PATH that leads to aberrant behavior.

dragon788 · 2016-11-01T18:07:16Z

Since I needed to remove multiple entries from the PATH and I didn't want to loop over the list multiple times, I improved a little bit on your workaround on ec2clitools.

@DarwinJS perhaps a similar "optimization" could be useful here? Since multiple foreach loops is rough, I opted to use the regex array comparison outlined http://masteringposh.com/powershell-script-multiple-array-comparisons-using-regular-expressions/ and ended up with the code below.

[regex] $PathsToRemove = "^(%NVM_HOME%|%NVM_SYMLINK%)"
$environmentPath = [Environment]::GetEnvironmentVariable("PATH","Machine")
[string[]]$newpath = ''
foreach ($path in $environmentPath.split(';'))
{
  If (($path) -and ($path -notmatch $PathsToRemove))
    {
        [string[]]$newpath += "$path"
        "$path added to `$newpath"
    } else {
        "Bad path found: $path"
    }
}
$AssembledNewPath = ($newpath -join(';')).trimend(';')

[Environment]::SetEnvironmentVariable("PATH",$AssembledNewPath,"Machine")

DarwinJS · 2016-11-01T18:23:31Z

@dragon788 -

I believe multiple path removal would be an edge case.
Not sure what would be rough about it as it's likely milliseconds of time among a much longer wait - you could just call the function twice.
the above does not seem to handle the trailing slash?

If multiple paths is deemed as a requirement, I would probably just make the function capable of receiving pipelined input then you could do this:
@('%NVM_HOME%','%NVM_SYMLINK%') | Uninstall-ChocolateyPath

Here is a brief article on why taking the pipeline approach can be very powerful: http://csi-windows.com/blog/all/74-powershell/438-paradigm-shift-the-power-of-functions-that-take-pipelined-input

I definitely wouldn't want have end consumers of the function have to remember that they are using a regex when it seems they won't need that most of the time.

@ferventcoder - what do you think about multiple path support with a pipeline capable function?

dragon788 · 2016-11-01T19:38:52Z

That's pretty awesome, and I definitely agree that multiple values in the path are unlikely. Hopefully most packages are following the single responsibility principle and keeping things grouped together, I just tend to hit corner cases (Nvm for Windows that manages node versions via symlinks in Program Files but also needs awareness of itself and it's location).

DarwinJS · 2016-11-01T20:00:44Z

Another really cool thing is that the user of the function never needs to even know that the function can take pipelined input when using it with a single value. So the predominant use case is natural and the advanced use case requires a little more discovery if you're willing to do it.

ferventcoder · 2016-11-01T20:03:24Z

multiple path support with a pipeline capable function?

Could be interesting. For first pass implementation, it doesn't cost much to loop over path information multiple times. So perhaps a new enhancement issue for this one?

jnm2 · 2018-01-27T03:04:36Z

src/chocolatey.resources/helpers/functions/Uninstall-ChocolateyPath.ps1

+  $PathToUninstall = $PathToUninstall.trimend('\')
+  #array drops blanks (one of which is always created by final semi-colon)
+  $actualPathArrayUser = (Get-EnvironmentVariable -Name 'Path' -Scope 'user' -PreserveVariables).split(';',[System.StringSplitOptions]::RemoveEmptyEntries)
+  $actualPathArrayMachine = (Get-EnvironmentVariable -Name 'Path' -Scope 'machine' -PreserveVariables).split(';',[System.StringSplitOptions]::RemoveEmptyEntries)


We're going to have to do something a little smarter since apparently semicolons can be escaped using quotes. Too bad there isn't a Windows API for this.

Reassembling the path will have to follow the same procedure in reverse.

Confirmed using Windows' environment variable list editor: a tl;dr entry causes ;"tl;dr"; to be added to the path. (Luckily a path can't contain ".)

Also, the path needs to be recognized and removed even if it is quoted in the PATH variable. I assume the input path would never be quoted.

So luckily PATH doesn't have " and it doesn't have escaped ; - if you make it have ", it doesn't like you anymore IIRC.

Am I understanding that a semi-colon is allowed in a folder name???!!!

I just died a little inside. 👎

jnm2 · 2018-01-27T04:07:35Z

@DarwinJS @ferventcoder I would really like to see this in Chocolatey. Can I do anything to help? I might start on a proof of concept for both Install- and Uninstall-ChocolateyPath which properly understands quoted paths when looking for existing paths or inserting a new path just to unblock my own package. Would be happier getting them in Chocolatey sooner though!

DarwinJS · 2018-01-27T13:10:40Z

@jnm2 - feel free to take this over. Handling quotes seems like a very rare case - maybe consider getting this merged and then do that as a new PR?

jnm2 · 2018-02-08T22:05:51Z

@DarwinJS Thank you.

This was the direction I was hoping we might go towards. It tries to make the uninstall work as symmetrically as possible with the install and leave a minimal footprint.

Can we do this? What would the next step be? /cc @ferventcoder

jnm2 · 2018-02-09T00:13:23Z

Here's the branch with the Un/install-ChocolateyPath pair which respect semicolons: master...jnm2:env_path_uninstall
I put in comments explaining why it is done that way. What do you guys think of this approach?

jnm2 · 2018-06-29T23:54:14Z

Hey @gep13, Rob said I should tag you for the two comments above. I'd like to help out and I have a proof of concept that deals with all the edge cases I know of, but I'm not sure if I should start a new PR?

FranklinYu · 2018-10-17T01:58:21Z

Any news? Shall we start reviewing @jnm2’s branch?

jnm2 · 2018-10-17T19:42:46Z

@FranklinYu I haven't heard anything from anyone about this, so I went ahead: #1663

CLAassistant · 2020-06-08T14:50:15Z

All committers have signed the CLA.

ferventcoder · 2020-06-08T17:20:34Z

This may be superseded with @jnm2 's changes.

vexx32 · 2024-04-19T14:52:10Z

Apologies for the long wait on this PR. This is being added as part of a larger scope of work involving some rewrites that will be submitted soon, so for now we'll close this PR.

Thank you very much for the contribution! 💜

ferventcoder reviewed Oct 15, 2016

View reviewed changes

ferventcoder changed the title ~~Issue310~~ (GH-310) Uninstall-ChocolateyPath Oct 15, 2016

ferventcoder reviewed Oct 17, 2016

View reviewed changes

DarwinJS mentioned this pull request Oct 19, 2016

Add Uninstall-ChocolateyPath function to manage removing entries from PATH #310

Closed

DarwinJS mentioned this pull request Apr 22, 2017

(#1015) Path exists check retains path scope specified by user, adds semi-colon to end of path #1017

Closed

jnm2 suggested changes Jan 27, 2018

View reviewed changes

jnm2 mentioned this pull request Oct 17, 2018

(#310) Adds Uninstall-ChocolateyPath and now respects semicolons in paths #1663

Closed

TheCakeIsNaOH mentioned this pull request Sep 10, 2020

Feature - Add Uninstall-ChocolateyPath funtion #2115

Closed

gep13 changed the title ~~(GH-310) Uninstall-ChocolateyPath~~ (#310) Uninstall-ChocolateyPath Oct 15, 2021

vexx32 closed this Apr 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(#310) Uninstall-ChocolateyPath #1019

(#310) Uninstall-ChocolateyPath #1019

DarwinJS commented Oct 15, 2016 •

edited

Loading

ferventcoder Oct 15, 2016

ferventcoder Oct 15, 2016

ferventcoder Oct 15, 2016 •

edited

Loading

ferventcoder commented Oct 15, 2016

ferventcoder commented Oct 15, 2016

DarwinJS commented Oct 17, 2016 •

edited

Loading

ferventcoder Oct 17, 2016

ferventcoder Oct 17, 2016

DarwinJS commented Oct 18, 2016

DarwinJS commented Oct 18, 2016

jberezanski commented Oct 19, 2016

DarwinJS commented Oct 19, 2016

dragon788 commented Oct 31, 2016

dragon788 commented Nov 1, 2016

DarwinJS commented Nov 1, 2016

dragon788 commented Nov 1, 2016

DarwinJS commented Nov 1, 2016

ferventcoder commented Nov 1, 2016

jnm2 Jan 27, 2018

jnm2 Jan 27, 2018 •

edited

Loading

jnm2 Jan 27, 2018

ferventcoder Jan 27, 2018

ferventcoder Jan 27, 2018

ferventcoder Jan 27, 2018

jnm2 commented Jan 27, 2018

DarwinJS commented Jan 27, 2018

jnm2 commented Feb 8, 2018

jnm2 commented Feb 9, 2018

jnm2 commented Jun 29, 2018

FranklinYu commented Oct 17, 2018

jnm2 commented Oct 17, 2018

CLAassistant commented Jun 8, 2020 •

edited

Loading

ferventcoder commented Jun 8, 2020

vexx32 commented Apr 19, 2024

		@@ -0,0 +1,117 @@
		# Copyright 2011 - Present RealDimensions Software, LLC & original authors/contributors from https://github.com/chocolatey/chocolatey

(#310) Uninstall-ChocolateyPath #1019

(#310) Uninstall-ChocolateyPath #1019

Conversation

DarwinJS commented Oct 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferventcoder Oct 15, 2016 • edited Loading

Choose a reason for hiding this comment

ferventcoder commented Oct 15, 2016

ferventcoder commented Oct 15, 2016

DarwinJS commented Oct 17, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarwinJS commented Oct 18, 2016

DarwinJS commented Oct 18, 2016

jberezanski commented Oct 19, 2016

DarwinJS commented Oct 19, 2016

dragon788 commented Oct 31, 2016

dragon788 commented Nov 1, 2016

DarwinJS commented Nov 1, 2016

dragon788 commented Nov 1, 2016

DarwinJS commented Nov 1, 2016

ferventcoder commented Nov 1, 2016

Choose a reason for hiding this comment

jnm2 Jan 27, 2018 • edited Loading

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

jnm2 commented Jan 27, 2018

DarwinJS commented Jan 27, 2018

jnm2 commented Feb 8, 2018

jnm2 commented Feb 9, 2018

jnm2 commented Jun 29, 2018

FranklinYu commented Oct 17, 2018

jnm2 commented Oct 17, 2018

CLAassistant commented Jun 8, 2020 • edited Loading

ferventcoder commented Jun 8, 2020

vexx32 commented Apr 19, 2024

DarwinJS commented Oct 15, 2016 •

edited

Loading

ferventcoder Oct 15, 2016 •

edited

Loading

DarwinJS commented Oct 17, 2016 •

edited

Loading

jnm2 Jan 27, 2018 •

edited

Loading

CLAassistant commented Jun 8, 2020 •

edited

Loading