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

Only auto-delete manually installed DLLs if also replacing them #4024

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Feb 13, 2024

Problem

If you manually install a module that contains a plugin that matches the "main" module's identifier but which has been separated into a separate module (e.g., NearFutureSolar and NearFutureSolar-Core, which installs NearFutureSolar.dll), the main module will be detected as manually installed and can be upgraded to have CKAN take control of it, and the Core module will be installed automatically as a dependency, but the Core module's DLL will be deleted during the installation and missing afterwards.

Workaround: A right-click reinstall of the Core module restores the DLL.

Causes

I found two specific bugs related to this:

  • Registry.installed_dlls is only supposed to contain files that aren't registered to any installed module, but if you have CKAN take ownership of any of these files via upgrading, they aren't removed from it, which can confuse code that handles manually installed files.
  • ModuleInstaller.InstallModule's DLL handling block has a check that is supposed to cover this case (the comment references NearFutureElectrical and NearFutureElectrical-Core), but it doesn't actually skip the deletion if the DLL is missing from the module. It's also affected by the above bug where the DLL remains in the manually installed list after it's registered as CKAN-installed.

The net effect is that CKAN deletes NearFutureSolar.dll during the install of NearFutureSolar, after it is replaced by NearFutureSolar-Core.

Changes

  • Now when we register a module, any of its files that were previously tracked as a manually installed DLL are removed from Registry.installed_dlls
    (this would have fixed the bug by itself)
  • Now if a module doesn't contain its own DLL, or if it's registered to another installed module, it won't be deleted during installation
    (this also would have fixed the bug by itself)
  • A straggling untranslated "Continue?" prompt at install (probably only visible from CmdLine and ConsoleUI) is now internationalized using the existing string intended for this purpose

Fixes #4018.

@HebaruSan HebaruSan added Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Registry Issues affecting the registry i18n Issues regarding the internationalization of CKAN and PRs that need translating labels Feb 13, 2024
@HebaruSan HebaruSan changed the title Only auto-delete manually installed DLLs unless also replacing them Only auto-delete manually installed DLLs if also replacing them Feb 13, 2024
@HebaruSan HebaruSan merged commit 2c937f3 into KSP-CKAN:master Feb 13, 2024
8 checks passed
@HebaruSan HebaruSan deleted the fix/del-manual-dll branch February 13, 2024 17:23
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 i18n Issues regarding the internationalization of CKAN and PRs that need translating Registry Issues affecting the registry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using "Update" on a manually-installed NearFutureSolar will fail to install NearFutureSolarCore
1 participant