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

Better version specific relationships at install and upgrade #4023

Merged
merged 2 commits into from
Mar 9, 2024

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Feb 12, 2024

I've been sitting on these changes for a while because I wasn't quite happy with the implementation, but I think it's about as good as it's going to get for now, and I want to get it off my plate to focus on other issues.

Problem

If you install a mod that has a version-specific dependency on an older version of another mod, CKAN will appear to treat that other mod as upgradeable, but the upgrade will fail if attempted. Intermediate versions that could satisfy all requirements will not be discovered automatically, but may be selected manually by the user.

E.g., RP-0 recently did this:

depends:
  - name: RealismOverhaul
    max_version: v15.99.0.0

Versions after that are meant to be used only with RP-1. But RealismOverhaul will still show up as upgradeable, and trying to upgrade it will fail.

Cause

  • AvailableModule.DependsAndConflictsOK doesn't check version requirements of other modules that depend on the given module
  • In general, mod upgradeability is treated as a property of the individual mod, when it really needs to be a collective property of groups of mods that are installed or being installed.

Changes

  • Now reverse dependencies of installed mods are considered when looking for available versions
  • IRegistryQuerier.HasUpdate now accepts a list of other modules that are used to limit the compatible versions of the given mod
  • A new function IRegistryQuerier.CheckUpgradeable is created to provide more complex upgrading logic; it first finds the latest available versions of all installed mods, then uses their relationship constraints to roll back one another until a consistent changeset is generated. This function is now used by:
    • GUI's upgrade checkboxes, via setting GUIMod.HasUpdate from outside GUIMod
    • The changeset is updated based on what upgrades are actually allowed
    • ckan upgrade --all
    • ckan list's status characters
  • Mods with a "held" label are treated as not upgradeable, so they're excluded from that initial step of looking for the latest versions. When you add or remove labels or edit a label, we recalculate which upgrade checkboxes are shown.
  • Several tests are added to ensure ckan upgrade works as intended

Fixes #3849.
Fixes #3945.

@HebaruSan HebaruSan added Bug Something is not working as intended Enhancement New features or functionality GUI Issues affecting the interactive GUI Cmdline Issues affecting the command line Core (ckan.dll) Issues affecting the core part of CKAN Tests Issues affecting the internal tests Registry Issues affecting the registry Relationships Issues affecting depends, recommends, etc. labels Feb 12, 2024
@HebaruSan
Copy link
Member Author

Hi @lamont-granquist, @NathanKell, and @JonnyOThan, would you have time to test these changes in the next few days? The build is under the Artifacts dropdown on the Checks tab. I'd appreciate knowing about any problems you discover.

@NathanKell
Copy link
Contributor

Oooh, that's cool! Sadly I'm very swamped with non-KSP stuff atm so I make no promises. :( (Especially sucks given how desirable I find this, and how grateful I am!)

Gonna ping @siimav and @Capkirk123 as other folks who might be able to provide help here.

@HebaruSan HebaruSan force-pushed the feature/smart-upgrade branch 4 times, most recently from 746ed49 to b94d966 Compare February 19, 2024 06:48
@HebaruSan HebaruSan force-pushed the feature/smart-upgrade branch 4 times, most recently from d0af0ed to eb3eb64 Compare March 5, 2024 16:20
@HebaruSan HebaruSan force-pushed the feature/smart-upgrade branch from eb3eb64 to b702e1e Compare March 7, 2024 00:03
@HebaruSan HebaruSan added the In progress We're still working on this label Mar 7, 2024
@HebaruSan

This comment was marked as resolved.

@HebaruSan HebaruSan force-pushed the feature/smart-upgrade branch from b702e1e to 0f7ca67 Compare March 9, 2024 02:22
@HebaruSan HebaruSan removed the In progress We're still working on this label Mar 9, 2024
@HebaruSan
Copy link
Member Author

Finally got this into a state I'm happy with. The ManageMods.MarkModForInstall / GUIMod.SetInstallChecked / ManageMods.MarkModForUpdate / GUIMod.SetUpgradeChecked mess is eliminated to reduce coupling between the data model and the UI. ManageMods now listens for property change notifications of GUIMod.SelectedMod like the Versions tab already did, so the universal way to tweak the changeset is now to set that property. (The exception is replacing, which isn't represented in the data model at all.)

We did not manage to assemble a volunteer testing corps for this, so might as well merge now...

@HebaruSan HebaruSan merged commit 1cd1901 into KSP-CKAN:master Mar 9, 2024
8 checks passed
@HebaruSan HebaruSan deleted the feature/smart-upgrade branch March 9, 2024 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Cmdline Issues affecting the command line Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI Registry Issues affecting the registry Relationships Issues affecting depends, recommends, etc. Tests Issues affecting the internal tests
Projects
None yet
2 participants