Automatically created shims should be removed during upgrade

AdmiringWorm commented 1 year ago

Checklist

[X] I have verified this is the correct repository for opening this issue.
[X] I have verified no other issues exist related to my problem.
[X] I have verified this is not an issue for a specific package.
[X] I have verified this issue is not security related.

What You Are Seeing?

When upgrading a package that previously relied on automatically created shims, but a new version no longer have an executable in the directory in the end of the execution the shims are still present due to them never being removed automatically on upgrades, unlike what is happening when uninstalling a package.

This semi-worked previously in pre-v2 version of Chocolatey CLI as it would not completely clean the directory of the previous package on upgrades which would leave the old executables in the directory and still implying that the shim works (just for the old version).

Now that Chocolatey CLI v2 completely cleans the directory, it is quickly found when such shims do not work anymore. As such we should delete these shims on upgrade (possibly right after creating a backup of the directory).

What is Expected?

Shims for old versions of a package automatically created by Chocolatey CLI should not be present after upgrading a package.

How Did You Get This To Happen?

The easiest way to find this issue, is with the starship package available on Chocolatey CLI.

Install version 1.11.0 of starship choco install starship --version 1.11.0
Upgrade this package using choco upgrade starship
Notice %ChocolateyInstall%\bin\starship.exe still exists
Notice %ChocolateyInstall%\lib\starship\tools\starship.exe does not exist (it will exist on pre-v2).

System Details

Operating System: Windows Server 2022 (10.0.20348.0)
Windows PowerShell version: 5.1.20348.859
Chocolatey CLI Version: 2.0.0
Chocolatey Licensed Extension version: N/A
Chocolatey License type: N/A
Terminal/Emulator: Windows PowerShell

Installed Packages

After Upgrade:

chocolatey 2.0.0
starship 1.15.0
starship.install 1.15.0

Output Log

https://gist.github.com/AdmiringWorm/f6d2a40dc9b7be9131369b55d44d424e

Additional Context

No response

gep13 commented 1 year ago

@AdmiringWorm something that came up during conversation with @corbob around this topic... Based on what you had planned for this one, what happens if there are lots of shims created during a package installation/upgrade? Would all the existing shims be removed and re-created, or would they only be removed/changed if they were removed/changed in the process of upgrading the package? The concern here being the length of time it takes to create all the shims in a package like sysinternals. If all the shims are re-created, this adds a fair bit of time to the overall process.

AdmiringWorm commented 1 year ago

@gep13 The change I am considering is just to remove broken shims (this will happen during the removal of the package files during the upgrade).

The existing logic for upgrade and installation is always to create the shims for the package, and I planned to keep this the same.

Although, that could be an improvement we can consider in the future.

chocolatey / choco