Fix auto-remove during upgrade#3913
Merged
Merged
Conversation
609c97d to
4286998
Compare
Contributor
|
Oh man we provided a lot of problems, didn't we :D |
Member
Author
|
@NathanKell, the KSP-RO project can be relied upon to probe formerly rare corner cases and sometimes turn them into core requirements overnight. 😜 |
techman83
approved these changes
Sep 20, 2023
Member
techman83
left a comment
There was a problem hiding this comment.
That looks like a sensible set of operations and an improvement of the logic. I shall leave the testing of the changes to @NathanKell / others.
I'd prefer the extra changes live their own commits, so context isn't lost if we have to look back through the history of why something was changed. But I'll leave it with your best judgement @HebaruSan
Great outcome!
This file contains hidden or 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
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.
Problem
In KSP-RO/RP-1@b9f0855,
RP-1's dependency onMagiCorewas removed from its metanetkan, and this change first reached users with the indexing ofRP-1v2.12 in KSP-CKAN/CKAN-meta@98a3461. When they upgraded,RP-1-ExpressInstalland all of its auto-installed dependencies were uninstalled (which should not have happened!). The dependency was subsequently restored pending a CKAN fix.How to reproduce for testing
https://github.com/KSP-CKAN/CKAN-meta/archive/0451525c2c9cefd16914032a2ca38d69c9e3f21c.tar.gz
RP-1-ExpressInstallRP-1, choose the 2.x series version(the other choices you make don't matter, issue will happen regardless)
RP-1dependency list because Show recommendations of full changeset with opt-out #3892 was merged,Recommendation suppression property for netkans KSP-RO/RP-1#2233 was reverted, and Recommendation suppression property for netkans KSP-RO/RP-1-ExpressInstall-Graphics#4 and Recommendation suppression property for netkans KSP-RO/RealismOverhaul#2913 have not yet been merged. I have sent a reminder about this.Those pull requests have now been merged, however I forgot that that will not help us here as we are intentionally reaching back into the archive to use old versions of the metadata for testing. But at least now it will not be as slow after the next client release.
https://github.com/KSP-CKAN/CKAN-meta/archive/98a346180f649c277e026a18cd69d0c3b5055ac8.tar.gz
You should see four mods ready to upgrade.
RP-1-ExpressInstall(and many other mods) would have been removed previously. Instead you should see this with this PR's changes:MagiCoreis gone andRP-1-ExpressInstallis still thereCause
ModuleInstaller.UninstallListbeforeModuleInstaller.InstallListbeforeModuleInstaller.Upgrade:CKAN/GUI/Main/MainInstall.cs
Lines 169 to 184 in 341adf8
... so the first change that happens is removal of MagiCore
... so RP-1, RealismOverhaul, RealFuels, and KSPCommunityFixes.
Changes
ModuleInstaller.Upgradehandles auto-removal internally(
ModuleInstaller.UninstallListalready handled auto-removal internally)ModuleInstaller.UninstallListunless the changeset consists only of installs and auto-removals, because we want them to be handled byUpgradeand/orUninstallListwhenever possibleSystem.dll,System.Core.dll, andSystem.Xml.dllin several of their ZIPs). Now this is fixed courtesy ofGroupBy.RP-1-ExpressInstallhas enough tags to wrap the display in GUI, but the panel containing them doesn't resize to fit. Now it does.Upgradeinstead of cobbling together its own custom changeset of uninstalls and installs, to take advantage of this smarter handling of auto-removal for that use case (fixes the part of [Bug]: CKAN 1.33 producing *numerous* problems on Kubuntu 20.04 #3912 where it's unnecessarily trying to uninstall Parallax-StockTextures). When this option is used, the changeset will say "Re-install (user requested)" in the English locale.Fixes #3911.