Fix installation of AVP while removing EVE default configs

Core/ModuleInstaller.cs

@@ -645,7 +645,9 @@ public void UninstallList(
                 )).Memoize();
             var goners = revdep.Union(
                     registry_manager.registry.FindRemovableAutoInstalled(
-                        registry_manager.registry.InstalledModules.Where(im => !revdep.Contains(im.identifier)))
+                        registry_manager.registry.InstalledModules
+                            .Where(im => !revdep.Contains(im.identifier))
+                            .Concat(installing?.Select(m => new InstalledModule(null, m, new string[0], false)) ?? new InstalledModule[0]))
                     .Select(im => im.identifier))
                 .ToList();
 

Core/Registry/IRegistryQuerier.cs

@@ -61,10 +61,11 @@ public interface IRegistryQuerier
         /// If no KSP version is provided, the latest module for *any* KSP version is given.
         /// </summary>
         List<CkanModule> LatestAvailableWithProvides(
-            string                  identifier,
-            GameVersionCriteria      ksp_version,
-            RelationshipDescriptor  relationship_descriptor = null,
-            IEnumerable<CkanModule> toInstall               = null
+            string identifier,
+            GameVersionCriteria ksp_version,
+            RelationshipDescriptor relationship_descriptor = null,
+            IEnumerable<CkanModule> installed = null,
+            IEnumerable<CkanModule> toInstall = null
         );
 
         /// <summary>

Core/Registry/Registry.cs

@@ -669,10 +669,12 @@ public void BuildTagIndex(ModuleTagList tags)
         /// <see cref="IRegistryQuerier.LatestAvailableWithProvides" />
         /// </summary>
         public List<CkanModule> LatestAvailableWithProvides(
-            string                  identifier,
-            GameVersionCriteria      ksp_version,
-            RelationshipDescriptor  relationship_descriptor = null,
-            IEnumerable<CkanModule> toInstall               = null)
+            string identifier,
+            GameVersionCriteria ksp_version,
+            RelationshipDescriptor relationship_descriptor = null,
+            IEnumerable<CkanModule> installed = null,
+            IEnumerable<CkanModule> toInstall = null
+        )
         {
             if (providers.TryGetValue(identifier, out HashSet<AvailableModule> provs))
             {
@@ -681,7 +683,7 @@ public List<CkanModule> LatestAvailableWithProvides(
                     .Select(am => am.Latest(
                         ksp_version,
                         relationship_descriptor,
-                        InstalledModules.Select(im => im.Module),
+                        installed ?? InstalledModules.Select(im => im.Module),
                         toInstall
                     ))
                     .Where(m => m?.ProvidesList?.Contains(identifier) ?? false)

Core/Relationships/RelationshipResolver.cs

@@ -86,8 +86,10 @@ public object Clone()
     /// </remarks>
     public class RelationshipResolver
     {
-        // A list of all the mods we're going to install.
         private static readonly ILog log = LogManager.GetLogger(typeof (RelationshipResolver));
+        /// <summary>
+        /// The list of all additional mods that need to be installed to satisfy all relationships.
+        /// </summary>
         private readonly Dictionary<string, CkanModule> modlist = new Dictionary<string, CkanModule>();
         private readonly List<CkanModule> user_requested_mods = new List<CkanModule>();
 
@@ -101,6 +103,9 @@ public class RelationshipResolver
         private readonly IRegistryQuerier registry;
         private readonly GameVersionCriteria GameVersion;
         private readonly RelationshipResolverOptions options;
+        /// <summary>
+        /// The list of already installed modules.
+        /// </summary>
         private readonly HashSet<CkanModule> installed_modules;
 
         /// <summary>
@@ -360,7 +365,8 @@ private void Resolve(CkanModule module, RelationshipResolverOptions options, IEn
         /// See RelationshipResolverOptions for further adjustments that can be made.
         /// </summary>
         private void ResolveStanza(IEnumerable<RelationshipDescriptor> stanza, SelectionReason reason,
-            RelationshipResolverOptions options, bool soft_resolve = false, IEnumerable<RelationshipDescriptor> old_stanza = null)
+            RelationshipResolverOptions options, bool soft_resolve = false,
+            IEnumerable<RelationshipDescriptor> old_stanza = null)
         {
             if (stanza == null)
             {
@@ -424,24 +430,24 @@ private void ResolveStanza(IEnumerable<RelationshipDescriptor> stanza, Selection
                 // Pass mod list in case an older version of a module is conflict-free while later versions have conflicts
                 var descriptor1 = descriptor;
                 List<CkanModule> candidates = descriptor
-                    .LatestAvailableWithProvides(registry, GameVersion, modlist.Values)
+                    .LatestAvailableWithProvides(registry, GameVersion, installed_modules, modlist.Values)
                     .Where(mod => !modlist.ContainsKey(mod.identifier)
                         && descriptor1.WithinBounds(mod)
-                        && MightBeInstallable(mod))
+                        && MightBeInstallable(mod, reason.Parent, installed_modules))
                     .ToList();
-                if (candidates.Count == 0)
+                if (!candidates.Any())
                 {
-                    // Nothing found, try again without mod list
-                    // (conflicts will still be caught below)
+                    // Nothing found, try again while simulating an empty mod list
+                    // Necessary for e.g. proceed_with_inconsistencies, conflicts will still be caught below
                     candidates = descriptor
-                        .LatestAvailableWithProvides(registry, GameVersion)
+                        .LatestAvailableWithProvides(registry, GameVersion, new CkanModule[0])
                         .Where(mod => !modlist.ContainsKey(mod.identifier)
                             && descriptor1.WithinBounds(mod)
-                            && MightBeInstallable(mod))
+                            && MightBeInstallable(mod, null, new CkanModule[0]))
                         .ToList();
                 }
 
-                if (candidates.Count == 0)
+                if (!candidates.Any())
                 {
                     if (!soft_resolve)
                     {
@@ -454,8 +460,7 @@ private void ResolveStanza(IEnumerable<RelationshipDescriptor> stanza, Selection
                 if (candidates.Count > 1)
                 {
                     // Oh no, too many to pick from!
-                    // TODO: It would be great if instead we picked the one with the
-                    // most recommendations.
+                    // TODO: It would be great if instead we picked the one with the most recommendations.
                     if (options.without_toomanyprovides_kraken)
                     {
                         continue;
@@ -579,9 +584,12 @@ private void Add(CkanModule module, SelectionReason reason)
         /// exist for current version.
         /// </summary>
         /// <param name="module">The module to consider</param>
+        /// <param name="stanzaSource">The source of the relationship stanza we're investigating the candidate for</param>
+        /// <param name="installed">The list of installed modules in the current resolver state</param>
         /// <param name="compatible">For internal use</param>
-        /// <returns>If it has dependencies compatible for the current version</returns>
-        private bool MightBeInstallable(CkanModule module, List<string> compatible = null)
+        /// <returns>Whether its dependencies are compatible with the current game version</returns>
+        private bool MightBeInstallable(CkanModule module, CkanModule stanzaSource = null,
+            IEnumerable<CkanModule> installed = null, List<string> compatible = null)
         {
             if (module.IsDLC)
                 return false;
@@ -599,18 +607,21 @@ private bool MightBeInstallable(CkanModule module, List<string> compatible = nul
             // in case a dependent depends on it
             compatible.Add(module.identifier);
 
+            var toInstall = stanzaSource != null ? new List<CkanModule> {stanzaSource} : null;
+
             // Get list of lists of dependency choices
             var needed = module.depends
                 // Skip dependencies satisfied by installed modules
-                .Where(depend => !depend.MatchesAny(installed_modules, null, null))
-                .Select(depend => depend.LatestAvailableWithProvides(registry, GameVersion));
+                .Where(depend => !depend.MatchesAny(installed, null, null))
+                // ... or by modules that are about to be installed
+                .Select(depend => depend.LatestAvailableWithProvides(registry, GameVersion, installed, toInstall)).ToList();
 
             log.DebugFormat("Trying to satisfy: {0}",
                 string.Join("; ", needed.Select(need =>
                     string.Join(", ", need.Select(mod => mod.identifier)))));
 
-            //We need every dependency to have at least one possible module
-            var installable = needed.All(need => need.Any(mod => MightBeInstallable(mod, compatible)));
+            // We need every dependency to have at least one possible module
+            var installable = needed.All(need => need.Any(mod => MightBeInstallable(mod, stanzaSource, installed, compatible)));
             compatible.Remove(module.identifier);
             return installable;
         }

Core/Types/RelationshipDescriptor.cs

@@ -17,7 +17,10 @@ IDictionary<string, ModuleVersion> dlc
 
         public abstract bool WithinBounds(CkanModule otherModule);
 
-        public abstract List<CkanModule> LatestAvailableWithProvides(IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable<CkanModule> toInstall = null);
+        public abstract List<CkanModule> LatestAvailableWithProvides(
+            IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable<CkanModule> installed = null,
+            IEnumerable<CkanModule> toInstall = null
+        );
 
         public abstract bool Equals(RelationshipDescriptor other);
 
@@ -138,9 +141,12 @@ IDictionary<string, ModuleVersion> dlc
             return false;
         }
 
-        public override List<CkanModule> LatestAvailableWithProvides(IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable<CkanModule> toInstall = null)
+        public override List<CkanModule> LatestAvailableWithProvides(
+            IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable<CkanModule> installed = null,
+            IEnumerable<CkanModule> toInstall = null
+        )
         {
-            return registry.LatestAvailableWithProvides(name, crit, this, toInstall);
+            return registry.LatestAvailableWithProvides(name, crit, this, installed, toInstall);
         }
 
         public override bool Equals(RelationshipDescriptor other)
@@ -225,9 +231,12 @@ IDictionary<string, ModuleVersion> dlc
                 ?? false;
         }
 
-        public override List<CkanModule> LatestAvailableWithProvides(IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable<CkanModule> toInstall = null)
+        public override List<CkanModule> LatestAvailableWithProvides(
+            IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable<CkanModule> installed = null,
+            IEnumerable<CkanModule> toInstall = null
+        )
         {
-            return any_of?.SelectMany(r => r.LatestAvailableWithProvides(registry, crit, toInstall)).ToList();
+            return any_of?.SelectMany(r => r.LatestAvailableWithProvides(registry, crit, installed, toInstall)).ToList();
         }
 
         public override bool Equals(RelationshipDescriptor other)

GUI/Main/MainInstall.cs

@@ -186,6 +186,7 @@ out Dictionary<CkanModule, HashSet<string>> supporters
                         {
                             installer.UninstallList(toUninstall.Select(m => m.identifier),
                                 ref possibleConfigOnlyDirs, registry_manager, false, toInstall);
+                            toUninstall.Clear();
                             processSuccessful = true;
                         }
                     }
@@ -195,6 +196,7 @@ out Dictionary<CkanModule, HashSet<string>> supporters
                         if (!installCanceled)
                         {
                             installer.InstallList(toInstall, opts.Value, registry_manager, ref possibleConfigOnlyDirs, downloader, false);
+                            toInstall.Clear();
                             processSuccessful = true;
                         }
                     }
@@ -204,6 +206,7 @@ out Dictionary<CkanModule, HashSet<string>> supporters
                         if (!installCanceled)
                         {
                             installer.Upgrade(toUpgrade, downloader, ref possibleConfigOnlyDirs, registry_manager, true, true, false);
+                            toUpgrade.Clear();
                             processSuccessful = true;
                         }
                     }

GUI/Model/ModList.cs

@@ -161,9 +161,12 @@ public IEnumerable<ModChange> ComputeChangeSetFromModList(
                 }
             }
             foreach (var im in registry.FindRemovableAutoInstalled(
-                registry.InstalledModules.Where(im => !modules_to_remove.Any(m => m.identifier == im.identifier) || modules_to_install.Any(m => m.identifier == im.identifier))
+                registry.InstalledModules
+                    .Where(im => modules_to_remove.All(m => m.identifier != im.identifier) || modules_to_install.Any(m => m.identifier == im.identifier))
+                    .Concat(modules_to_install.Select(m => new InstalledModule(null, m, new string[0], false)))
             ))
             {
+                // TODO this removes modules that a newly installed mod will depend on, and gets readded as module to install in the RelationshipResolver
                 changeSet.Add(new ModChange(im.Module, GUIModChangeType.Remove, new SelectionReason.NoLongerUsed()));
                 modules_to_remove.Add(im.Module);
             }

Tests/Core/Relationships/RelationshipResolver.cs

@@ -6,7 +6,6 @@
 using Tests.Data;
 using System.IO;
 using CKAN.Versioning;
-using Tests.Core.Types;
 using RelationshipDescriptor = CKAN.RelationshipDescriptor;
 
 namespace Tests.Core.Relationships
@@ -72,7 +71,6 @@ public void Constructor_WithConflictingModules()
 
         [Test]
         [Category("Version")]
-
         public void Constructor_WithConflictingModulesVersion_Throws()
         {
             var list = new List<string>();
@@ -836,7 +834,7 @@ public void ReasonFor_WithUserAddedMods_GivesReasonUserAdded()
         }
 
         [Test]
-        public void ReasonFor_WithSugestedMods_GivesCorrectParent()
+        public void ReasonFor_WithSuggestedMods_GivesCorrectParent()
         {
             var list = new List<string>();
             var suggested = generator.GeneratorRandomModule();
@@ -913,6 +911,94 @@ public void AutodetectedCanSatisfyRelationships()
             }
         }
 
+        // Models the EVE - EVE-Config - AVP - AVP-Textures relationship
+        [Test]
+        public void UninstallingConflictingModule_InstallingRecursiveDependencies_ResolvesSuccessfully()
+        {
+            using (var ksp = new DisposableKSP())
+            {
+                // Arrange: create dummy modules that resemble the relationship entanglement, and make them available
+                var eve = generator.GeneratorRandomModule(
+                    identifier: "EnvironmentalVisualEnhancements",
+                    depends: new List<RelationshipDescriptor>
+                        {new ModuleRelationshipDescriptor {name = "EnvironmentalVisualEnhancements-Config"}}
+                );
+                var eveDefaultConfig = generator.GeneratorRandomModule(
+                    identifier: "EnvironmentalVisualEnhancements-Config-stock",
+                    provides: new List<string> {"EnvironmentalVisualEnhancements-Config"},
+                    conflicts: new List<RelationshipDescriptor>
+                        {new ModuleRelationshipDescriptor {name = "EnvironmentalVisualEnhancements-Config"}}
+                );
+                var avp = generator.GeneratorRandomModule(
+                    identifier: "AstronomersVisualPack",
+                    provides: new List<string> {"EnvironmentalVisualEnhancements-Config"},
+                    depends: new List<RelationshipDescriptor>
+                    {
+                        new ModuleRelationshipDescriptor {name = "AVP-Textures"},
+                        new ModuleRelationshipDescriptor {name = "EnvironmentalVisualEnhancements"}
+                    },
+                    conflicts: new List<RelationshipDescriptor>
+                            {new ModuleRelationshipDescriptor {name = "EnvironmentalVisualEnhancements-Config"}}
+                );
+                var avp2kTextures = generator.GeneratorRandomModule(
+                    identifier: "AVP-2kTextures",
+                    provides: new List<string> {"AVP-Textures"},
+                    depends: new List<RelationshipDescriptor>
+                        {new ModuleRelationshipDescriptor {name = "AstronomersVisualPack"}},
+                    conflicts: new List<RelationshipDescriptor>
+                        {new ModuleRelationshipDescriptor {name = "AVP-Textures"}}
+                );
+
+                AddToRegistry(eve, eveDefaultConfig, avp, avp2kTextures);
+
+                // Start with eve and eveDefaultConfig installed
+                registry.RegisterModule(eve, new string[0], ksp.KSP, false);
+                registry.RegisterModule(eveDefaultConfig, new string[0], ksp.KSP, false);
+
+                Assert.DoesNotThrow(() => registry.CheckSanity());
+
+                List<CkanModule> modulesToInstall;
+                List<CkanModule> modulesToRemove;
+                RelationshipResolver resolver;
+
+                // Act and assert: play through different possible user interactions
+                // Scenario 1 - Try installing AVP, expect an exception for proceed_with_inconsistencies=false
+
+                modulesToInstall = new List<CkanModule> { avp };
+                modulesToRemove = new List<CkanModule>();
+
+                options.proceed_with_inconsistencies = false;
+                Assert.Throws<InconsistentKraken>(() =>
+                {
+                    resolver = new RelationshipResolver(modulesToInstall, modulesToRemove, options, registry, null);
+                });
+
+                // Scenario 2 - Try installing AVP, expect no exception for proceed_with_inconsistencies=true, but a conflict list
+
+                resolver = null;
+                options.proceed_with_inconsistencies = true;
+                Assert.DoesNotThrow(() =>
+                {
+                    resolver = new RelationshipResolver(modulesToInstall, modulesToRemove, options, registry, null);
+                });
+                CollectionAssert.AreEquivalent(new List<CkanModule> {avp, eveDefaultConfig}, resolver.ConflictList.Keys);
+
+                // Scenario 3 - Try uninstalling eveDefaultConfig and installing avp, should work and result in no conflicts
+
+                modulesToInstall = new List<CkanModule> { avp };
+                modulesToRemove = new List<CkanModule> { eveDefaultConfig };
+
+                resolver = null;
+                options.proceed_with_inconsistencies = false;
+                Assert.DoesNotThrow(() =>
+                {
+                    resolver = new RelationshipResolver(modulesToInstall, modulesToRemove, options, registry, null);
+                });
+                Assert.IsEmpty(resolver.ConflictList);
+                CollectionAssert.AreEquivalent(new List<CkanModule> {avp, avp2kTextures}, resolver.ModList());
+            }
+        }
+
         private void AddToRegistry(params CkanModule[] modules)
         {
             foreach (var module in modules)