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

(#508) Fix some files not being removed during uninstall/upgrade #3151

Merged
merged 5 commits into from
May 10, 2023

Conversation

AdmiringWorm
Copy link
Member

@AdmiringWorm AdmiringWorm commented May 10, 2023

Description Of Changes

This pull request makes changes to normalize the version number when creating file snapshots, and still support legacy version numbers. This fixes issues in licensed extension that would result in the file snapshot being null when trying to uninstall or upgrade packages.
Changes to ensure removal of nupkg files has also been added as if the nupkg file has been modified either manually, or by Chocolatey Licensed Extension it would be kept on disk during uninstalls and not removed.

Motivation and Context

To have uninstall and upgrade reliably works similar to how open source works.

Testing

  1. Ran all the upgrade and uninstall E2E tests in test kitchen.
  2. Manual run of the enabled tests in Server and client OS both with and without CLE installed.

Operating Systems Testing

  • Windows 10
  • Windows Server 2022
  • Windows Server 2019 (test kitchen)

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added (enabled).
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

@AdmiringWorm AdmiringWorm self-assigned this May 10, 2023
@AdmiringWorm AdmiringWorm requested review from gep13 and vexx32 and removed request for gep13 May 10, 2023 11:24
@AdmiringWorm AdmiringWorm force-pushed the PROJ-629/file-removal branch from 5fc7bcd to fbdb577 Compare May 10, 2023 12:36
@AdmiringWorm
Copy link
Member Author

@vexx32 thanks for catching those, I have updated the PR to fix the missing usage of throwError and using the correct parameter when formatting versions.

Copy link
Member

@corbob corbob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments about the Pester tests.

tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 Outdated Show resolved Hide resolved
tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 Outdated Show resolved Hide resolved
tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 Outdated Show resolved Hide resolved
tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 Outdated Show resolved Hide resolved
AdmiringWorm and others added 4 commits May 10, 2023 12:15
…tion

This commit updates the file snapshot service to honor version normalization
with fallbacks to old version methods.

This is done as we are moving towards normalizing version numbers in most of
the code, which causes a conflict when installing, upgrading or uninstalling
packages that sometimes it would use non-normalized version number,
and other times it would use normalized version numbers.
Now with this change it will prefer to use normalized version number, but will
continue to read/update package versions that was installed using
non-normalized version numbers.
This commit updates the uninstall process to ensure that the nupkg
file is removed, even in the cases where only partial file removals has
happened. This will prevent the package from being listed as installed
when partial removal is done.

While the original code does not pose any problems with normal usage
of CLI, it will not remove the nuget packages when the package has been
modified, like what Licensed Extension does when optimizing installations.
There are several tests that are fixed by the changes in a previous
commit, and in commits made in other projects. As such we need
to remove the tag FossOnly from these tests.
@corbob corbob force-pushed the PROJ-629/file-removal branch from fbdb577 to 12bf790 Compare May 10, 2023 19:40
Switch to use `--no-reduce` to prevent all reduction instead of just
reducing the nupkg.
@corbob corbob force-pushed the PROJ-629/file-removal branch from 12bf790 to 7cb721a Compare May 10, 2023 19:47
Copy link
Member

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, great work Kim! And thanks to Cory for fixing up the last few little things.

@vexx32 vexx32 merged commit 8e00b25 into chocolatey:release/2.0.0 May 10, 2023
@AdmiringWorm AdmiringWorm deleted the PROJ-629/file-removal branch September 25, 2023 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants