(#508) Fix multiple packages being uninstall fails if one is a dependency #3142
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gep13
requested changes
Apr 28, 2023
After talking with @gep13 it was decided that we should prefer the usage of `var` instead of using an explicit type when possible. This commit makes the necessary change to the editor config to make this a suggestion.
The summary outputted by Chocolatey CLI about installed, upgrade or installed packages are listed in an unorder list of package names. This commit updates the report summary to ensure these are reported in alphabetical order in each category.
THe uninstall resolver has the potential to throw exceptions when it finds a package that can not be uninstalled due to it being a dependency on other packages, as such we need to be able to handle these scenarios and not stop further processing of the additional package names. The changes in this commit adds a try catch block around the resolver, and if it fails a new package result is created with the reason of the failure.
There are times where we have created a package result before attempting the uninstall of a package, when we come across scenarios for these we need to remove any previously acquired package to prevent further failures from occurring during the uninstall, or running of uninstall scripts. This was noticed when the user had requested to uninstall multiple packages, where one package was specified and at the same time was a dependency on a different package. When this happens, the first attempt may fail due to it being a dependency, but once the package referencing this package is uninstalled while removing all dependencies, the uninstall would still fail due to the previously cached result would have missing information about the package.
When we are uninstalling a package, and especially when one of its dependencies are uninstalled there should be a mention about what package and its version is being attempted. This commit adds information about this.
This commit updates the handling of collecting information about all package names the user has specified on the command line before attempting to do any uninstalls, or resolving any of its dependencies. This allows us to make a better decision about which packages need to be installed before others. The code that was added here ensures that if the user specified two packages, where one was a dependency on the other the package having the dependency will be attempted to be uninstalled first.
AdmiringWorm
force-pushed
the
PROJ-572/multiple-pkg-uninstalls
branch
from
2598de0 to
876a50b
Compare
There are a few E2E tests that we run through test kitchen that are failing when running on a licensed extension, however these have not been reproducible outside of test kitchen as of this date. As such we mark these tests as only available for Open Source where they run as expected. Further investigations may be needed to figure out the root cause of the issue.
AdmiringWorm
force-pushed
the
PROJ-572/multiple-pkg-uninstalls
branch
from
876a50b to
7cf83cc
Compare
2 tasks
|
@AdmiringWorm thanks for getting this fixed up! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description Of Changes
The changes in this pull request updates the uninstall behavior by now allowing multiple packages to be uninstalled at the same time, even if one of those packages are a dependency of the other package without it aborting all uninstalls.
To do this, it made it easier to do a custom sort of the packages to uninstall by keeping the order of dependencies being tried in the correct order. Unfortunately this is not always possible, and in these cases a Warning will be outputted to the user without stopping any uninstalls for other packages.
If the package that was warned about is still not uninstalled at the end of the entire execution, this will show up as an error/failure for the package.
Before running the uninstall script or any other uninstall related behavior, we remove any failures on a previously created package result for the package being uninstalled, this inadvertently fixes an issue where failures in a Before Modify script would stop fail the uninstallation even when it had been uninstalled.
Additionally, to make debugging easier it alphabetical sort was added in the report summary of the package, this change also makes the report summary be alphabetically sorted during installations and upgrades.
Motivation and Context
To have uninstallations succeed when it is possible, and to keep some of the backwards compatibility with v1.x
Testing
Operating Systems Testing
Change Types Made
Change Checklist
Related Issue