Skip to content

Better partial upgrade checking#4669

Merged
HebaruSan merged 7 commits into
KSP-CKAN:masterfrom
HebaruSan:fix/upgradeability
Jun 16, 2026
Merged

Better partial upgrade checking#4669
HebaruSan merged 7 commits into
KSP-CKAN:masterfrom
HebaruSan:fix/upgradeability

Conversation

@HebaruSan

@HebaruSan HebaruSan commented Jun 16, 2026

Copy link
Copy Markdown
Member

Problems

  • Discord user Hyperion_21 reported that an old installed version of MechJeb2-dev wasn't being shown as upgradeable, even though selecting the latest version manually worked fine.
  • If you had a lot of mods installed and unchecked the checkbox at the upper left to uninstall them all, CKAN could become unresponsive for a second or three.
  • Since the Description column can contain long strings, Layout and scaling related fixes #4591 added a tooltip so the user can easily read the full contents by hovering. However, if a mod is marked red due to a conflict, the conflict description overwrites the description contents tooltip.
  • If a mod's download count is unknown (e.g., the mods from the Sol metadata repository from Add Sol metadata as an official repository CKAN-meta#3383), the grid displays it as 0, incorrectly suggesting there have been no downloads.
  • If a mod's download size or install size is unknown, the grid displays it as "N/A", incorrectly suggesting that there is no download or install size.
  • The "Supported by" section of the recommendations screen could show mods that conflict with your installed mods, even though trying to install them would result in an error.

Causes

  • The upgradeability logic from Better version specific relationships at install and upgrade #4023 is somewhat limited; it only checks an installed mod's upgradeability in the context of upgrading everything else.
    In Hyperion_21's case, both MechJeb2-dev and RasterPropMonitor had old versions installed, and RasterPropMonitor's latest version had a conflict with MechJeb, which is provided by all versions of MechJeb2-dev, so upgrading RasterPropMonitor wasn't allowed. However, that potential update still ended up in the "context of upgrading everything else" against which MechJeb2-dev was checked, which blocked that mod's upgrade as well.
  • When you choose to uninstall a mod, there are some checks for other mods to be uninstalled, which are unnecessary if you're already uninstalling everything.
  • I didn't remember conflict tooltips existed during Layout and scaling related fixes #4591.
  • RepositoryDataManager.GetDownloadCount used .OfType<int>().FirstOrDefault() to handle null values, but the default of int is 0, so the function could never return null even thought it was designed for that. But even without that, GetValueOrDefault also returns 0 for a missing value when the dict's value type is int!
  • GUIMod.DownloadSize and GUIMod.InstallSize were set to "N/A" if the underlying values were 0
  • ModuleInstaller.CanInstall, which is responsible for checking which supporting mods are installable, lacked a toRemove param to handle mods being removed. Instead, it decided to pretend that all mods were being removed, so installed mods' conflicts would be ignored.

Motivations

Changes

  • Now the upgradeability logic is rewritten to generate sets of mods to upgrade using recursion. For each installed mod, it calls HasUpdate against the accumulated set of updates, and if it returns true, it first adds that update to the set and continues checking the rest of the mods. Then, regardless of what HasUpdate returned, it tries a group with the currently installed module instead, to catch cases where another mod prevents this one from updating. Each time it has a complete group of choices, it uses RelationshipResolver to make sure it's valid.
    Not only is this more correct, I think it may also be faster based on timings of test runs (it does incur fewer HasUpdate calls on average).
    A test is added to exercise Hyperion_21's use case, which passes on the new code and fails on the old code.
  • Now if you're uninstalling everything, we skip the additional checks for mods to uninstall, which will speed up that action.
  • Now when a mod is conflicted, the description tooltip is not overwritten.
  • Now GetDownloadCount uses TryGetValue to get the values and an explicit null check to avoid ever accidentally returning default(int), so the grid cells will be blank instead of 0 for unknown counts.
  • Now "N/A" only appears for the download and install sizes of metapackages; other mods with unknown sizes will have blank cells
  • Now mods being removed are passed to a new toRemove param of ModuleInstaller.CanInstall, so the "Supported by" list will be more accurate.
  • Now the blacklist info from Add blacklist to repositories.json CKAN-meta#3411 is parsed into a new property of RepositoryList and used to display an error when the user tries to add a known-bad repo:
    image

@HebaruSan HebaruSan added Bug Something is not working as intended Enhancement New features or functionality GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN Registry Issues affecting the registry Relationships Issues affecting depends, recommends, etc. labels Jun 16, 2026
@coveralls

coveralls commented Jun 16, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 27596207600

Coverage increased (+0.08%) to 87.801%

Details

  • Coverage increased (+0.08%) from the base build.
  • Patch coverage: 6 uncovered changes across 2 files (74 of 80 lines covered, 92.5%).
  • 4 coverage regressions across 2 files.

Uncovered Changes

File Changed Covered %
GUI/Model/ModList.cs 26 21 80.77%
Core/IO/ModuleInstaller.cs 5 4 80.0%
Total (8 files) 80 74 92.5%

Coverage Regressions

4 previously-covered lines in 2 files lost coverage.

File Lines Losing Coverage Coverage
GUI/Model/ModList.cs 3 78.64%
Core/IO/ModuleInstaller.cs 1 90.61%

Coverage Stats

Coverage Status
Relevant Lines: 9981
Covered Lines: 8628
Line Coverage: 86.44%
Relevant Branches: 2143
Covered Branches: 2017
Branch Coverage: 94.12%
Branches in Coverage %: Yes
Coverage Strength: 1.8 hits per line

💛 - Coveralls

@HebaruSan HebaruSan added the Tests Issues affecting the internal tests label Jun 16, 2026
@HebaruSan HebaruSan force-pushed the fix/upgradeability branch from af5dbc8 to 2d1c025 Compare June 16, 2026 04:32
@HebaruSan HebaruSan force-pushed the fix/upgradeability branch 2 times, most recently from a2df7a4 to 7f64f80 Compare June 16, 2026 05:26
@HebaruSan HebaruSan merged commit a524b8f into KSP-CKAN:master Jun 16, 2026
9 checks passed
@HebaruSan HebaruSan deleted the fix/upgradeability branch June 16, 2026 05:39
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 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

Development

Successfully merging this pull request may close these issues.

2 participants