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

Restore uninstallation of dependencies #2579

Merged
merged 1 commit into from
Nov 17, 2018

Conversation

HebaruSan
Copy link
Member

Problem

As of #2561, if you try to remove a module that other installed modules depend on, you can't; the Apply button stays disabled and the status bar reports missing dependencies:

image

Before that pull request, this was allowed and would allow you to remove the module and mods depending on it. This isn't something we should break.

Background

MainModList.ComputeChangeSetFromModList creates multiple RelationshipResolvers for different purposes. First it creates one or more in a loop to resolve all provides relationships (even if there are no provides relationships involved):

CKAN/GUI/MainModList.cs

Lines 604 to 616 in 8b791df

while (!handled_all_too_many_provides)
{
//Can't await in catch clause - doesn't seem to work in mono. Hence this flag
TooManyModsProvideKraken kraken;
try
{
new RelationshipResolver(
modules_to_install,
modules_to_remove,
options, registry, version);
handled_all_too_many_provides = true;
continue;
}

After that it find mods with unsatisfied dependencies and queues them up to be removed:

CKAN/GUI/MainModList.cs

Lines 634 to 640 in 8b791df

foreach (var dependency in registry.FindReverseDependencies(modules_to_remove.Select(mod=>mod.identifier)))
{
//TODO This would be a good place to have a event that alters the row's graphics to show it will be removed
CkanModule module_by_version = registry.GetModuleByVersion(installed_modules[dependency].identifier,
installed_modules[dependency].version) ?? registry.InstalledModule(dependency).Module;
changeSet.Add(new ModChange(new GUIMod(module_by_version, registry, version), GUIModChangeType.Remove, null));
}

Then at the end it creates another RelationshipResolver to determine the actual change set:

CKAN/GUI/MainModList.cs

Lines 642 to 651 in 8b791df

var resolver = new RelationshipResolver(
modules_to_install,
changeSet.Where(change => change.ChangeType.Equals(GUIModChangeType.Remove)).Select(m => m.Mod.ToModule()),
options, registry, version);
changeSet.UnionWith(
resolver.ModList()
.Select(m => new ModChange(new GUIMod(m, registry, version), GUIModChangeType.Install, resolver.ReasonFor(m))));
return changeSet;

Cause

As of #2561, RelationshipResolver has a parameter for modules to uninstall. This allows us to specify a complete change set for evaluation, so that operations that depend on removal for their validity can succeed.

When that parameter was added, the provides loop of MainModList.ComputeChangeSetFromModList was updated to pass modules_to_remove, but this caused exceptions to be thrown if you were removing a dependency. This loop isn't designed to handle those exceptions, so they propagated out and messed everything up.

Changes

Now the loop to resolve provides relationships passes null instead of modules_to_remove. This will avoid raising exceptions for missing dependencies, and so the rest of the logic can run as expected. Other usages of RelationshipResolver still use work as before.

Fixes #2578.

@HebaruSan HebaruSan added Bug GUI Issues affecting the interactive GUI Pull request Relationships Issues affecting depends, recommends, etc. labels Nov 17, 2018
@politas politas merged commit 3ac163d into KSP-CKAN:master Nov 17, 2018
politas added a commit that referenced this pull request Nov 17, 2018
@HebaruSan HebaruSan deleted the fix/uninstall-dependencies branch November 17, 2018 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug GUI Issues affecting the interactive GUI Pull request Relationships Issues affecting depends, recommends, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't uninstall dependencies
2 participants