From 01b93f77c72f7dce832b79c9fbf0c0d7956d3889 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Thu, 17 Oct 2024 21:51:07 -0500 Subject: [PATCH] Fix skipping failed dependency downloads --- Core/Net/NetFileCache.cs | 2 +- Core/Registry/Registry.cs | 57 +++++++++++----------- Core/Relationships/SanityChecker.cs | 46 ++++++++---------- GUI/Main/MainInstall.cs | 22 ++++----- Tests/Core/Relationships/SanityChecker.cs | 58 ++++++++++++----------- 5 files changed, 94 insertions(+), 91 deletions(-) diff --git a/Core/Net/NetFileCache.cs b/Core/Net/NetFileCache.cs index 046c11e87..9fbd10d08 100644 --- a/Core/Net/NetFileCache.cs +++ b/Core/Net/NetFileCache.cs @@ -339,7 +339,7 @@ public void EnforceSizeLimit(long bytes, Registry registry) } } - private static int compareFiles(Dictionary> hashMap, FileInfo a, FileInfo b) + private static int compareFiles(IReadOnlyDictionary> hashMap, FileInfo a, FileInfo b) { // Compatible modules for file A hashMap.TryGetValue(a.Name[..8], out List? modulesA); diff --git a/Core/Registry/Registry.cs b/Core/Registry/Registry.cs index fe321b80b..3086a4ca3 100644 --- a/Core/Registry/Registry.cs +++ b/Core/Registry/Registry.cs @@ -1099,23 +1099,26 @@ public void CheckSanity() /// Installed DLCs /// List of modules whose dependencies are about to be or already removed. public static IEnumerable FindReverseDependencies( - List modulesToRemove, - List? modulesToInstall, - HashSet origInstalled, + ICollection modulesToRemove, + ICollection? modulesToInstall, + ICollection origInstalled, ICollection dlls, - IDictionary? dlc, + IDictionary dlc, Func? satisfiedFilter = null) { log.DebugFormat("Finding reverse dependencies of: {0}", string.Join(", ", modulesToRemove)); log.DebugFormat("From installed mods: {0}", string.Join(", ", origInstalled)); - log.DebugFormat("Installing mods: {0}", string.Join(", ", modulesToInstall ?? new List())); + if (modulesToInstall != null) + { + log.DebugFormat("Installing mods: {0}", string.Join(", ", modulesToInstall)); + } // The empty list has no reverse dependencies // (Don't remove broken modules if we're only installing) if (modulesToRemove.Count != 0) { // All modules in the input are included in the output - foreach (string starter in modulesToRemove) + foreach (var starter in modulesToRemove) { yield return starter; } @@ -1123,12 +1126,12 @@ public static IEnumerable FindReverseDependencies( { // Make our hypothetical install, and remove the listed modules from it. // Clone because we alter hypothetical. - var hypothetical = new HashSet(origInstalled); + var hypothetical = origInstalled.ToHashSet(); if (modulesToInstall != null) { // Pretend the mods we are going to install are already installed, so that dependencies that will be // satisfied by a mod that is going to be installed count as satisfied. - hypothetical = hypothetical.Concat(modulesToInstall).ToHashSet(); + hypothetical.UnionWith(modulesToInstall); } hypothetical.RemoveWhere(mod => modulesToRemove.Contains(mod.identifier)); @@ -1136,13 +1139,15 @@ public static IEnumerable FindReverseDependencies( log.DebugFormat("Keeping: {0}", string.Join(", ", hypothetical)); // Find what would break with this configuration - var brokenDeps = SanityChecker.FindUnsatisfiedDepends(hypothetical, dlls, dlc) + var brokenDeps = SanityChecker.FindUnsatisfiedDepends(hypothetical, + dlls, dlc) .ToList(); if (satisfiedFilter != null) { - brokenDeps.RemoveAll(kvp => satisfiedFilter(kvp.Item2)); + brokenDeps.RemoveAll(tuple => satisfiedFilter(tuple.Item2)); } - var brokenIdents = brokenDeps.Select(x => x.Item1.identifier).ToHashSet(); + var brokenIdents = brokenDeps.Select(tuple => tuple.Item1.identifier) + .ToHashSet(); if (modulesToInstall != null) { @@ -1153,24 +1158,23 @@ public static IEnumerable FindReverseDependencies( } log.DebugFormat("Broken: {0}", string.Join(", ", brokenIdents)); // Lazily return each newly found rev dep - foreach (string newFound in brokenIdents.Except(modulesToRemove)) + foreach (var newFound in brokenIdents.Except(modulesToRemove)) { yield return newFound; } // If nothing else would break, it's just the list of modules we're removing - var to_remove = new HashSet(modulesToRemove); - - if (to_remove.IsSupersetOf(brokenIdents)) + var toRemove = modulesToRemove.ToHashSet(); + if (toRemove.IsSupersetOf(brokenIdents)) { log.DebugFormat("{0} is a superset of {1}, work done", - string.Join(", ", to_remove), + string.Join(", ", toRemove), string.Join(", ", brokenIdents)); break; } // Otherwise, remove our broken modules as well, and recurse - modulesToRemove = brokenIdents.Union(to_remove).ToList(); + modulesToRemove = brokenIdents.Union(toRemove).ToList(); } } } @@ -1179,14 +1183,13 @@ public static IEnumerable FindReverseDependencies( /// Return modules which are dependent on the modules passed in or modules in the return list /// public IEnumerable FindReverseDependencies( - List modulesToRemove, - List? modulesToInstall = null, - Func? satisfiedFilter = null) - => FindReverseDependencies(modulesToRemove, - modulesToInstall, - new HashSet(installed_modules.Values.Select(x => x.Module)), - installed_dlls.Keys, - InstalledDlc, + List modulesToRemove, + List? modulesToInstall = null, + Func? satisfiedFilter = null) + => FindReverseDependencies(modulesToRemove, modulesToInstall, + installed_modules.Values.Select(im => im.Module) + .ToHashSet(), + InstalledDlls, InstalledDlc, satisfiedFilter); /// @@ -1196,7 +1199,7 @@ public IEnumerable FindReverseDependencies( /// /// dictionary[sha256 or sha1] = {mod1, mod2, mod3}; /// - public Dictionary> GetDownloadHashesIndex() + public IReadOnlyDictionary> GetDownloadHashesIndex() => downloadHashesIndex ??= (repoDataMgr?.GetAllAvailableModules(Repositories.Values) .SelectMany(availMod => availMod.module_version.Values) @@ -1229,7 +1232,7 @@ private IEnumerable> ModWithDownloadHashes(CkanModule /// /// dictionary[urlHash] = {mod1, mod2, mod3}; /// - public Dictionary> GetDownloadUrlHashIndex() + public IReadOnlyDictionary> GetDownloadUrlHashIndex() => downloadUrlHashIndex ??= (repoDataMgr?.GetAllAvailableModules(Repositories.Values) ?? Enumerable.Empty()) diff --git a/Core/Relationships/SanityChecker.cs b/Core/Relationships/SanityChecker.cs index 8e5b9fb84..3b2dbf4bd 100644 --- a/Core/Relationships/SanityChecker.cs +++ b/Core/Relationships/SanityChecker.cs @@ -19,9 +19,9 @@ public static class SanityChecker /// Throws a BadRelationshipsKraken describing the problems otherwise. /// Does nothing if the modules can happily co-exist. /// - public static void EnforceConsistency(IEnumerable modules, - ICollection dlls, - IDictionary? dlc = null) + public static void EnforceConsistency(IEnumerable modules, + ICollection dlls, + IDictionary dlc) { if (!CheckConsistency(modules, dlls, dlc, out List> unmetDepends, @@ -35,15 +35,15 @@ public static void EnforceConsistency(IEnumerable module /// Returns true if the mods supplied can co-exist. This checks depends/pre-depends/conflicts only. /// This is only used by tests! /// - public static bool IsConsistent(IEnumerable modules, - ICollection? dlls = null, - IDictionary? dlc = null) + public static bool IsConsistent(IEnumerable modules, + ICollection dlls, + IDictionary dlc) => CheckConsistency(modules, dlls, dlc, out var _, out var _); private static bool CheckConsistency(IEnumerable modules, - ICollection? dlls, - IDictionary? dlc, + ICollection dlls, + IDictionary dlc, out List> UnmetDepends, out modRelList Conflicts) { @@ -64,16 +64,12 @@ private static bool CheckConsistency(IEnumerable /// Each Key is the depending module, and each Value is the relationship. /// public static IEnumerable> FindUnsatisfiedDepends( - ICollection modules, - ICollection? dlls, - IDictionary? dlc) - => (modules?.Where(m => m.depends != null) - .SelectMany(m => (m.depends ?? Enumerable.Empty()) - .Select(dep => - new Tuple(m, dep))) - .Where(kvp => !kvp.Item2.MatchesAny(modules, dlls, dlc)) - ?? Enumerable.Empty>()); + ICollection modules, + ICollection? dlls, + IDictionary dlc) + => modules.SelectMany(m => (m.depends ?? Enumerable.Empty()) + .Where(dep => !dep.MatchesAny(modules, dlls, dlc)) + .Select(dep => Tuple.Create(m, dep))); /// /// Find conflicts among the given modules and DLLs. @@ -85,9 +81,9 @@ public static IEnumerable> FindUnsatis /// List of conflicts represented as pairs. /// Each Key is the depending module, and each Value is the relationship. /// - private static modRelList FindConflicting(List modules, - ICollection? dlls, - IDictionary? dlc) + private static modRelList FindConflicting(List modules, + ICollection dlls, + IDictionary dlc) => modules.Where(m => m.conflicts != null) .SelectMany(m => FindConflictingWith( m, @@ -96,10 +92,10 @@ private static modRelList FindConflicting(List mo dlls, dlc)) .ToList(); - private static IEnumerable FindConflictingWith(CkanModule module, - List otherMods, - ICollection? dlls, - IDictionary? dlc) + private static IEnumerable FindConflictingWith(CkanModule module, + List otherMods, + ICollection dlls, + IDictionary dlc) => module.conflicts?.Select(rel => rel.MatchesAny(otherMods, dlls, dlc, out CkanModule? other) ? new modRelPair(module, rel, other) : null) diff --git a/GUI/Main/MainInstall.cs b/GUI/Main/MainInstall.cs index f4f2041ba..d7b149c61 100644 --- a/GUI/Main/MainInstall.cs +++ b/GUI/Main/MainInstall.cs @@ -260,27 +260,27 @@ private void InstallMods(object? sender, DoWorkEventArgs? e) (m1, m2) => (m1 as CkanModule)?.download == (m2 as CkanModule)?.download); dfd.ShowDialog(this); }); - var skip = dfd?.Wait()?.Select(m => m as CkanModule) - .OfType() - .ToArray(); - var abort = dfd?.Abort; + var skip = (dfd?.Wait()?.OfType() ?? Enumerable.Empty()) + .ToArray(); + var abort = dfd?.Abort ?? false; dfd?.Dispose(); - if (abort ?? false) + if (abort) { canceled = true; e.Result = new InstallResult(false, changes); throw new CancelledActionKraken(); } - if (skip != null && skip.Length > 0) + if (skip.Length > 0) { // Remove mods from changeset that user chose to skip // and any mods depending on them - var dependers = registry.FindReverseDependencies( - skip.Select(s => s.identifier).ToList(), - fullChangeset, - // Consider virtual dependencies satisfied so user can make a new choice if they skip - rel => rel.LatestAvailableWithProvides(registry, crit).Count > 1) + var dependers = Registry.FindReverseDependencies( + skip.Select(s => s.identifier).ToList(), + fullChangeset, fullChangeset, + registry.InstalledDlls, registry.InstalledDlc, + // Consider virtual dependencies satisfied so user can make a new choice if they skip + rel => rel.LatestAvailableWithProvides(registry, crit).Count > 1) .ToHashSet(); toInstall.RemoveAll(m => dependers.Contains(m.identifier)); } diff --git a/Tests/Core/Relationships/SanityChecker.cs b/Tests/Core/Relationships/SanityChecker.cs index e99e6fe48..14a2a1860 100644 --- a/Tests/Core/Relationships/SanityChecker.cs +++ b/Tests/Core/Relationships/SanityChecker.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; @@ -21,6 +22,9 @@ public class SanityChecker private DisposableKSP? ksp; private TemporaryRepositoryData? repoData; + private readonly string[] dlls = Array.Empty(); + private readonly IDictionary dlc = new Dictionary(); + [OneTimeSetUp] public void Setup() { @@ -53,7 +57,7 @@ public void TearDown() [Test] public void Empty() { - Assert.IsTrue(CKAN.SanityChecker.IsConsistent(new List())); + Assert.IsTrue(CKAN.SanityChecker.IsConsistent(new List(), dlls, dlc)); } [Test] @@ -62,7 +66,7 @@ public void DogeCoin() // Test with a module that depends and conflicts with nothing. var mods = new List { registry?.LatestAvailable("DogeCoinFlag", null) }.OfType(); - Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods), "DogeCoinFlag"); + Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods, dlls, dlc), "DogeCoinFlag"); } [Test] @@ -70,13 +74,13 @@ public void CustomBiomes() { var mods = Enumerable.Repeat(registry?.LatestAvailable("CustomBiomes", null), 1).OfType().ToList(); - Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods), "CustomBiomes without data"); + Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods, dlls, dlc), "CustomBiomes without data"); mods.Add(registry?.LatestAvailable("CustomBiomesKerbal", null)!); - Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods), "CustomBiomes with stock data"); + Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods, dlls, dlc), "CustomBiomes with stock data"); mods.Add(registry?.LatestAvailable("CustomBiomesRSS", null)!); - Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods), "CustomBiomes with conflicting data"); + Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods, dlls, dlc), "CustomBiomes with conflicting data"); } [Test] @@ -85,15 +89,15 @@ public void CustomBiomesWithDlls() var mods = new List(); var dlls = new Dictionary { { "CustomBiomes", "" } }.Keys; - Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods, dlls), "CustomBiomes dll by itself"); + Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods, dlls, dlc), "CustomBiomes dll by itself"); // This would actually be a terrible thing for users to have, but it tests the // relationship we want. mods.Add(registry?.LatestAvailable("CustomBiomesKerbal", null)!); - Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods, dlls), "CustomBiomes DLL, with config added"); + Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods, dlls, dlc), "CustomBiomes DLL, with config added"); mods.Add(registry?.LatestAvailable("CustomBiomesRSS", null)!); - Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods, dlls), "CustomBiomes with conflicting data"); + Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods, dlls, dlc), "CustomBiomes with conflicting data"); } [Test] @@ -102,8 +106,8 @@ public void ConflictWithDll() var mods = new List { registry?.LatestAvailable("SRL", null)! }; var dlls = new Dictionary { { "QuickRevert", "" } }.Keys; - Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods), "SRL can be installed by itself"); - Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods, dlls), "SRL conflicts with QuickRevert DLL"); + Assert.IsTrue(CKAN.SanityChecker.IsConsistent(mods, this.dlls, dlc), "SRL can be installed by itself"); + Assert.IsFalse(CKAN.SanityChecker.IsConsistent(mods, dlls, dlc), "SRL conflicts with QuickRevert DLL"); } [Test] @@ -154,27 +158,27 @@ public void ReverseDepends() // Removing DCF should only remove itself. var to_remove = new List {"DogeCoinFlag"}; - TestDepends(to_remove, mods, null, null, to_remove, "DogeCoin Removal"); + TestDepends(to_remove, mods, dlls, dlc, to_remove, "DogeCoin Removal"); // Removing CB should remove its data, and vice-versa. to_remove.Clear(); to_remove.Add("CustomBiomes"); var expected = new List {"CustomBiomes", "CustomBiomesKerbal"}; - TestDepends(to_remove, mods, null, null, expected, "CustomBiomes removed"); + TestDepends(to_remove, mods, dlls, dlc, expected, "CustomBiomes removed"); // We expect the same result removing CBK to_remove.Clear(); to_remove.Add("CustomBiomesKerbal"); - TestDepends(to_remove, mods, null, null, expected, "CustomBiomesKerbal removed"); + TestDepends(to_remove, mods, dlls, dlc, expected, "CustomBiomesKerbal removed"); // And we expect the same result if we try to remove both. to_remove.Add("CustomBiomes"); - TestDepends(to_remove, mods, null, null, expected, "CustomBiomesKerbal and data removed"); + TestDepends(to_remove, mods, dlls, dlc, expected, "CustomBiomesKerbal and data removed"); // Finally, if we try to remove nothing, we shold get back the empty set. expected.Clear(); to_remove.Clear(); - TestDepends(to_remove, mods, null, null, expected, "Removing nothing"); + TestDepends(to_remove, mods, dlls, dlc, expected, "Removing nothing"); } [Test] @@ -204,7 +208,7 @@ public void IsConsistent_MismatchedDependencyVersion_Inconsistent() }; // Act & Assert - Assert.IsFalse(CKAN.SanityChecker.IsConsistent(modules)); + Assert.IsFalse(CKAN.SanityChecker.IsConsistent(modules, dlls, dlc)); } [Test] @@ -234,7 +238,7 @@ public void IsConsistent_MatchedDependencyVersion_Consistent() }; // Act & Assert - Assert.IsTrue(CKAN.SanityChecker.IsConsistent(modules)); + Assert.IsTrue(CKAN.SanityChecker.IsConsistent(modules, dlls, dlc)); } [Test] @@ -264,7 +268,7 @@ public void IsConsistent_MismatchedConflictVersion_Consistent() }; // Act & Assert - Assert.IsTrue(CKAN.SanityChecker.IsConsistent(modules)); + Assert.IsTrue(CKAN.SanityChecker.IsConsistent(modules, dlls, dlc)); } [Test] @@ -294,7 +298,7 @@ public void IsConsistent_MatchedConflictVersion_Inconsistent() }; // Act & Assert - Assert.IsFalse(CKAN.SanityChecker.IsConsistent(modules)); + Assert.IsFalse(CKAN.SanityChecker.IsConsistent(modules, dlls, dlc)); } [Test] @@ -326,7 +330,7 @@ public void IsConsistent_MultipleVersionsOfSelfConflictingModule_Consistent() }; // Act & Assert - Assert.IsTrue(CKAN.SanityChecker.IsConsistent(modules)); + Assert.IsTrue(CKAN.SanityChecker.IsConsistent(modules, dlls, dlc)); } [Test] @@ -360,15 +364,15 @@ public void IsConsistent_MultipleVersionsOfSelfProvidesConflictingModule_Consist }; // Act & Assert - Assert.IsTrue(CKAN.SanityChecker.IsConsistent(modules)); + Assert.IsTrue(CKAN.SanityChecker.IsConsistent(modules, dlls, dlc)); } - private static void TestDepends(List to_remove, - HashSet mods, - ICollection? dlls, - Dictionary? dlc, - List expected, - string message) + private static void TestDepends(List to_remove, + HashSet mods, + ICollection dlls, + IDictionary dlc, + List expected, + string message) { dlls ??= new Dictionary().Keys;