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.

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

Please persist package installation parameters for uninstall scripts #1479

Open
jnm2 opened this issue Jan 27, 2018 · 22 comments · May be fixed by #2648
Open

Please persist package installation parameters for uninstall scripts #1479

jnm2 opened this issue Jan 27, 2018 · 22 comments · May be fixed by #2648
Assignees
Milestone

Comments

@jnm2
Copy link

jnm2 commented Jan 27, 2018

Imagine you're not relying on an MSI; you're just bin-deploying to a default (overridable) location and adding an entry to %path% for the machine (overridable to 'user' or 'none').

$ErrorActionPreference = 'Stop'

$pp = Get-PackageParameters

$pathType =
    if ('None', 'User', 'Machine' -contains $pp.PathType) { $pp.PathType }
    elseif (!$pp.PathType) { 'Machine' }
    else { throw "Unrecognized PathType '$($pp.PathType)'" }

$installDir = if ($pp.InstallDir) { $pp.InstallDir } else { "$Env:ProgramFiles\Elevate" }
$toolsPath = Split-Path -Parent $MyInvocation.MyCommand.Definition
$platform = if (Get-OSArchitectureWidth 64) { 'x64' } else { 'x86' }

New-Item $installDir -ItemType Directory -Force | Out-Null

Get-ChildItem "$toolsPath/$platform" |
    Move-Item -Destination $installDir -Force

Remove-Item "$toolsPath/*" -Recurse -Exclude *.ps1

if ($pathType -ne 'None') { Install-ChocolateyPath $installDir -PathType $pathType }

(Even if this utility worked when shimmed, there's still the question of the configurable installation location.)

When the user types choco uninstall thepackage, how should chocolateyUninstall.ps1 find the correct installation folder to delete and the correct path variable (machine/user/none) to modify?

I would have expected Get-PackageParameters to be persisted by Chocolatey and provided to the uninstall script. It makes sense to allow them to be individually overridden by passing --params to choco uninstall, but it doesn't make sense for them to start null and thus require the user to duplicate the exact parameters used previously if the user wants a successful uninstall. That's not very user-friendly.

Also, I don't want to have to author an MSI just to have a decently reliable package uninstaller. (I notice that the only other package I can find that allows a user/machine switch only uninstalls the machine path.)

Is there a reason we can't make this just start working out of the box? I'd expect this to apply to uninstalls for sure; probably upgrades as well. Seems like the obvious place to store the --params string is the same place that keeps a record that the package is installed.

@ferventcoder
Copy link
Member

@jnm2 remembered arguments are only provided to upgrades. So we'd need somewhere to store those package parameters for a package outside of that - then we'd need to make sure it is encrypted as it could contain passwords.

@ferventcoder
Copy link
Member

That's not very user-friendly

Agreed, but it's probably not user friendly to install something to a location outside the packaging folders if you don't have a way to tracking how to uninstall it.

Typically folks use Get-ChocolateyToolsLocation if they need to be outside of the packaging folders for some reason and use that location (which a user can customize, but it is set to $env:SystemDrive\tools by default. Then on uninstall they simply use that.

https://github.com/chocolatey/choco/issues/1303#issuecomment-303804561 goes over packaging patterns for zip installations. That will end up in docs.

@jnm2
Copy link
Author

jnm2 commented Jan 27, 2018

Typically folks use Get-ChocolateyToolsLocation if they need to be outside of the packaging folders for some reason and use that location (which a user can customize, but it is set to $env:SystemDrive\tools by default. Then on uninstall they simply use that.

Since I want to follow the standard of adding the installation directory to the machine path by default rather than the user path, there's no guarantee that Get-ChocolateyToolsLocation is accessible to every user on the machine.

Agreed, but it's probably not user friendly to install something to a location outside the packaging folders if you don't have a way to tracking how to uninstall it.

Thus this request.

passwords

😭 good point.

CryptProtectData?

#1303 (comment) goes over packaging patterns for zip installations. That will end up in docs.

With installing a zip, explain Get-ToolsLocation and why Program Files is not the best idea.

If you were installing this exe utility, what would your preference be for the default installation path and addition to the machine/user/none path?

@ferventcoder
Copy link
Member

You do know we already store the switches and things passed?

@ferventcoder
Copy link
Member

If you were installing this exe utility, what would your preference be for the default installation path and addition to the machine/user/none path?

You might explain somewhere what "this exe utility" is exactly.

@ferventcoder
Copy link
Member

Typically for an exe utility, I just have it be in the package and let choco generate a shim into the bin folder.

@ferventcoder
Copy link
Member

ferventcoder commented Jan 27, 2018

(Even if this utility worked when shimmed, there's still the question of the configurable installation location.)

Just saw this. It's a good point. SysInternals package is a good example of the one that does that, but it uses Install-ChocolateyZipPackage with the path to allow for Chocolatey to capture the files as they get unpacked to a file it keeps around. There is a bug right now in finding that file - #1415 at uninstall time.

@ferventcoder
Copy link
Member

So you technically should be good just running Uninstall-ChocolateyZipPackage, except for a small issue currently.

@jnm2
Copy link
Author

jnm2 commented Jan 27, 2018

You might explain somewhere what "this exe utility" is exactly.

Still need to write the readme before I can package. 😳
Sudo for Windows. You type elevate <filename> <arguments> for a one-off and elevate on its own to elevate your current shell in-window. Unlike the existing Chocolatey elevate package and a couple others, this one is native not a script, doesn't rely on hacks like creating a scheduled task (!), and handles edge cases such as preserving cmd.exe's per-drive current directory tracking. I do wish I could have the elevate package ID. I'll see what the current owner thinks once I'm ready to go.

Shims don't work because it thinks the shim is the shell and executes it elevated which causes infinite recursion. I could try to make it work, but in principle this utility is incredibly light-weight on purpose. Going through your current shims kills that value proposition because your shims initialize the CLR (last I checked).

@ferventcoder
Copy link
Member

Shims don't work because it thinks the shim is the shell and executes it elevated which causes infinite recursion.

Yikes!

@ferventcoder
Copy link
Member

This utility sounds very cool - why not just call it sudo?

@jnm2
Copy link
Author

jnm2 commented Jan 27, 2018

Yikes!

I'm open to suggestions. But it's quite something when you type elevate in cmd or powershell and you stay in-window. It took a lot of trial and error.

why not just call it sudo?

Cause Windows' term is 'elevation'? Maybe I should. I'll probably have to talk to a different existing package owner.

@jnm2
Copy link
Author

jnm2 commented Jan 27, 2018

Where's the docs for #358? Title sounds like my issue is an exact duplicate for it.

https://github.com/jnm2/Elevate/tree/master/src/Elevate

@tcase
Copy link

tcase commented Feb 2, 2018

Just as a note, have you seen this tool? http://code.kliu.org/misc/elevate/

@jnm2
Copy link
Author

jnm2 commented Feb 3, 2018

I don't recall that one, thanks!

@bcurran3
Copy link

bcurran3 commented Feb 3, 2018

Not sure if it's the same author/version, but I use this all the time and it's already available in the Chocolatey community repository: https://chocolatey.org/packages/elevate

@jnm2
Copy link
Author

jnm2 commented Feb 3, 2018

Huh, apparently the contents of the package changed since I last saw it? https://chocolatey.org/packages/elevate#comment-3439057341

@coldacid
Copy link

Having access to package params in the uninstaller will really help with the retroarch package. Knowing when I can and should clean up out-of-tools shims and desktop icon would be very nice for users with the params I'm adding to the package.

TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Mar 10, 2022
Adds the ability for implementations of uninstall_run to reset the
config in ChocolateyPackageService via the resetConfigAction.
This will be required to prevent an issue similar to chocolatey#1443 for
uninstall, once useRememberedArgumentsForUninstall is added in.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Mar 10, 2022
This renames the set_package_config_for_upgrade to a more generic name,
and adds in another parameter to prepare for setting remembered args
for uninstall as well as upgrade
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Mar 10, 2022
Similar to the UseRememberedArgumentsForUpgrade feature, this will be
used to toggle if the remembered arguments are used during uninstall.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Mar 10, 2022
This adds the ability for the remembered argument to be reused for
uninstalls. It can be controlled via the userememberedargs and
ignorerememberedargs arguments, or via the previously added feature.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Mar 11, 2022
This renames the set_package_config_for_upgrade to a more generic name,
and adds in another parameter to prepare for setting remembered args
for uninstall as well as upgrade. It also updates the logging and
comments to make them generic for both upgrades and uninstalls.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Mar 11, 2022
Similar to the UseRememberedArgumentsForUpgrade feature, this will be
used to toggle if the remembered arguments are used during uninstall.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Mar 11, 2022
This adds the ability for the remembered argument to be reused for
uninstalls. It can be controlled via the userememberedargs and
ignorerememberedargs arguments, or via the previously added feature.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Mar 11, 2022
This adds the ability for the remembered argument to be reused for
uninstalls. It can be controlled via the userememberedargs and
ignorerememberedargs arguments, or via the previously added feature.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jun 27, 2022
This adds the ability for the remembered argument to be reused for
uninstalls. It can be controlled via the userememberedargs and
ignorerememberedargs arguments, or via the previously added feature.
@TheCakeIsNaOH TheCakeIsNaOH self-assigned this Jul 30, 2022
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Sep 25, 2022
This brings out the functionality from the
set_package_config_for_upgrade method to a more generic name, and adds
in another parameter to prepare for setting remembered args for
uninstall as well as upgrade. It also updates the logging and comments
to make them generic for both upgrades and uninstalls.

An alias/forwarding method is added for backwards compatibility.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Sep 25, 2022
Similar to the UseRememberedArgumentsForUpgrade feature, this will be
used to toggle if the remembered arguments are used during uninstall.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Sep 25, 2022
This adds the ability for the remembered argument to be reused for
uninstalls. It can be controlled via the userememberedargs and
ignorerememberedargs arguments, or via the previously added feature.
@Destroy666x
Copy link

Destroy666x commented Sep 23, 2023

This would be really handy for packages like tor-browser which just unpack files to some directory and that also support InstallDir param to change the path. Currently its uninstaller does nothing except removing desktop shortcut and I see no way around it as nothing is stored in registry either.

@pauby
Copy link
Member

pauby commented Sep 23, 2023

You can persist the information to disk on install and read it back on uninstall. I do this for PowerShell module packages I maintain.

@Destroy666x
Copy link

Yeah, you could so some workarounds, but that's not standarized and won't work for e.g. currently installed software.

@gep13 gep13 added this to the 2.3.0 milestone Dec 14, 2023
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jan 10, 2024
This brings out the functionality from the
set_package_config_for_upgrade method to a more generic name, and adds
in another parameter to prepare for setting remembered args for
uninstall as well as upgrade. It also updates the logging and comments
to make them generic for both upgrades and uninstalls.

An alias/forwarding method is added for backwards compatibility.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jan 10, 2024
Similar to the UseRememberedArgumentsForUpgrade feature, this will be
used to toggle if the remembered arguments are used during uninstall.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jan 10, 2024
This adds the ability for the remembered argument to be reused for
uninstalls. It can be controlled via the userememberedargs and
ignorerememberedargs arguments, or via the previously added feature.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jan 10, 2024
This brings out the functionality from the
set_package_config_for_upgrade method to a more generic name, and adds
in another parameter to prepare for setting remembered args for
uninstall as well as upgrade. It also updates the logging and comments
to make them generic for both upgrades and uninstalls.

An alias/forwarding method is added for backwards compatibility.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jan 10, 2024
Similar to the UseRememberedArgumentsForUpgrade feature, this will be
used to toggle if the remembered arguments are used during uninstall.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Jan 10, 2024
This adds the ability for the remembered argument to be reused for
uninstalls. It can be controlled via the userememberedargs and
ignorerememberedargs arguments, or via the previously added feature.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Apr 28, 2024
This brings out the functionality from the
set_package_config_for_upgrade method to a more generic name, and adds
in another parameter to prepare for setting remembered args for
uninstall as well as upgrade. It also updates the logging and comments
to make them generic for both upgrades and uninstalls.

An alias/forwarding method is added for backwards compatibility.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Apr 28, 2024
Similar to the UseRememberedArgumentsForUpgrade feature, this will be
used to toggle if the remembered arguments are used during uninstall.
TheCakeIsNaOH added a commit to TheCakeIsNaOH/choco that referenced this issue Apr 28, 2024
This adds the ability for the remembered argument to be reused for
uninstalls. It can be controlled via the userememberedargs and
ignorerememberedargs arguments, or via the previously added feature.
@gep13 gep13 modified the milestones: 2.3.0, 2.4.0 May 7, 2024
@gep13
Copy link
Member

gep13 commented May 7, 2024

Based on the changes in the associated PR's for this issue, I am going to have to bump this issue from the 2.3.0 release, and instead look at this again in the 2.4.0.

The suggested changes to the INuGetService mean that this would essentially be a breaking change, that would need a corresponding change in the Chocolatey Licensed Extension, which we haven't planned to do at the minute.

Instead of holding up the release of 2.3.0, we will look to do the work in this issue in a later release.

Apologies for any inconvenience that this causes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants