From cc9197920625cf84c1b7a4e570e70c967ce6bd5b Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Thu, 28 Sep 2023 10:30:40 -0500 Subject: [PATCH 01/22] Trivial formatting changes --- Cmdline/Action/Import.cs | 9 +- Cmdline/Action/List.cs | 5 +- Cmdline/Action/Upgrade.cs | 6 +- Core/ModuleInstaller.cs | 15 +- Core/Properties/Resources.resx | 2 +- Core/Registry/IRegistryQuerier.cs | 14 +- Core/Registry/Registry.cs | 25 +- Core/Relationships/RelationshipResolver.cs | 372 +++++++++--------- Core/Relationships/SanityChecker.cs | 74 +--- Core/Repositories/RepositoryData.cs | 11 +- Core/Types/CkanModule.cs | 6 - Core/Types/Kraken.cs | 69 ++-- Core/Types/RelationshipDescriptor.cs | 56 +-- GUI/Controls/ChooseRecommendedMods.cs | 7 +- GUI/Controls/EditModpack.cs | 4 +- GUI/Controls/InstallationHistory.cs | 7 +- GUI/Controls/ManageMods.cs | 2 +- GUI/Controls/ModInfoTabs/Relationships.cs | 7 +- GUI/Controls/ModInfoTabs/Versions.cs | 11 +- Netkan/ConsoleUser.cs | 2 +- .../Transformers/SpaceWarpInfoTransformer.cs | 4 +- Tests/CmdLine/ResourcesTests.cs | 1 + Tests/Core/Net/NetAsyncDownloaderTests.cs | 21 +- .../Relationships/RelationshipResolver.cs | 2 +- Tests/Data/TemporaryRepository.cs | 2 +- 25 files changed, 327 insertions(+), 407 deletions(-) diff --git a/Cmdline/Action/Import.cs b/Cmdline/Action/Import.cs index fc51d74546..5abfed3e46 100644 --- a/Cmdline/Action/Import.cs +++ b/Cmdline/Action/Import.cs @@ -52,11 +52,10 @@ public int RunCommand(CKAN.GameInstance instance, object options) HashSet possibleConfigOnlyDirs = null; if (toInstall.Count > 0) { - installer.InstallList( - toInstall, - new RelationshipResolverOptions(), - regMgr, - ref possibleConfigOnlyDirs); + installer.InstallList(toInstall, + new RelationshipResolverOptions(), + regMgr, + ref possibleConfigOnlyDirs); } return Exit.OK; } diff --git a/Cmdline/Action/List.cs b/Cmdline/Action/List.cs index 3a16482914..904a8963c4 100644 --- a/Cmdline/Action/List.cs +++ b/Cmdline/Action/List.cs @@ -1,4 +1,5 @@ using System; +using System.IO; using System.Collections.Generic; using log4net; @@ -39,7 +40,9 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options) if (!(options.porcelain) && exportFileType == null) { user.RaiseMessage(""); - user.RaiseMessage(Properties.Resources.ListGameFound, instance.game.ShortName, instance.GameDir()); + user.RaiseMessage(Properties.Resources.ListGameFound, + instance.game.ShortName, + instance.GameDir().Replace('/', Path.DirectorySeparatorChar)); user.RaiseMessage(""); user.RaiseMessage(Properties.Resources.ListGameVersion, instance.game.ShortName, instance.Version()); user.RaiseMessage(""); diff --git a/Cmdline/Action/Upgrade.cs b/Cmdline/Action/Upgrade.cs index 919bc3a73f..a23fa20aa5 100644 --- a/Cmdline/Action/Upgrade.cs +++ b/Cmdline/Action/Upgrade.cs @@ -134,7 +134,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options) } catch (ModuleNotFoundKraken kraken) { - user.RaiseMessage(Properties.Resources.UpgradeNotFound, kraken.module); + user.RaiseMessage(Properties.Resources.UpgradeNotFound, $"{kraken.module} {kraken.version}"); return Exit.ERROR; } catch (InconsistentKraken kraken) @@ -166,7 +166,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options) /// IUser object for output /// Game instance to use /// List of modules to upgrade - public void UpgradeModules(GameInstanceManager manager, IUser user, CKAN.GameInstance instance, bool ConfirmPrompt, List modules) + private void UpgradeModules(GameInstanceManager manager, IUser user, CKAN.GameInstance instance, bool ConfirmPrompt, List modules) { UpgradeModules(manager, user, instance, repoData, (ModuleInstaller installer, NetAsyncModulesDownloader downloader, RegistryManager regMgr, ref HashSet possibleConfigOnlyDirs) => @@ -183,7 +183,7 @@ public void UpgradeModules(GameInstanceManager manager, IUser user, CKAN.GameIns /// IUser object for output /// Game instance to use /// List of identifier[=version] to upgrade - public void UpgradeModules(GameInstanceManager manager, IUser user, CKAN.GameInstance instance, List identsAndVersions) + private void UpgradeModules(GameInstanceManager manager, IUser user, CKAN.GameInstance instance, List identsAndVersions) { UpgradeModules(manager, user, instance, repoData, (ModuleInstaller installer, NetAsyncModulesDownloader downloader, RegistryManager regMgr, ref HashSet possibleConfigOnlyDirs) => diff --git a/Core/ModuleInstaller.cs b/Core/ModuleInstaller.cs index 6634488c0e..8ec222e1f0 100644 --- a/Core/ModuleInstaller.cs +++ b/Core/ModuleInstaller.cs @@ -133,7 +133,12 @@ reason is SelectionReason.UserRequested /// Propagates a FileExistsKraken if we were going to overwrite a file. /// Propagates a CancelledActionKraken if the user cancelled the install. /// - public void InstallList(ICollection modules, RelationshipResolverOptions options, RegistryManager registry_manager, ref HashSet possibleConfigOnlyDirs, IDownloader downloader = null, bool ConfirmPrompt = true) + public void InstallList(ICollection modules, + RelationshipResolverOptions options, + RegistryManager registry_manager, + ref HashSet possibleConfigOnlyDirs, + IDownloader downloader = null, + bool ConfirmPrompt = true) { // TODO: Break this up into smaller pieces! It's huge! if (modules.Count == 0) @@ -1060,7 +1065,13 @@ public void Upgrade(IEnumerable identifiers, IDownloader netAsyncDownloa /// Will *re-install* or *downgrade* (with a warning) as well as upgrade. /// Throws ModuleNotFoundKraken if a module is not installed. /// - public void Upgrade(IEnumerable modules, IDownloader netAsyncDownloader, ref HashSet possibleConfigOnlyDirs, RegistryManager registry_manager, bool enforceConsistency = true, bool resolveRelationships = false, bool ConfirmPrompt = true) + public void Upgrade(IEnumerable modules, + IDownloader netAsyncDownloader, + ref HashSet possibleConfigOnlyDirs, + RegistryManager registry_manager, + bool enforceConsistency = true, + bool resolveRelationships = false, + bool ConfirmPrompt = true) { modules = modules.Memoize(); var registry = registry_manager.registry; diff --git a/Core/Properties/Resources.resx b/Core/Properties/Resources.resx index 6ec64b20a3..ad7957c401 100644 --- a/Core/Properties/Resources.resx +++ b/Core/Properties/Resources.resx @@ -260,7 +260,7 @@ Please remove manually before trying to install it. {0} dependency on {1} version {2} not satisfied (any) Module {0} is provided by more than one available module. Please choose one of the following: - The following inconsistencies were found: + Inconsistencies were found: {0} missing dependency {1} {0} conflicts with {1} Uh oh, the following things went wrong when downloading... diff --git a/Core/Registry/IRegistryQuerier.cs b/Core/Registry/IRegistryQuerier.cs index 14832d4cfa..695ae53a48 100644 --- a/Core/Registry/IRegistryQuerier.cs +++ b/Core/Registry/IRegistryQuerier.cs @@ -16,10 +16,10 @@ namespace CKAN /// public interface IRegistryQuerier { - ReadOnlyDictionary Repositories { get; } - IEnumerable InstalledModules { get; } - IEnumerable InstalledDlls { get; } - IDictionary InstalledDlc { get; } + ReadOnlyDictionary Repositories { get; } + IEnumerable InstalledModules { get; } + IEnumerable InstalledDlls { get; } + IDictionary InstalledDlc { get; } /// /// Returns a simple array of the latest compatible module for each identifier for @@ -81,9 +81,9 @@ List LatestAvailableWithProvides( /// /// Finds and returns all modules that could not exist without the listed modules installed, including themselves. /// - IEnumerable FindReverseDependencies( - List modulesToRemove, List modulesToInstall = null, Func satisfiedFilter = null - ); + IEnumerable FindReverseDependencies(List modulesToRemove, + List modulesToInstall = null, + Func satisfiedFilter = null); /// /// Gets the installed version of a mod. Does not check for provided or autodetected mods. diff --git a/Core/Registry/Registry.cs b/Core/Registry/Registry.cs index 3505b74cc1..5f472cdc9d 100644 --- a/Core/Registry/Registry.cs +++ b/Core/Registry/Registry.cs @@ -38,10 +38,15 @@ public class Registry : IEnlistmentNotification, IRegistryQuerier private SortedDictionary repositories; // name => path - [JsonProperty] private Dictionary installed_dlls; - [JsonProperty] private Dictionary installed_modules; + [JsonProperty] + private Dictionary installed_dlls; + + [JsonProperty] + private Dictionary installed_modules; + // filename (case insensitive on Windows) => module - [JsonProperty] private Dictionary installed_files; + [JsonProperty] + private Dictionary installed_files; /// /// Returns all the activated registries. @@ -65,6 +70,7 @@ public void RepositoriesSet(SortedDictionary value) InvalidateAvailableModCaches(); repositories = value; } + /// /// Wrapper around this.repositories.Clear() that invalidates /// available mod caches @@ -789,11 +795,8 @@ public List LatestAvailableWithProvides( { // For each AvailableModule, we want the latest one matching our constraints return provs - .Select(am => am.Latest( - gameVersion, - relationship_descriptor, - installed ?? InstalledModules.Select(im => im.Module), - toInstall)) + .Select(am => am.Latest(gameVersion, relationship_descriptor, + installed, toInstall)) .Where(m => m?.ProvidesList?.Contains(identifier) ?? false) .ToList(); } @@ -1118,12 +1121,6 @@ public void CheckSanity() installed_dlls.Keys, InstalledDlc); } - public List GetSanityErrors() - => SanityChecker.ConsistencyErrors(installed_modules.Select(pair => pair.Value.Module), - installed_dlls.Keys, - InstalledDlc) - .ToList(); - /// /// Finds and returns all modules that could not exist without the listed modules installed, including themselves. /// Acts recursively and lazily. diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index 1364dcf5d5..b0a8e7d23e 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -2,128 +2,46 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; + using log4net; + using CKAN.Versioning; using CKAN.Extensions; namespace CKAN { - - // TODO: It would be lovely to get rid of the `without` fields, - // and replace them with `with` fields. Humans suck at inverting - // cases in their heads. - public class RelationshipResolverOptions : ICloneable - { - /// - /// If true, add recommended mods, and their recommendations. - /// - public bool with_recommends = true; - - /// - /// If true, add suggests, but not suggested suggests. :) - /// - public bool with_suggests = false; - - /// - /// If true, add suggested modules, and *their* suggested modules, too! - /// - public bool with_all_suggests = false; - - /// - /// If true, surpresses the TooManyProvides kraken when resolving - /// relationships. Otherwise, we just pick the first. - /// - public bool without_toomanyprovides_kraken = false; - - /// - /// If true, we skip our sanity check at the end of our relationship - /// resolution. Note that non-sane resolutions can't actually be - /// installed, so this is mostly useful for giving the user feedback - /// on failed resolutions. - /// - public bool without_enforce_consistency = false; - - /// - /// If true, we'll populate the `conflicts` field, rather than immediately - /// throwing a kraken when inconsistencies are detected. Again, these - /// solutions are non-installable, so mostly of use to provide user - /// feedback when things go wrong. - /// - public bool proceed_with_inconsistencies = false; - - /// - /// If true, then if a module has no versions that are compatible with - /// the current game version, then we will consider incompatible versions - /// of that module. - /// This replaces the former behavior of ignoring compatibility for - /// `install identifier=version` commands. - /// - public bool allow_incompatible = false; - - /// - /// If true, get the list of mods that should be checked for - /// recommendations and suggestions. - /// Differs from normal resolution in that it stops when - /// ModuleRelationshipDescriptor.suppress_recommendations==true - /// - public bool get_recommenders = false; - - public object Clone() => MemberwiseClone(); - } - - // TODO: RR currently conducts a depth-first resolution of requirements. While we do the - // right thing in processing all dependencies first, then recommends, and then suggests, - // we could find that a recommendation many layers deep prevents a recommendation in the - // original mod's recommends list. - // - // If we resolved in things breadth-first order, we're less likely to encounter surprises - // where a nth-deep recommend blocks a top-level recommend. - - // TODO: Add mechanism so that clients can add mods with relationshup other than UserAdded. - // Currently only made to support the with_{} options. - + using ModPair = KeyValuePair; /// - /// A class used to resolve relationships between mods. Primarily used to satisfy missing dependencies and to check for conflicts on proposed installs. + /// Resolves relationships between mods. Primarily used to satisfy missing dependencies and to check for conflicts on proposed installs. /// /// /// All constructors start with currently installed modules, to remove /// public class RelationshipResolver { - private static readonly ILog log = LogManager.GetLogger(typeof (RelationshipResolver)); /// - /// The list of all additional mods that need to be installed to satisfy all relationships. - /// - private readonly Dictionary modlist = new Dictionary(); - private readonly List user_requested_mods = new List(); - - //TODO As the conflict detection gets more advanced there is a greater need to have messages in here - // as recreating them from reasons is no longer possible. - private readonly List> conflicts = - new List>(); - private readonly Dictionary> reasons = - new Dictionary>(new NameComparer()); - - private readonly IRegistryQuerier registry; - private readonly GameVersionCriteria GameVersion; - private readonly RelationshipResolverOptions options; - /// - /// The list of already installed modules. + /// Creates a new resolver that will find a way to install all the modules specified. /// - private readonly HashSet installed_modules; - - private HashSet alreadyResolved = new HashSet(); - - private void AddReason(CkanModule module, SelectionReason reason) + /// Modules to install + /// Modules to remove + /// Options for the RelationshipResolver + /// CKAN registry object for current game instance + /// The current KSP version criteria to consider + public RelationshipResolver(IEnumerable modulesToInstall, + IEnumerable modulesToRemove, + RelationshipResolverOptions options, + IRegistryQuerier registry, + GameVersionCriteria versionCrit) + : this(options, registry, versionCrit) { - if (reasons.TryGetValue(module, out List modReasons)) + if (modulesToRemove != null) { - modReasons.Add(reason); + RemoveModsFromInstalledList(modulesToRemove); } - else + if (modulesToInstall != null) { - reasons.Add(module, new List() { reason }); + AddModulesToInstall(modulesToInstall.ToArray()); } } @@ -132,14 +50,18 @@ private void AddReason(CkanModule module, SelectionReason reason) /// /// Options for the RelationshipResolver /// CKAN registry object for current game instance - /// The current KSP version criteria to consider - public RelationshipResolver(RelationshipResolverOptions options, IRegistryQuerier registry, GameVersionCriteria GameVersion) + /// The current KSP version criteria to consider + private RelationshipResolver(RelationshipResolverOptions options, + IRegistryQuerier registry, + GameVersionCriteria versionCrit) { - this.registry = registry; - this.GameVersion = GameVersion; - this.options = options; + this.options = options; + this.registry = registry; + this.versionCrit = versionCrit; - installed_modules = new HashSet(registry.InstalledModules.Select(i_module => i_module.Module)); + installed_modules = registry.InstalledModules + .Select(i_module => i_module.Module) + .ToHashSet(); var installed_relationship = new SelectionReason.Installed(); foreach (var module in installed_modules) { @@ -154,11 +76,11 @@ public RelationshipResolver(RelationshipResolverOptions options, IRegistryQuerie /// Identifiers of modules to remove, will be converted to CkanModules using Registry.InstalledModule /// Options for the RelationshipResolver /// CKAN registry object for current game instance - /// The current KSP version criteria to consider + /// The current KSP version criteria to consider public RelationshipResolver(IEnumerable modulesToInstall, IEnumerable modulesToRemove, RelationshipResolverOptions options, IRegistryQuerier registry, - GameVersionCriteria GameVersion) : + GameVersionCriteria versionCrit) : this( - modulesToInstall?.Select(mod => TranslateModule(mod, options, registry, GameVersion)), + modulesToInstall?.Select(mod => TranslateModule(mod, options, registry, versionCrit)), modulesToRemove? .Select(mod => { @@ -167,7 +89,7 @@ public RelationshipResolver(IEnumerable modulesToInstall, IEnumerable registry.InstalledModule(identifier) != null) .Select(identifier => registry.InstalledModule(identifier).Module), - options, registry, GameVersion) + options, registry, versionCrit) { // Does nothing, just calls the other overloaded constructor } @@ -179,15 +101,15 @@ public RelationshipResolver(IEnumerable modulesToInstall, IEnumerableThe identifier or identifier=version of the module /// If options.allow_incompatible is set, fall back to searching incompatible modules if no compatible has been found /// CKAN registry object for current game instance - /// The current KSP version criteria to consider + /// The current KSP version criteria to consider /// A CkanModule - private static CkanModule TranslateModule(string name, RelationshipResolverOptions options, IRegistryQuerier registry, GameVersionCriteria GameVersion) + private static CkanModule TranslateModule(string name, RelationshipResolverOptions options, IRegistryQuerier registry, GameVersionCriteria versionCrit) { if (options.allow_incompatible) { try { - return CkanModule.FromIDandVersion(registry, name, GameVersion); + return CkanModule.FromIDandVersion(registry, name, versionCrit); } catch (ModuleNotFoundKraken) { @@ -198,60 +120,33 @@ private static CkanModule TranslateModule(string name, RelationshipResolverOptio } else { - return CkanModule.FromIDandVersion(registry, name, GameVersion); + return CkanModule.FromIDandVersion(registry, name, versionCrit); } } /// - /// Creates a new resolver that will find a way to install all the modules specified. + /// Returns the default options for relationship resolution. /// - /// Modules to install - /// Modules to remove - /// Options for the RelationshipResolver - /// CKAN registry object for current game instance - /// The current KSP version criteria to consider - public RelationshipResolver(IEnumerable modulesToInstall, IEnumerable modulesToRemove, RelationshipResolverOptions options, IRegistryQuerier registry, - GameVersionCriteria GameVersion) - : this(options, registry, GameVersion) - { - if (modulesToRemove != null) - { - RemoveModsFromInstalledList(modulesToRemove); - } - if (modulesToInstall != null) - { - AddModulesToInstall(modulesToInstall); - } - } - - /// - /// Returns the default options for relationship resolution. - /// - - // TODO: This should just be able to return a new RelationshipResolverOptions - // and the defaults in the class definition should do the right thing. public static RelationshipResolverOptions DefaultOpts() - { - return new RelationshipResolverOptions + // TODO: This should just be able to return a new RelationshipResolverOptions + // and the defaults in the class definition should do the right thing. + => new RelationshipResolverOptions { with_recommends = true, with_suggests = false, - with_all_suggests = false + with_all_suggests = false, }; - } /// /// Options to install without recommendations. /// public static RelationshipResolverOptions DependsOnlyOpts() - { - return new RelationshipResolverOptions + => new RelationshipResolverOptions { with_recommends = false, with_suggests = false, - with_all_suggests = false + with_all_suggests = false, }; - } /// /// Add modules to consideration of the relationship resolver. @@ -272,7 +167,9 @@ private void AddModulesToInstall(IEnumerable modules) log.DebugFormat("Preparing to resolve relationships for {0} {1}", module.identifier, module.version); // Need to check against installed mods and those to install. - var conflictingModules = modlist.Values.Concat(installed_modules).Where(listed_mod => listed_mod.ConflictsWith(module)); + var conflictingModules = modlist.Values + .Concat(installed_modules) + .Where(listed_mod => listed_mod.ConflictsWith(module)); foreach (CkanModule listed_mod in conflictingModules) { if (options.proceed_with_inconsistencies) @@ -303,12 +200,10 @@ private void AddModulesToInstall(IEnumerable modules) try { - // Finally, let's do a sanity check that our solution is actually sane. - SanityChecker.EnforceConsistency( - modlist.Values.Concat(installed_modules), - registry.InstalledDlls, - registry.InstalledDlc - ); + // Check that our solution is actually sane + SanityChecker.EnforceConsistency(modlist.Values.Concat(installed_modules), + registry.InstalledDlls, + registry.InstalledDlc); } catch (BadRelationshipsKraken k) { @@ -345,7 +240,9 @@ private void RemoveModsFromInstalledList(IEnumerable mods) /// Resolves all relationships for a module. /// May recurse to ResolveStanza, which may add additional modules to be installed. /// - private void Resolve(CkanModule module, RelationshipResolverOptions options, IEnumerable old_stanza = null) + private void Resolve(CkanModule module, + RelationshipResolverOptions options, + IEnumerable old_stanza = null) { if (alreadyResolved.Contains(module)) { @@ -358,7 +255,7 @@ private void Resolve(CkanModule module, RelationshipResolverOptions options, IEn } // Even though we may resolve top-level suggests for our module, - // we don't install suggestions all the down unless with_all_suggests + // we don't install suggestions all the way down unless with_all_suggests // is true. var sub_options = (RelationshipResolverOptions) options.Clone(); sub_options.with_suggests = false; @@ -368,6 +265,14 @@ private void Resolve(CkanModule module, RelationshipResolverOptions options, IEn log.DebugFormat("Resolving dependencies for {0}", module.identifier); ResolveStanza(module.depends, new SelectionReason.Depends(module), sub_options, false, old_stanza); + // TODO: RR currently conducts a depth-first resolution of requirements. While we do the + // right thing in processing all dependencies first, then recommends, and then suggests, + // we could find that a recommendation many layers deep prevents a recommendation in the + // original mod's recommends list. + // + // If we resolved in things breadth-first order, we're less likely to encounter surprises + // where a nth-deep recommend blocks a top-level recommend. + if (options.with_recommends) { log.DebugFormat("Resolving recommends for {0}", module.identifier); @@ -431,8 +336,8 @@ private void ResolveStanza(IEnumerable stanza, Selection } else if (descriptor.ContainsAny(modlist.Keys)) { - CkanModule module = modlist.Values - .FirstOrDefault(m => descriptor.ContainsAny(new string[] { m.identifier })); + // Two installing mods depend on different versions of this dependency + CkanModule module = modlist.Values.FirstOrDefault(m => descriptor.ContainsAny(new string[] { m.identifier })); if (options.proceed_with_inconsistencies) { conflicts.Add(new KeyValuePair(module, reason.Parent)); @@ -448,17 +353,16 @@ private void ResolveStanza(IEnumerable stanza, Selection } // If it's already installed, skip. - if (descriptor.MatchesAny( - installed_modules, - registry.InstalledDlls.ToHashSet(), - registry.InstalledDlc)) + if (descriptor.MatchesAny(installed_modules, + registry.InstalledDlls.ToHashSet(), + registry.InstalledDlc)) { continue; } else if (descriptor.ContainsAny(installed_modules.Select(im => im.identifier))) { - CkanModule module = installed_modules - .FirstOrDefault(m => descriptor.ContainsAny(new string[] { m.identifier })); + // We need a different version of the mod than is already installed + CkanModule module = installed_modules.FirstOrDefault(m => descriptor.ContainsAny(new string[] { m.identifier })); if (options.proceed_with_inconsistencies) { conflicts.Add(new KeyValuePair(module, reason.Parent)); @@ -476,17 +380,17 @@ private void ResolveStanza(IEnumerable 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 candidates = descriptor - .LatestAvailableWithProvides(registry, GameVersion, installed_modules, modlist.Values) + .LatestAvailableWithProvides(registry, versionCrit, installed_modules, modlist.Values) .Where(mod => !modlist.ContainsKey(mod.identifier) - && descriptor1.WithinBounds(mod) - && MightBeInstallable(mod, reason.Parent, installed_modules)) + && descriptor1.WithinBounds(mod) + && MightBeInstallable(mod, reason.Parent, installed_modules)) .ToList(); if (!candidates.Any()) { // 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, new CkanModule[0]) + .LatestAvailableWithProvides(registry, versionCrit, new CkanModule[0]) .Where(mod => !modlist.ContainsKey(mod.identifier) && descriptor1.WithinBounds(mod) && MightBeInstallable(mod, null, new CkanModule[0])) @@ -506,7 +410,6 @@ private void ResolveStanza(IEnumerable 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. if (options.without_toomanyprovides_kraken) { continue; @@ -519,7 +422,7 @@ private void ResolveStanza(IEnumerable stanza, Selection List provide = candidates .Where(cand => old_stanza.Any(rel => rel.WithinBounds(cand))) .ToList(); - if (!provide.Any() || provide.Count() > 1) + if (provide.Count != 1) { //We still have either nothing, or too many to pick from //Just throw the TMP now @@ -538,11 +441,9 @@ private void ResolveStanza(IEnumerable stanza, Selection // Finally, check our candidate against everything which might object // to it being installed; that's all the mods which are fixed in our // list thus far, as well as everything on the system. + var fixed_mods = modlist.Values.Concat(installed_modules).ToHashSet(); - var fixed_mods = new HashSet(modlist.Values); - fixed_mods.UnionWith(installed_modules); - - CkanModule conflicting_mod = fixed_mods.FirstOrDefault(mod => mod.ConflictsWith(candidate)); + var conflicting_mod = fixed_mods.FirstOrDefault(mod => mod.ConflictsWith(candidate)); if (conflicting_mod == null) { // Okay, looks like we want this one. Adding. @@ -558,8 +459,8 @@ private void ResolveStanza(IEnumerable stanza, Selection if (options.proceed_with_inconsistencies) { Add(candidate, reason); - conflicts.Add(new KeyValuePair(conflicting_mod, candidate)); - conflicts.Add(new KeyValuePair(candidate, conflicting_mod)); + conflicts.Add(new ModPair(conflicting_mod, candidate)); + conflicts.Add(new ModPair(candidate, conflicting_mod)); } else { @@ -649,7 +550,8 @@ private bool MightBeInstallable(CkanModule module, CkanModule stanzaSource = nul { return true; } - //When checking the dependencies we assume that this module is installable + + // When checking the dependencies we assume that this module is installable // in case a dependent depends on it compatible.Add(module.identifier); @@ -660,7 +562,7 @@ private bool MightBeInstallable(CkanModule module, CkanModule stanzaSource = nul // Skip dependencies satisfied by installed modules .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(); + .Select(depend => depend.LatestAvailableWithProvides(registry, versionCrit, installed, toInstall)).ToList(); log.DebugFormat("Trying to satisfy: {0}", string.Join("; ", needed.Select(need => @@ -738,7 +640,7 @@ public IEnumerable ModList() } /// - /// Returns a dictionary consisting of keyValuePairs containing conflicting mods. + /// Returns a dictionary consisting of KeyValuePairs containing conflicting mods. /// public Dictionary ConflictList { @@ -762,10 +664,7 @@ public Dictionary ConflictList } } - public bool IsConsistent - { - get { return !conflicts.Any(); } - } + public bool IsConsistent => !conflicts.Any(); public List ReasonsFor(CkanModule mod) { @@ -790,10 +689,107 @@ public List ReasonsFor(CkanModule mod) /// true if auto-installed, false otherwise /// public bool IsAutoInstalled(CkanModule mod) + => ReasonsFor(mod).All(reason => reason is SelectionReason.Depends + && !reason.Parent.IsMetapackage); + + private void AddReason(CkanModule module, SelectionReason reason) { - return ReasonsFor(mod).All(reason => - reason is SelectionReason.Depends && !reason.Parent.IsMetapackage); + if (reasons.TryGetValue(module, out List modReasons)) + { + modReasons.Add(reason); + } + else + { + reasons.Add(module, new List() { reason }); + } } + + /// + /// The list of all additional mods that need to be installed to satisfy all relationships. + /// + private readonly Dictionary modlist = new Dictionary(); + private readonly List user_requested_mods = new List(); + + //TODO As the conflict detection gets more advanced there is a greater need to have messages in here + // as recreating them from reasons is no longer possible. + private readonly List conflicts = new List(); + private readonly Dictionary> reasons = + new Dictionary>(new NameComparer()); + + private readonly IRegistryQuerier registry; + private readonly GameVersionCriteria versionCrit; + private readonly RelationshipResolverOptions options; + + /// + /// The list of already installed modules. + /// + private readonly HashSet installed_modules; + + private HashSet alreadyResolved = new HashSet(); + + private static readonly ILog log = LogManager.GetLogger(typeof(RelationshipResolver)); + } + + // TODO: It would be lovely to get rid of the `without` fields, + // and replace them with `with` fields. Humans suck at inverting + // cases in their heads. + public class RelationshipResolverOptions : ICloneable + { + /// + /// If true, add recommended mods, and their recommendations. + /// + public bool with_recommends = true; + + /// + /// If true, add suggests, but not suggested suggests. :) + /// + public bool with_suggests = false; + + /// + /// If true, add suggested modules, and *their* suggested modules, too! + /// + public bool with_all_suggests = false; + + /// + /// If true, surpresses the TooManyProvides kraken when resolving + /// relationships. Otherwise, we just pick the first. + /// + public bool without_toomanyprovides_kraken = false; + + /// + /// If true, we skip our sanity check at the end of our relationship + /// resolution. Note that non-sane resolutions can't actually be + /// installed, so this is mostly useful for giving the user feedback + /// on failed resolutions. + /// + public bool without_enforce_consistency = false; + + /// + /// If true, we'll populate the `conflicts` field, rather than immediately + /// throwing a kraken when inconsistencies are detected. Again, these + /// solutions are non-installable, so mostly of use to provide user + /// feedback when things go wrong. + /// + public bool proceed_with_inconsistencies = false; + + /// + /// If true, then if a module has no versions that are compatible with + /// the current game version, then we will consider incompatible versions + /// of that module. + /// This replaces the former behavior of ignoring compatibility for + /// `install identifier=version` commands. + /// + public bool allow_incompatible = false; + + /// + /// If true, get the list of mods that should be checked for + /// recommendations and suggestions. + /// Differs from normal resolution in that it stops when + /// ModuleRelationshipDescriptor.suppress_recommendations==true + /// + public bool get_recommenders = false; + + public object Clone() => MemberwiseClone(); } /// diff --git a/Core/Relationships/SanityChecker.cs b/Core/Relationships/SanityChecker.cs index 53b12609ee..1611866bf4 100644 --- a/Core/Relationships/SanityChecker.cs +++ b/Core/Relationships/SanityChecker.cs @@ -1,8 +1,6 @@ using System.Collections.Generic; using System.Linq; -using log4net; - using CKAN.Extensions; using CKAN.Versioning; @@ -13,47 +11,14 @@ namespace CKAN /// public static class SanityChecker { - private static readonly ILog log = LogManager.GetLogger(typeof(SanityChecker)); - - /// - /// Checks the list of modules for consistency errors, returning a list of - /// errors found. The list will be empty if everything is fine. - /// - public static ICollection ConsistencyErrors( - IEnumerable modules, - IEnumerable dlls, - IDictionary dlc) - { - List> unmetDepends; - List> conflicts; - var errors = new HashSet(); - if (!CheckConsistency(modules, dlls, dlc, out unmetDepends, out conflicts)) - { - foreach (var kvp in unmetDepends) - { - errors.Add(string.Format( - Properties.Resources.SanityCheckerUnsatisfiedDependency, - kvp.Key, kvp.Value)); - } - foreach (var kvp in conflicts) - { - errors.Add(string.Format( - Properties.Resources.SanityCheckerConflictsWith, - kvp.Key, kvp.Value)); - } - } - return errors; - } - /// /// Ensures all modules in the list provided can co-exist. /// Throws a BadRelationshipsKraken describing the problems otherwise. /// Does nothing if the modules can happily co-exist. /// - public static void EnforceConsistency( - IEnumerable modules, - IEnumerable dlls = null, - IDictionary dlc = null) + public static void EnforceConsistency(IEnumerable modules, + IEnumerable dlls = null, + IDictionary dlc = null) { List> unmetDepends; List> conflicts; @@ -65,16 +30,13 @@ public static void EnforceConsistency( /// /// 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, - IEnumerable dlls = null, - IDictionary dlc = null) - { - List> unmetDepends; - List> conflicts; - return CheckConsistency(modules, dlls, dlc, out unmetDepends, out conflicts); - } + public static bool IsConsistent(IEnumerable modules, + IEnumerable dlls = null, + IDictionary dlc = null) + => CheckConsistency(modules, dlls, dlc, + out var _, out var _); private static bool CheckConsistency( IEnumerable modules, @@ -147,24 +109,6 @@ public static List> FindConflic return confl; } - private sealed class ProvidesInfo - { - public string ProviderIdentifier { get; } - public ModuleVersion ProviderVersion { get; } - public string ProvideeIdentifier { get; } - public ModuleVersion ProvideeVersion { get; } - public ProvidesInfo( - string providerIdentifier, - ModuleVersion providerVersion, - string provideeIdentifier, - ModuleVersion provideeVersion) - { - ProviderIdentifier = providerIdentifier; - ProviderVersion = providerVersion; - ProvideeIdentifier = provideeIdentifier; - ProvideeVersion = provideeVersion; - } - } } } diff --git a/Core/Repositories/RepositoryData.cs b/Core/Repositories/RepositoryData.cs index 23af21e8bc..43ac5d7881 100644 --- a/Core/Repositories/RepositoryData.cs +++ b/Core/Repositories/RepositoryData.cs @@ -134,9 +134,10 @@ public static RepositoryData FromDownload(string path, IGame game, IProgress progress) { - using (var inputStream = File.OpenRead(path)) - using (var gzipStream = new GZipInputStream(inputStream)) - using (var tarStream = new TarInputStream(gzipStream, Encoding.UTF8)) + using (var inputStream = File.OpenRead(path)) + using (var progressStream = new ReadProgressStream(inputStream, progress)) + using (var gzipStream = new GZipInputStream(progressStream)) + using (var tarStream = new TarInputStream(gzipStream, Encoding.UTF8)) { var modules = new List(); SortedDictionary counts = null; @@ -179,7 +180,9 @@ private static string tarStreamString(TarInputStream stream, TarEntry entry) private static RepositoryData FromZip(string path, IGame game, IProgress progress) { - using (var zipfile = new ZipFile(path)) + using (var inputStream = File.OpenRead(path)) + using (var progressStream = new ReadProgressStream(inputStream, progress)) + using (var zipfile = new ZipFile(progressStream)) { var modules = new List(); SortedDictionary counts = null; diff --git a/Core/Types/CkanModule.cs b/Core/Types/CkanModule.cs index 8b5ea67024..9136e7dd26 100644 --- a/Core/Types/CkanModule.cs +++ b/Core/Types/CkanModule.cs @@ -574,12 +574,6 @@ private GameVersion LatestCompatibleRealGameVersion(GameVersionRange range, => (realVers?.LastOrDefault(v => range.Contains(v)) ?? LatestCompatibleGameVersion()); - /// - /// Returns true if this module provides the functionality requested. - /// - public bool DoesProvide(string identifier) - => this.identifier == identifier || provides.Contains(identifier); - public bool IsMetapackage => kind == "metapackage"; public bool IsDLC => kind == "dlc"; diff --git a/Core/Types/Kraken.cs b/Core/Types/Kraken.cs index ea89a193cd..5ce1e7bc61 100644 --- a/Core/Types/Kraken.cs +++ b/Core/Types/Kraken.cs @@ -12,7 +12,8 @@ namespace CKAN /// public class Kraken : Exception { - public Kraken(string reason = null, Exception innerException = null) : base(reason, innerException) + public Kraken(string reason = null, Exception innerException = null) + : base(reason, innerException) { } } @@ -63,11 +64,8 @@ public NotEnoughSpaceKraken(string description, DirectoryInfo destination, long /// public class BadInstallLocationKraken : Kraken { - // Okay C#, you really need a keyword in your class declaration that says we call our - // parent constructors by default. This sort of thing is unacceptable in a modern - // programming langauge. - - public BadInstallLocationKraken(string reason = null, Exception innerException = null) : base(reason, innerException) + public BadInstallLocationKraken(string reason = null, Exception innerException = null) + : base(reason, innerException) { } } @@ -77,11 +75,10 @@ public class ModuleNotFoundKraken : Kraken public readonly string module; public readonly string version; - // TODO: Is there a way to set the stringify version of this? public ModuleNotFoundKraken(string module, string version, string reason, Exception innerException = null) - : base( - reason ?? string.Format(Properties.Resources.KrakenDependencyNotSatisfied, module, version), - innerException) + : base(reason + ?? string.Format(Properties.Resources.KrakenDependencyNotSatisfied, module, version), + innerException) { this.module = module; this.version = version; @@ -112,11 +109,17 @@ public class DependencyNotSatisfiedKraken : ModuleNotFoundKraken /// Message parameter for base class /// Originating exception parameter for base class public DependencyNotSatisfiedKraken(CkanModule parentModule, - string module, string version = null, string reason = null, Exception innerException = null) + string module, + string version = null, + string reason = null, + Exception innerException = null) : base(module, version, - reason ?? string.Format(Properties.Resources.KrakenParentDependencyNotSatisfied, - parentModule.identifier, module, version ?? Properties.Resources.KrakenAny), - innerException) + reason ?? string.Format( + Properties.Resources.KrakenParentDependencyNotSatisfied, + parentModule.identifier, + module, + version ?? Properties.Resources.KrakenAny), + innerException) { parent = parentModule; } @@ -176,21 +179,18 @@ public class TooManyModsProvideKraken : Kraken public readonly string requested; public readonly string choice_help_text; - public TooManyModsProvideKraken(string requested, List modules, string choice_help_text = null, Exception innerException = null) - : base(FormatMessage(requested, modules, choice_help_text), innerException) + public TooManyModsProvideKraken(string requested, + List modules, + string choice_help_text = null, + Exception innerException = null) + : base(choice_help_text ?? string.Format(Properties.Resources.KrakenProvidedByMoreThanOne, + requested), + innerException) { this.requested = requested; this.modules = modules; this.choice_help_text = choice_help_text; } - - private static string FormatMessage(string requested, List modules, string choice_help_text = null) - { - return choice_help_text - ?? string.Format( - Properties.Resources.KrakenProvidedByMoreThanOne, - requested); - } } /// @@ -199,8 +199,6 @@ private static string FormatMessage(string requested, List modules, /// public class InconsistentKraken : Kraken { - public readonly ICollection inconsistencies; - public string InconsistenciesPretty { get @@ -211,14 +209,6 @@ public string InconsistenciesPretty } } - public string ShortDescription - { - get - { - return String.Join("; ", inconsistencies); - } - } - public InconsistentKraken(ICollection inconsistencies, Exception innerException = null) : base(null, innerException) { @@ -231,10 +221,15 @@ public InconsistentKraken(string inconsistency, Exception innerException = null) inconsistencies = new List { inconsistency }; } + public string ShortDescription + => string.Join("; ", inconsistencies); + public override string ToString() { return InconsistenciesPretty + Environment.NewLine + Environment.NewLine + StackTrace; } + + private readonly ICollection inconsistencies; } /// @@ -295,9 +290,9 @@ public readonly List> Exceptions = new List>(); public DownloadErrorsKraken(List> errors) - : base(String.Join(Environment.NewLine, - new string[] { Properties.Resources.KrakenDownloadErrorsHeader, "" } - .Concat(errors.Select(e => e.Value.Message)))) + : base(string.Join(Environment.NewLine, + new string[] { Properties.Resources.KrakenDownloadErrorsHeader, "" } + .Concat(errors.Select(e => e.Value.Message)))) { Exceptions = new List>(errors); } diff --git a/Core/Types/RelationshipDescriptor.cs b/Core/Types/RelationshipDescriptor.cs index c498005074..320e64d528 100644 --- a/Core/Types/RelationshipDescriptor.cs +++ b/Core/Types/RelationshipDescriptor.cs @@ -83,30 +83,12 @@ public override bool WithinBounds(CkanModule otherModule) /// /// True if other_version is within the bounds public bool WithinBounds(ModuleVersion other) - { // UnmanagedModuleVersions with unknown versions always satisfy the bound - if (other is UnmanagedModuleVersion unmanagedModuleVersion && unmanagedModuleVersion.IsUnknownVersion) - return true; - - if (version == null) - { - if (max_version == null && min_version == null) - return true; - - var minSat = min_version == null || min_version <= other; - var maxSat = max_version == null || max_version >= other; - - if (minSat && maxSat) - return true; - } - else - { - if (version.Equals(other)) - return true; - } - - return false; - } + => (other is UnmanagedModuleVersion unmanagedModuleVersion + && unmanagedModuleVersion.IsUnknownVersion) + || (version?.Equals(other) + ?? ((min_version == null || min_version <= other) + && (max_version == null || max_version >= other))); /// /// Check whether any of the modules in a given list match this descriptor. @@ -189,14 +171,14 @@ public override CkanModule ExactMatch( .FirstOrDefault(mod => mod.identifier == name); public override bool Equals(RelationshipDescriptor other) - { - ModuleRelationshipDescriptor modRel = other as ModuleRelationshipDescriptor; - return modRel != null - && name == modRel.name - && version == modRel.version - && min_version == modRel.min_version - && max_version == modRel.max_version; - } + => Equals(other as ModuleRelationshipDescriptor); + + protected bool Equals(ModuleRelationshipDescriptor other) + => other != null + && name == other.name + && version == other.version + && min_version == other.min_version + && max_version == other.max_version; public override bool ContainsAny(IEnumerable identifiers) => identifiers.Contains(name); @@ -237,7 +219,7 @@ public class AnyOfRelationshipDescriptor : RelationshipDescriptor "name", "version", "min_version", - "max_version" + "max_version", }; public override bool WithinBounds(CkanModule otherModule) @@ -276,11 +258,11 @@ public override CkanModule ExactMatch( => null; public override bool Equals(RelationshipDescriptor other) - { - AnyOfRelationshipDescriptor anyRel = other as AnyOfRelationshipDescriptor; - return anyRel != null - && (any_of?.SequenceEqual(anyRel.any_of) ?? anyRel.any_of == null); - } + => Equals(other as AnyOfRelationshipDescriptor); + + protected bool Equals(AnyOfRelationshipDescriptor other) + => other != null + && (any_of?.SequenceEqual(other.any_of) ?? other.any_of == null); public override bool ContainsAny(IEnumerable identifiers) => any_of?.Any(r => r.ContainsAny(identifiers)) ?? false; diff --git a/GUI/Controls/ChooseRecommendedMods.cs b/GUI/Controls/ChooseRecommendedMods.cs index f8ad6575f2..cafafb25b3 100644 --- a/GUI/Controls/ChooseRecommendedMods.cs +++ b/GUI/Controls/ChooseRecommendedMods.cs @@ -133,8 +133,7 @@ private void MarkConflicts() }; private Dictionary FindConflicts() - { - return new RelationshipResolver( + => new RelationshipResolver( RecommendedModsListView.CheckedItems.Cast() .Select(item => item.Tag as CkanModule) .Concat(toInstall) @@ -142,14 +141,12 @@ private Dictionary FindConflicts() toUninstall, conflictOptions, registry, GameVersion ).ConflictList; - } private IEnumerable getRecSugRows( NetModuleCache cache, Dictionary>> recommendations, Dictionary> suggestions, - Dictionary> supporters - ) + Dictionary> supporters) { foreach (var kvp in recommendations) { diff --git a/GUI/Controls/EditModpack.cs b/GUI/Controls/EditModpack.cs index 232e9f3c82..8f508c6303 100644 --- a/GUI/Controls/EditModpack.cs +++ b/GUI/Controls/EditModpack.cs @@ -360,9 +360,7 @@ private CkanModule ApplyVersionsCheckbox(CkanModule input) { if (rels != null) { - foreach (var rel in rels - .Select(rel => rel as ModuleRelationshipDescriptor) - .Where(rel => rel != null)) + foreach (var rel in rels.OfType()) { rel.version = null; rel.min_version = null; diff --git a/GUI/Controls/InstallationHistory.cs b/GUI/Controls/InstallationHistory.cs index a58fd4f5a8..ef444344e9 100644 --- a/GUI/Controls/InstallationHistory.cs +++ b/GUI/Controls/InstallationHistory.cs @@ -88,8 +88,7 @@ private void HistoryListView_ItemSelectionChanged(Object sender, ListViewItemSel .First(); var modRows = CkanModule.FromFile(path.FullName) .depends - .Select(rel => rel as ModuleRelationshipDescriptor) - .Where(rel => rel != null) + .OfType() .Select(ItemFromRelationship) .Where(row => row != null) .ToArray(); @@ -213,8 +212,8 @@ private void InstallButton_Click(object sender, EventArgs e) Install?.Invoke(ModsListView.Items .Cast() .Where(lvi => lvi.Group == NotInstalledGroup) - .Select(lvi => lvi.Tag as CkanModule) - .Where(mod => mod != null) + .Select(lvi => lvi.Tag) + .OfType() .ToArray()); } diff --git a/GUI/Controls/ManageMods.cs b/GUI/Controls/ManageMods.cs index 1005b85a50..c3da94eeee 100644 --- a/GUI/Controls/ManageMods.cs +++ b/GUI/Controls/ManageMods.cs @@ -1220,7 +1220,7 @@ private bool _UpdateModsList(Dictionary old_modules = null) // Copy the new mod flag from the old list. var old_new_mods = new HashSet( mainModList.Modules.Where(m => m.IsNew)); - foreach (var gui_mod in gui_mods.Where(m => old_new_mods.Contains(m))) + foreach (var gui_mod in gui_mods.Intersect(old_new_mods)) { gui_mod.IsNew = true; } diff --git a/GUI/Controls/ModInfoTabs/Relationships.cs b/GUI/Controls/ModInfoTabs/Relationships.cs index e37851cd0e..8549ffbf05 100644 --- a/GUI/Controls/ModInfoTabs/Relationships.cs +++ b/GUI/Controls/ModInfoTabs/Relationships.cs @@ -296,9 +296,10 @@ private TreeNode findDependencyShallow(IRegistryQuerier registry, RelationshipDe else { // Several found or not same id, return a "provides" node - return providesNode(relDescr.ToString(), relationship, - dependencyModules.Select(dep => indexedNode(registry, dep, relationship, relDescr, crit)) - ); + return providesNode(relDescr.ToString(), + relationship, + dependencyModules.Select(dep => indexedNode( + registry, dep, relationship, relDescr, crit))); } } diff --git a/GUI/Controls/ModInfoTabs/Versions.cs b/GUI/Controls/ModInfoTabs/Versions.cs index 8f025dafe9..a6f7bda121 100644 --- a/GUI/Controls/ModInfoTabs/Versions.cs +++ b/GUI/Controls/ModInfoTabs/Versions.cs @@ -91,13 +91,10 @@ private void VersionsListView_ItemCheck(object sender, ItemCheckEventArgs e) [ForbidGUICalls] private bool installable(ModuleInstaller installer, CkanModule module, IRegistryQuerier registry) - { - return module.IsCompatible(Main.Instance.CurrentInstance.VersionCriteria()) - && installer.CanInstall( - RelationshipResolver.DependsOnlyOpts(), - new List() { module }, - registry); - } + => module.IsCompatible(Main.Instance.CurrentInstance.VersionCriteria()) + && installer.CanInstall(RelationshipResolver.DependsOnlyOpts(), + new List() { module }, + registry); private bool allowInstall(CkanModule module) { diff --git a/Netkan/ConsoleUser.cs b/Netkan/ConsoleUser.cs index 30e3961a12..18588b3518 100644 --- a/Netkan/ConsoleUser.cs +++ b/Netkan/ConsoleUser.cs @@ -3,7 +3,7 @@ using System.Text.RegularExpressions; using log4net; -namespace CKAN +namespace CKAN.NetKAN { /// /// The commandline implementation of the IUser interface. diff --git a/Netkan/Transformers/SpaceWarpInfoTransformer.cs b/Netkan/Transformers/SpaceWarpInfoTransformer.cs index badd565e6c..2ad9b9552c 100644 --- a/Netkan/Transformers/SpaceWarpInfoTransformer.cs +++ b/Netkan/Transformers/SpaceWarpInfoTransformer.cs @@ -84,8 +84,8 @@ public IEnumerable Transform(Metadata metadata, TransformOptions opts) log.InfoFormat("Found compatibility: {0}–{1}", minVer, maxVer); ModuleService.ApplyVersions(json, null, minVer, maxVer); } - var moduleDeps = (mod.depends?.Select(r => (r as ModuleRelationshipDescriptor)?.name) - .Where(ident => ident != null) + var moduleDeps = (mod.depends?.OfType() + .Select(r => r.name) ?? Enumerable.Empty()) .ToHashSet(); var missingDeps = swinfo.dependencies diff --git a/Tests/CmdLine/ResourcesTests.cs b/Tests/CmdLine/ResourcesTests.cs index 83c6dad3dd..d1671c65b8 100644 --- a/Tests/CmdLine/ResourcesTests.cs +++ b/Tests/CmdLine/ResourcesTests.cs @@ -2,6 +2,7 @@ using System.Linq; using System.Resources; using System.Globalization; + using NUnit.Framework; namespace Tests.CmdLine diff --git a/Tests/Core/Net/NetAsyncDownloaderTests.cs b/Tests/Core/Net/NetAsyncDownloaderTests.cs index cce197ae16..9e1f8795b3 100644 --- a/Tests/Core/Net/NetAsyncDownloaderTests.cs +++ b/Tests/Core/Net/NetAsyncDownloaderTests.cs @@ -201,16 +201,19 @@ public void DownloadAndWait_WithSomeInvalidUrls_ThrowsDownloadErrorsKraken( { downloader.DownloadAndWait(targets); }); - CollectionAssert.AreEquivalent(badIndices, exception.Exceptions.Select(kvp => kvp.Key).ToArray()); - foreach (var kvp in exception.Exceptions) + Assert.Multiple(() => { - var baseExc = kvp.Value.GetBaseException() as FileNotFoundException; - Assert.AreEqual(fromPaths[kvp.Key], baseExc.FileName); - } - foreach (var t in validTargets) - { - Assert.IsTrue(File.Exists(t.filename)); - } + CollectionAssert.AreEquivalent(badIndices, exception.Exceptions.Select(kvp => kvp.Key).ToArray()); + foreach (var kvp in exception.Exceptions) + { + var baseExc = kvp.Value.GetBaseException() as FileNotFoundException; + Assert.AreEqual(fromPaths[kvp.Key], baseExc.FileName); + } + foreach (var t in validTargets) + { + Assert.IsTrue(File.Exists(t.filename)); + } + }); } } } diff --git a/Tests/Core/Relationships/RelationshipResolver.cs b/Tests/Core/Relationships/RelationshipResolver.cs index 408a46cffd..19f7c5ac86 100644 --- a/Tests/Core/Relationships/RelationshipResolver.cs +++ b/Tests/Core/Relationships/RelationshipResolver.cs @@ -996,7 +996,7 @@ public void AutodetectedCanSatisfyRelationships() new RelationshipResolver( new CkanModule[] { mod }, null, RelationshipResolver.DefaultOpts(), - registry, new GameVersionCriteria (GameVersion.Parse("1.0.0"))); + registry, new GameVersionCriteria(GameVersion.Parse("1.0.0"))); } } diff --git a/Tests/Data/TemporaryRepository.cs b/Tests/Data/TemporaryRepository.cs index 9f94d68fda..5264f0453e 100644 --- a/Tests/Data/TemporaryRepository.cs +++ b/Tests/Data/TemporaryRepository.cs @@ -11,7 +11,7 @@ namespace Tests.Data { /// - /// A disposable repository backed by an auto-created ZIP file + /// A disposable repository backed by an auto-created tar.gz file /// containing the given modules. /// Will be automatically cleaned up on falling out of using() scope. /// From 1e768f2edb801816bf9bce979e21a35d95021e00 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 15:39:04 -0500 Subject: [PATCH 02/22] Move identifier=version syntax from Core to Cmdline --- Cmdline/Action/Import.cs | 4 +- Cmdline/Action/Install.cs | 10 +- Cmdline/Action/Upgrade.cs | 11 +- ConsoleUI/InstallScreen.cs | 7 +- Core/ModuleInstaller.cs | 33 ------ Core/Net/NetModuleCache.cs | 2 +- Core/Relationships/RelationshipResolver.cs | 55 --------- Tests/CapturingUser.cs | 58 ++++++++++ Tests/CmdLine/InstallTests.cs | 101 +++++++++++++++++ Tests/CmdLine/UpgradeTests.cs | 104 ++++++++++++++++++ Tests/Core/ModuleInstallerDirTest.cs | 2 +- Tests/Core/ModuleInstallerTests.cs | 64 +++-------- .../Relationships/RelationshipResolver.cs | 75 ++++++------- 13 files changed, 336 insertions(+), 190 deletions(-) create mode 100644 Tests/CapturingUser.cs create mode 100644 Tests/CmdLine/InstallTests.cs create mode 100644 Tests/CmdLine/UpgradeTests.cs diff --git a/Cmdline/Action/Import.cs b/Cmdline/Action/Import.cs index 5abfed3e46..7d23af6b44 100644 --- a/Cmdline/Action/Import.cs +++ b/Cmdline/Action/Import.cs @@ -45,10 +45,10 @@ public int RunCommand(CKAN.GameInstance instance, object options) else { log.InfoFormat("Importing {0} files", toImport.Count); - var toInstall = new List(); + var toInstall = new List(); var installer = new ModuleInstaller(instance, manager.Cache, user); var regMgr = RegistryManager.Instance(instance, repoData); - installer.ImportFiles(toImport, user, mod => toInstall.Add(mod.identifier), regMgr.registry, !opts.Headless); + installer.ImportFiles(toImport, user, mod => toInstall.Add(mod), regMgr.registry, !opts.Headless); HashSet possibleConfigOnlyDirs = null; if (toInstall.Count > 0) { diff --git a/Cmdline/Action/Install.cs b/Cmdline/Action/Install.cs index c62af424fb..9d2a0701f8 100644 --- a/Cmdline/Action/Install.cs +++ b/Cmdline/Action/Install.cs @@ -136,7 +136,15 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options) { HashSet possibleConfigOnlyDirs = null; var installer = new ModuleInstaller(instance, manager.Cache, user); - installer.InstallList(modules, install_ops, regMgr, ref possibleConfigOnlyDirs); + installer.InstallList(modules.Select(arg => CkanModule.FromIDandVersion( + regMgr.registry, arg, + options.allow_incompatible + ? null + : instance.VersionCriteria())) + .ToList(), + install_ops, + regMgr, + ref possibleConfigOnlyDirs); user.RaiseMessage(""); done = true; } diff --git a/Cmdline/Action/Upgrade.cs b/Cmdline/Action/Upgrade.cs index a23fa20aa5..bb1d91afd2 100644 --- a/Cmdline/Action/Upgrade.cs +++ b/Cmdline/Action/Upgrade.cs @@ -187,8 +187,15 @@ private void UpgradeModules(GameInstanceManager manager, IUser user, CKAN.GameIn { UpgradeModules(manager, user, instance, repoData, (ModuleInstaller installer, NetAsyncModulesDownloader downloader, RegistryManager regMgr, ref HashSet possibleConfigOnlyDirs) => - installer.Upgrade(identsAndVersions, downloader, - ref possibleConfigOnlyDirs, regMgr, true), + installer.Upgrade( + identsAndVersions.Select(arg => CkanModule.FromIDandVersion( + regMgr.registry, arg, + instance.VersionCriteria())) + .ToList(), + downloader, + ref possibleConfigOnlyDirs, + regMgr, + true), m => identsAndVersions.Add(m.identifier) ); } diff --git a/ConsoleUI/InstallScreen.cs b/ConsoleUI/InstallScreen.cs index e5ad54a2ae..d1f5509040 100644 --- a/ConsoleUI/InstallScreen.cs +++ b/ConsoleUI/InstallScreen.cs @@ -1,6 +1,7 @@ using System; using System.Transactions; using System.Collections.Generic; +using System.Linq; using CKAN.ConsoleUI.Toolkit; @@ -76,7 +77,11 @@ public override void Run(ConsoleTheme theme, Action process = null plan.Install.Clear(); } if (plan.Upgrade.Count > 0) { - inst.Upgrade(plan.Upgrade, dl, ref possibleConfigOnlyDirs, regMgr); + inst.Upgrade(plan.Upgrade + .Select(ident => regMgr.registry.LatestAvailable( + ident, manager.CurrentInstance.VersionCriteria())) + .ToList(), + dl, ref possibleConfigOnlyDirs, regMgr); plan.Upgrade.Clear(); } if (plan.Replace.Count > 0) { diff --git a/Core/ModuleInstaller.cs b/Core/ModuleInstaller.cs index 8ec222e1f0..7035148e14 100644 --- a/Core/ModuleInstaller.cs +++ b/Core/ModuleInstaller.cs @@ -112,19 +112,6 @@ public static string CachedOrDownload(CkanModule module, NetModuleCache cache, s return full_path; } - public void InstallList(List modules, RelationshipResolverOptions options, RegistryManager registry_manager, ref HashSet possibleConfigOnlyDirs, IDownloader downloader = null) - { - var resolver = new RelationshipResolver(modules, null, options, registry_manager.registry, ksp.VersionCriteria()); - // Only pass the CkanModules of the parameters, so we can tell which are auto-installed, - // and relationships of metapackages, since metapackages aren't included in the RR modlist. - var list = resolver.ModList() - .Where(m => resolver.ReasonsFor(m).Any(reason => - reason is SelectionReason.UserRequested - || (reason.Parent?.IsMetapackage ?? false))) - .ToList(); - InstallList(list, options, registry_manager, ref possibleConfigOnlyDirs, downloader); - } - /// /// Installs all modules given a list of identifiers as a transaction. Resolves dependencies. /// This *will* save the registry at the end of operation. @@ -1040,26 +1027,6 @@ private void AddRemove(ref HashSet possibleConfigOnlyDirs, RegistryManag } } - /// - /// Upgrades the mods listed to the latest versions for the user's KSP. - /// Will *re-install* with warning even if an upgrade is not available. - /// Throws ModuleNotFoundKraken if module is not installed, or not available. - /// - public void Upgrade(IEnumerable identifiers, IDownloader netAsyncDownloader, ref HashSet possibleConfigOnlyDirs, RegistryManager registry_manager, bool enforceConsistency = true) - { - // When upgrading, we are removing these mods first and install them again afterwards (but in different versions). - // So the list of identifiers of modulesToRemove and modulesToInstall is the same, - // RelationshipResolver take care of finding the right CkanModule for each identifier. - List identifierList = identifiers.ToList(); - var resolver = new RelationshipResolver( - identifierList, - identifierList, - RelationshipResolver.DependsOnlyOpts(), - registry_manager.registry, ksp.VersionCriteria() - ); - Upgrade(resolver.ModList(), netAsyncDownloader, ref possibleConfigOnlyDirs, registry_manager, enforceConsistency); - } - /// /// Upgrades or installs the mods listed to the specified versions for the user's KSP. /// Will *re-install* or *downgrade* (with a warning) as well as upgrade. diff --git a/Core/Net/NetModuleCache.cs b/Core/Net/NetModuleCache.cs index 8c0bf0a9c8..dca351a359 100644 --- a/Core/Net/NetModuleCache.cs +++ b/Core/Net/NetModuleCache.cs @@ -157,7 +157,7 @@ public string DescribeAvailability(CkanModule m) const int hashSha1Percent = 20; const int hashSha256Percent = 20; - progress.Report(0); + progress?.Report(0); // Check file exists FileInfo fi = new FileInfo(path); if (!fi.Exists) diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index b0a8e7d23e..c3e194e273 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -69,61 +69,6 @@ private RelationshipResolver(RelationshipResolverOptions options, } } - /// - /// Attempts to convert the identifiers to CkanModules and then calls RelationshipResolver.ctor(IEnumerable{CkanModule}, IEnumerable{CkanModule}, Registry, GameVersion)"/> - /// - /// Identifiers of modules to install, will be converted to CkanModules using CkanModule.FromIDandVersion - /// Identifiers of modules to remove, will be converted to CkanModules using Registry.InstalledModule - /// Options for the RelationshipResolver - /// CKAN registry object for current game instance - /// The current KSP version criteria to consider - public RelationshipResolver(IEnumerable modulesToInstall, IEnumerable modulesToRemove, RelationshipResolverOptions options, IRegistryQuerier registry, - GameVersionCriteria versionCrit) : - this( - modulesToInstall?.Select(mod => TranslateModule(mod, options, registry, versionCrit)), - modulesToRemove? - .Select(mod => - { - var match = CkanModule.idAndVersionMatcher.Match(mod); - return match.Success ? match.Groups["mod"].Value : mod; - }) - .Where(identifier => registry.InstalledModule(identifier) != null) - .Select(identifier => registry.InstalledModule(identifier).Module), - options, registry, versionCrit) - { - // Does nothing, just calls the other overloaded constructor - } - - /// - /// Translate mods from identifiers in its default or identifier=version format into CkanModules, - /// optionally falling back to incompatible modules if no compatibles could be found. - /// - /// The identifier or identifier=version of the module - /// If options.allow_incompatible is set, fall back to searching incompatible modules if no compatible has been found - /// CKAN registry object for current game instance - /// The current KSP version criteria to consider - /// A CkanModule - private static CkanModule TranslateModule(string name, RelationshipResolverOptions options, IRegistryQuerier registry, GameVersionCriteria versionCrit) - { - if (options.allow_incompatible) - { - try - { - return CkanModule.FromIDandVersion(registry, name, versionCrit); - } - catch (ModuleNotFoundKraken) - { - // No versions found matching our game version, so - // look for incompatible versions. - return CkanModule.FromIDandVersion(registry, name, null); - } - } - else - { - return CkanModule.FromIDandVersion(registry, name, versionCrit); - } - } - /// /// Returns the default options for relationship resolution. /// diff --git a/Tests/CapturingUser.cs b/Tests/CapturingUser.cs new file mode 100644 index 0000000000..567268d9d3 --- /dev/null +++ b/Tests/CapturingUser.cs @@ -0,0 +1,58 @@ +using System; +using System.Collections.Generic; + +using CKAN; + +namespace Tests +{ + public class CapturingUser : IUser + { + public CapturingUser(bool headless, + Func yesNoAnswerer, + Func selectionDialogAnswerer) + { + Headless = headless; + this.yesNoAnswerer = yesNoAnswerer; + this.selectionDialogAnswerer = selectionDialogAnswerer; + } + + public bool Headless { get; private set; } + + public bool RaiseYesNoDialog(string question) + { + RaisedYesNoDialogQuestions.Add(question); + return yesNoAnswerer(question); + } + + public int RaiseSelectionDialog(string message, params object[] args) + { + RaisedSelectionDialogs.Add(new Tuple(message, args)); + return selectionDialogAnswerer(message, args); + } + + public void RaiseError(string message, params object[] args) + { + RaisedErrors.Add(string.Format(message, args)); + } + + public void RaiseProgress(string message, int percent) + { + RaisedProgresses.Add(new Tuple(message, percent)); + } + + public void RaiseMessage(string message, params object[] args) + { + RaisedMessages.Add(string.Format(message, args)); + } + + public readonly List RaisedYesNoDialogQuestions = new List(); + public readonly List> RaisedSelectionDialogs = new List>(); + public readonly List RaisedErrors = new List(); + public readonly List> RaisedProgresses = new List>(); + public readonly List RaisedMessages = new List(); + + + private Func yesNoAnswerer; + private Func selectionDialogAnswerer; + } +} diff --git a/Tests/CmdLine/InstallTests.cs b/Tests/CmdLine/InstallTests.cs new file mode 100644 index 0000000000..4e19815204 --- /dev/null +++ b/Tests/CmdLine/InstallTests.cs @@ -0,0 +1,101 @@ +using System.Collections.Generic; +using System.Linq; + +using NUnit.Framework; + +using CKAN; +using CKAN.CmdLine; + +using Tests.Data; +using Tests.Core.Configuration; + +namespace Tests.CmdLine +{ + [TestFixture] + public class InstallTests + { + [Test, + TestCase(new string[] + { + @"{ + ""spec_version"": 1, + ""identifier"": ""InstallableMod"", + ""version"": ""1.0.0"", + ""download"": ""https://github.com/"", + ""install"": [ + { + ""find"": ""DogeCoinFlag"", + ""install_to"": ""GameData"" + } + ] + }", + @"{ + ""spec_version"": 1, + ""identifier"": ""InstallableMod"", + ""version"": ""1.1.0"", + ""download"": ""https://github.com/"", + ""install"": [ + { + ""find"": ""DogeCoinFlag"", + ""install_to"": ""GameData"" + } + ] + }", + @"{ + ""spec_version"": 1, + ""identifier"": ""InstallableMod"", + ""version"": ""1.2.0"", + ""download"": ""https://github.com/"", + ""install"": [ + { + ""find"": ""DogeCoinFlag"", + ""install_to"": ""GameData"" + } + ] + }" + }, + "InstallableMod", + "1.1.0"), + ] + public void RunCommand_IdentifierEqualsVersionSyntax_InstallsCorrectVersion( + string[] modules, + string identifier, + string version) + { + // Arrange + var user = new CapturingUser(false, q => true, (msg, objs) => 0); + using (var inst = new DisposableKSP()) + using (var repo = new TemporaryRepository(modules)) + using (var repoData = new TemporaryRepositoryData(user, repo.repo)) + using (var config = new FakeConfiguration(inst.KSP, inst.KSP.Name)) + using (var manager = new GameInstanceManager(user, config) + { + CurrentInstance = inst.KSP, + }) + { + var regMgr = RegistryManager.Instance(inst.KSP, repoData.Manager); + regMgr.registry.RepositoriesClear(); + regMgr.registry.RepositoriesAdd(repo.repo); + var module = regMgr.registry.GetModuleByVersion(identifier, version); + manager.Cache.Store(module, TestData.DogeCoinFlagZip(), null); + var opts = new InstallOptions() + { + modules = new List { $"{identifier}={version}" }, + }; + + // Act + ICommand cmd = new Install(manager, repoData.Manager, user); + cmd.RunCommand(inst.KSP, opts); + + // Assert + Assert.Multiple(() => + { + CollectionAssert.AreEqual(Enumerable.Empty(), + user.RaisedErrors); + CollectionAssert.AreEqual(new CkanModule[] { module }, + regMgr.registry.InstalledModules.Select(m => m.Module)); + }); + } + } + } +} diff --git a/Tests/CmdLine/UpgradeTests.cs b/Tests/CmdLine/UpgradeTests.cs new file mode 100644 index 0000000000..a0da76a277 --- /dev/null +++ b/Tests/CmdLine/UpgradeTests.cs @@ -0,0 +1,104 @@ +using System.Collections.Generic; +using System.Linq; + +using NUnit.Framework; + +using CKAN; +using CKAN.CmdLine; + +using Tests.Data; +using Tests.Core.Configuration; + +namespace Tests.CmdLine +{ + [TestFixture] + public class UpgradeTests + { + [Test, + TestCase(new string[] + { + @"{ + ""spec_version"": 1, + ""identifier"": ""UpgradableMod"", + ""version"": ""1.0.0"", + ""download"": ""https://github.com/"", + ""install"": [ + { + ""find"": ""DogeCoinFlag"", + ""install_to"": ""GameData"" + } + ] + }", + @"{ + ""spec_version"": 1, + ""identifier"": ""UpgradableMod"", + ""version"": ""1.1.0"", + ""download"": ""https://github.com/"", + ""install"": [ + { + ""find"": ""DogeCoinFlag"", + ""install_to"": ""GameData"" + } + ] + }", + @"{ + ""spec_version"": 1, + ""identifier"": ""UpgradableMod"", + ""version"": ""1.2.0"", + ""download"": ""https://github.com/"", + ""install"": [ + { + ""find"": ""DogeCoinFlag"", + ""install_to"": ""GameData"" + } + ] + }"}, + "UpgradableMod", + "1.0.0", + "1.1.0"), + ] + public void RunCommand_IdentifierEqualsVersionSyntax_UpgradesToCorrectVersion( + string[] modules, + string identifier, + string fromVersion, + string toVersion) + { + // Arrange + var user = new CapturingUser(false, q => true, (msg, objs) => 0); + using (var inst = new DisposableKSP()) + using (var repo = new TemporaryRepository(modules)) + using (var repoData = new TemporaryRepositoryData(user, repo.repo)) + using (var config = new FakeConfiguration(inst.KSP, inst.KSP.Name)) + using (var manager = new GameInstanceManager(user, config) + { + CurrentInstance = inst.KSP, + }) + { + var regMgr = RegistryManager.Instance(inst.KSP, repoData.Manager); + regMgr.registry.RepositoriesClear(); + regMgr.registry.RepositoriesAdd(repo.repo); + var fromModule = regMgr.registry.GetModuleByVersion(identifier, fromVersion); + var toModule = regMgr.registry.GetModuleByVersion(identifier, toVersion); + regMgr.registry.RegisterModule(fromModule, Enumerable.Empty(), inst.KSP, false); + manager.Cache.Store(toModule, TestData.DogeCoinFlagZip(), null); + var opts = new UpgradeOptions() + { + modules = new List { $"{identifier}={toVersion}" }, + }; + + // Act + ICommand cmd = new Upgrade(manager, repoData.Manager, user); + cmd.RunCommand(inst.KSP, opts); + + // Assert + Assert.Multiple(() => + { + CollectionAssert.AreEqual(Enumerable.Empty(), + user.RaisedErrors); + CollectionAssert.AreEqual(new CkanModule[] { toModule }, + regMgr.registry.InstalledModules.Select(m => m.Module)); + }); + } + } + } +} diff --git a/Tests/Core/ModuleInstallerDirTest.cs b/Tests/Core/ModuleInstallerDirTest.cs index 77cbc2be58..944097242a 100644 --- a/Tests/Core/ModuleInstallerDirTest.cs +++ b/Tests/Core/ModuleInstallerDirTest.cs @@ -65,7 +65,7 @@ public void SetUp() _manager.Cache.Store(_testModule, testModFile, new Progress(bytes => {})); HashSet possibleConfigOnlyDirs = null; _installer.InstallList( - new List() { _testModule.identifier }, + new List() { _testModule }, new RelationshipResolverOptions(), _registryManager, ref possibleConfigOnlyDirs); diff --git a/Tests/Core/ModuleInstallerTests.cs b/Tests/Core/ModuleInstallerTests.cs index 28dbf8d0d7..7262c3fe3a 100644 --- a/Tests/Core/ModuleInstallerTests.cs +++ b/Tests/Core/ModuleInstallerTests.cs @@ -423,7 +423,7 @@ public void CanInstallMod() Assert.AreEqual(1, registry.CompatibleModules(ksp.KSP.VersionCriteria()).Count()); // Attempt to install it. - List modules = new List { TestData.DogeCoinFlag_101_module().identifier }; + var modules = new List { TestData.DogeCoinFlag_101_module() }; HashSet possibleConfigOnlyDirs = null; new CKAN.ModuleInstaller(ksp.KSP, manager.Cache, nullUser) @@ -437,47 +437,6 @@ public void CanInstallMod() } } - [Test] - public void InstallList_IdentifierEqualsVersionSyntax_InstallsModule() - { - // Arrange - using (var ksp = new DisposableKSP()) - using (var config = new FakeConfiguration(ksp.KSP, ksp.KSP.Name)) - using (var repo = new TemporaryRepository(TestData.DogeCoinFlag_101())) - using (var repoData = new TemporaryRepositoryData(nullUser, repo.repo)) - using (var manager = new GameInstanceManager(nullUser, config) - { - CurrentInstance = ksp.KSP - }) - { - var registry = CKAN.RegistryManager.Instance(ksp.KSP, repoData.Manager).registry; - registry.RepositoriesClear(); - registry.RepositoriesAdd(repo.repo); - - var inst = new CKAN.ModuleInstaller(ksp.KSP, manager.Cache, nullUser); - - const string mod_file_name = "DogeCoinFlag/Flags/dogecoin.png"; - string mod_file_path = Path.Combine(ksp.KSP.game.PrimaryModDirectory(ksp.KSP), mod_file_name); - - var mod = registry.GetModuleByVersion("DogeCoinFlag", "1.01"); - Assert.IsNotNull(mod, "DogeCoinFlag 1.01 should exist"); - manager.Cache.Store(mod, TestData.DogeCoinFlagZip(), new Progress(bytes => {})); - List modules = new List() - { - $"{mod.identifier}={mod.version}" - }; - - // Act - HashSet possibleConfigOnlyDirs = null; - inst.InstallList(modules, new RelationshipResolverOptions(), - CKAN.RegistryManager.Instance(manager.CurrentInstance, repoData.Manager), - ref possibleConfigOnlyDirs); - - // Assert - Assert.IsTrue(File.Exists(mod_file_path)); - } - } - [Test] public void CanUninstallMod() { @@ -503,7 +462,7 @@ public void CanUninstallMod() TestData.DogeCoinFlagZip(), new Progress(bytes => {})); - List modules = new List { TestData.DogeCoinFlag_101_module().identifier }; + var modules = new List { TestData.DogeCoinFlag_101_module() }; HashSet possibleConfigOnlyDirs = null; new CKAN.ModuleInstaller(manager.CurrentInstance, manager.Cache, nullUser) @@ -516,7 +475,8 @@ public void CanUninstallMod() // Attempt to uninstall it. new CKAN.ModuleInstaller(manager.CurrentInstance, manager.Cache, nullUser) - .UninstallList(modules, ref possibleConfigOnlyDirs, + .UninstallList(modules.Select(m => m.identifier), + ref possibleConfigOnlyDirs, CKAN.RegistryManager.Instance(manager.CurrentInstance, repoData.Manager)); // Check that the module is not installed. @@ -550,7 +510,7 @@ public void UninstallEmptyDirs() TestData.DogeCoinFlagZip(), new Progress(bytes => {})); - List modules = new List { TestData.DogeCoinFlag_101_module().identifier }; + var modules = new List { TestData.DogeCoinFlag_101_module() }; HashSet possibleConfigOnlyDirs = null; new CKAN.ModuleInstaller(manager.CurrentInstance, manager.Cache, nullUser) @@ -566,7 +526,7 @@ public void UninstallEmptyDirs() TestData.DogeCoinPluginZip(), new Progress(bytes => {})); - modules.Add(TestData.DogeCoinPlugin_module().identifier); + modules.Add(TestData.DogeCoinPlugin_module()); new CKAN.ModuleInstaller(manager.CurrentInstance, manager.Cache, nullUser) .InstallList(modules, @@ -581,11 +541,11 @@ public void UninstallEmptyDirs() // Uninstall both mods. - modules.Add(TestData.DogeCoinFlag_101_module().identifier); - modules.Add(TestData.DogeCoinPlugin_module().identifier); + modules.Add(TestData.DogeCoinFlag_101_module()); + modules.Add(TestData.DogeCoinPlugin_module()); new CKAN.ModuleInstaller(manager.CurrentInstance, manager.Cache, nullUser) - .UninstallList(modules, + .UninstallList(modules.Select(m => m.identifier), ref possibleConfigOnlyDirs, CKAN.RegistryManager.Instance(manager.CurrentInstance, repoData.Manager)); @@ -760,7 +720,7 @@ public void ModuleManagerInstancesAreDecoupled() new Progress(bytes => {})); // Attempt to install it. - List modules = new List { TestData.DogeCoinFlag_101_module().identifier }; + var modules = new List { TestData.DogeCoinFlag_101_module() }; HashSet possibleConfigOnlyDirs = null; new CKAN.ModuleInstaller(ksp.KSP, manager.Cache, nullUser) @@ -1150,7 +1110,9 @@ public void Upgrade_WithAutoInst_RemovesAutoRemovable(string[] regularMods, } // Act - installer.Upgrade(upgradeIdentifiers, downloader, ref possibleConfigOnlyDirs, regMgr, false); + installer.Upgrade(upgradeIdentifiers.Select(ident => + registry.LatestAvailable(ident, inst.KSP.VersionCriteria())), + downloader, ref possibleConfigOnlyDirs, regMgr, false); // Assert CollectionAssert.AreEquivalent(correctRemainingIdentifiers, diff --git a/Tests/Core/Relationships/RelationshipResolver.cs b/Tests/Core/Relationships/RelationshipResolver.cs index 19f7c5ac86..16ad38174f 100644 --- a/Tests/Core/Relationships/RelationshipResolver.cs +++ b/Tests/Core/Relationships/RelationshipResolver.cs @@ -34,7 +34,7 @@ public void Constructor_WithoutModules_AlwaysReturns() { var registry = CKAN.Registry.Empty(); options = RelationshipResolver.DefaultOpts(); - Assert.DoesNotThrow(() => new RelationshipResolver(new List(), + Assert.DoesNotThrow(() => new RelationshipResolver(new List(), null, options, registry, null)); } @@ -54,7 +54,7 @@ public void Constructor_WithConflictingModules() { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_a.identifier, mod_b.identifier }; + var list = new List { mod_a, mod_b }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); @@ -83,7 +83,7 @@ public void Constructor_WithConflictingModulesVersion_Throws() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_a.identifier, mod_b.identifier }; + var list = new List { mod_a, mod_b }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); @@ -108,7 +108,7 @@ public void Constructor_WithConflictingModulesVersionMin_Throws(string ver, stri using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_a.identifier, mod_b.identifier }; + var list = new List { mod_a, mod_b }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); @@ -133,7 +133,7 @@ public void Constructor_WithConflictingModulesVersionMax_Throws(string ver, stri using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_a.identifier, mod_b.identifier }; + var list = new List { mod_a, mod_b }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); @@ -164,7 +164,7 @@ public void Constructor_WithConflictingModulesVersionMinMax_Throws(string ver, s using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_a.identifier, mod_b.identifier }; + var list = new List { mod_a, mod_b }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); @@ -189,7 +189,7 @@ public void Constructor_WithNonConflictingModulesVersion_DoesNotThrow(string ver using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_a.identifier, mod_b.identifier }; + var list = new List { mod_a, mod_b }; Assert.DoesNotThrow(() => new RelationshipResolver( list, null, options, registry, null)); @@ -213,7 +213,7 @@ public void Constructor_WithConflictingModulesVersionMin_DoesNotThrow(string ver using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_a.identifier, mod_b.identifier }; + var list = new List { mod_a, mod_b }; Assert.DoesNotThrow(() => new RelationshipResolver( list, null, options, registry, null)); @@ -237,7 +237,7 @@ public void Constructor_WithConflictingModulesVersionMax_DoesNotThrow(string ver using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_a.identifier, mod_b.identifier }; + var list = new List { mod_a, mod_b }; Assert.DoesNotThrow(() => new RelationshipResolver( list, null, options, registry, null)); @@ -262,7 +262,7 @@ public void Constructor_WithConflictingModulesVersionMinMax_DoesNotThrow(string using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_a.identifier, mod_b.identifier }; + var list = new List { mod_a, mod_b }; Assert.DoesNotThrow(() => new RelationshipResolver( list, null, options, registry, null)); @@ -295,24 +295,13 @@ public void Constructor_WithMultipleModulesProviding_Throws() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_d.identifier }; + var list = new List { mod_d }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); } } - [Test] - public void Constructor_WithMissingModules_Throws() - { - var mod_a = generator.GeneratorRandomModule(); - var registry = CKAN.Registry.Empty(); - var list = new List() { mod_a.identifier }; - - Assert.Throws(() => new RelationshipResolver( - list, null, options, registry, null)); - } - // Right now our RR always returns the modules it was provided. However // if we've already got the same version(s) installed, it should be able to // return a list *without* them. This isn't a hard error at the moment, @@ -327,7 +316,7 @@ public void ModList_WithInstalledModules_DoesNotContainThem() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod_a.identifier }; + var list = new List { mod_a }; registry.Installed().Add(mod_a.identifier, mod_a.version); @@ -353,7 +342,7 @@ public void ModList_WithInstalledModulesSuggested_DoesNotContainThem() { var registry = new CKAN.Registry(repoData.Manager, repo.repo); registry.Installed().Add(suggested.identifier, suggested.version); - var list = new List { suggester.identifier }; + var list = new List { suggester }; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); CollectionAssert.Contains(relationship_resolver.ModList(), suggested); @@ -381,7 +370,7 @@ public void ModList_WithSuggestedModulesThatWouldConflict_DoesNotContainThem() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { suggester.identifier, mod.identifier }; + var list = new List { suggester, mod }; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); CollectionAssert.DoesNotContain(relationship_resolver.ModList(), suggested); @@ -408,7 +397,7 @@ public void Constructor_WithConflictingModulesInDependencies_ThrowUnderDefaultSe using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { depender.identifier, conflicts_with_dependant.identifier }; + var list = new List { depender, conflicts_with_dependant }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); @@ -431,7 +420,7 @@ public void Constructor_WithSuggests_HasSuggestedInModlist() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { suggester.identifier }; + var list = new List { suggester }; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); CollectionAssert.Contains(relationship_resolver.ModList(), suggested); @@ -462,7 +451,7 @@ public void Constructor_ContainsSugestedOfSuggested_When_With_all_suggests() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { suggester.identifier }; + var list = new List { suggester }; options.with_all_suggests = true; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); @@ -494,7 +483,7 @@ public void Constructor_ProvidesSatisfyDependencies() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { depender.identifier }; + var list = new List { depender }; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); CollectionAssert.AreEquivalent(relationship_resolver.ModList(), new List @@ -521,7 +510,7 @@ public void Constructor_WithMissingDependants_Throws() var registry = new CKAN.Registry(repoData.Manager, repo.repo); Assert.Throws(() => - new RelationshipResolver(new List { depender.identifier }, + new RelationshipResolver(new List { depender }, null, options, registry, null)); } } @@ -547,7 +536,7 @@ public void Constructor_WithMissingDependantsVersion_Throws(string ver, string d { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { depender.identifier }; + var list = new List { depender }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); @@ -571,11 +560,11 @@ public void Constructor_WithMissingDependantsVersionMin_Throws(string ver, strin using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List() { depender.identifier }; + var list = new List() { depender }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); - list.Add(dependant.identifier); + list.Add(dependant); Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); } @@ -598,7 +587,7 @@ public void Constructor_WithMissingDependantsVersionMax_Throws(string ver, strin using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { depender.identifier, dependant.identifier }; + var list = new List { depender, dependant }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); @@ -628,7 +617,7 @@ public void Constructor_WithMissingDependantsVersionMinMax_Throws(string ver, st using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { depender.identifier, dependant.identifier }; + var list = new List { depender, dependant }; Assert.Throws(() => new RelationshipResolver( list, null, options, registry, null)); @@ -656,7 +645,7 @@ public void Constructor_WithDependantVersion_ChooseCorrectly(string ver, string using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { depender.identifier }; + var list = new List { depender }; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); CollectionAssert.AreEquivalent(relationship_resolver.ModList(), new List @@ -689,7 +678,7 @@ public void Constructor_WithDependantVersionMin_ChooseCorrectly(string ver, stri using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { depender.identifier }; + var list = new List { depender }; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); CollectionAssert.AreEquivalent(relationship_resolver.ModList(), new List @@ -722,7 +711,7 @@ public void Constructor_WithDependantVersionMax_ChooseCorrectly(string ver, stri using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { depender.identifier }; + var list = new List { depender }; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); CollectionAssert.AreEquivalent(relationship_resolver.ModList(), new List @@ -760,7 +749,7 @@ public void Constructor_WithDependantVersionMinMax_ChooseCorrectly(string ver, s using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { depender.identifier }; + var list = new List { depender }; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); CollectionAssert.AreEquivalent(relationship_resolver.ModList(), new List @@ -876,7 +865,7 @@ public void ReasonFor_WithModsNotInList_ThrowsArgumentException() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod.identifier }; + var list = new List { mod }; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); var mod_not_in_resolver_list = generator.GeneratorRandomModule(); @@ -895,7 +884,7 @@ public void ReasonFor_WithUserAddedMods_GivesReasonUserAdded() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod.identifier }; + var list = new List { mod }; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); var reasons = relationship_resolver.ReasonsFor(mod); @@ -917,7 +906,7 @@ public void ReasonFor_WithSuggestedMods_GivesCorrectParent() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod.identifier }; + var list = new List { mod }; options.with_all_suggests = true; var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); @@ -954,7 +943,7 @@ public void ReasonFor_WithTreeOfMods_GivesCorrectParents() using (var repoData = new TemporaryRepositoryData(user, repo.repo)) { var registry = new CKAN.Registry(repoData.Manager, repo.repo); - var list = new List { mod.identifier }; + var list = new List { mod }; options.with_all_suggests = true; options.with_recommends = true; From bea0831cd38c315cae2e099391591921dc71d0f1 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 15:46:38 -0500 Subject: [PATCH 03/22] Use standard Message property for InconsistentKraken --- Cmdline/Action/Install.cs | 3 +-- Cmdline/Main.cs | 2 +- ConsoleUI/InstallScreen.cs | 2 +- ConsoleUI/ModListScreen.cs | 2 +- Core/Registry/RegistryManager.cs | 4 ++-- Core/Types/Kraken.cs | 25 +++++++------------------ GUI/Main/MainInstall.cs | 2 +- 7 files changed, 14 insertions(+), 26 deletions(-) diff --git a/Cmdline/Action/Install.cs b/Cmdline/Action/Install.cs index 9d2a0701f8..033aeca7c4 100644 --- a/Cmdline/Action/Install.cs +++ b/Cmdline/Action/Install.cs @@ -221,8 +221,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options) } catch (InconsistentKraken ex) { - // The prettiest Kraken formats itself for us. - user.RaiseError("{0}", ex.InconsistenciesPretty); + user.RaiseError("{0}", ex.Message); user.RaiseMessage(Properties.Resources.InstallCancelled); return Exit.ERROR; } diff --git a/Cmdline/Main.cs b/Cmdline/Main.cs index 61f7f9500d..b7834d5911 100644 --- a/Cmdline/Main.cs +++ b/Cmdline/Main.cs @@ -323,7 +323,7 @@ private static int Scan(CKAN.GameInstance inst, IUser user, string next_command if (next_command == null) { - user.RaiseError("{0}", kraken.InconsistenciesPretty); + user.RaiseError("{0}", kraken.Message); user.RaiseError(Properties.Resources.ScanNotSaved); } else diff --git a/ConsoleUI/InstallScreen.cs b/ConsoleUI/InstallScreen.cs index d1f5509040..a7c289a234 100644 --- a/ConsoleUI/InstallScreen.cs +++ b/ConsoleUI/InstallScreen.cs @@ -121,7 +121,7 @@ public override void Run(ConsoleTheme theme, Action process = null } catch (MissingCertificateKraken ex) { RaiseError(ex.ToString()); } catch (InconsistentKraken ex) { - RaiseError(ex.InconsistenciesPretty); + RaiseError(ex.Message); } catch (TooManyModsProvideKraken ex) { ConsoleChoiceDialog ch = new ConsoleChoiceDialog( diff --git a/ConsoleUI/ModListScreen.cs b/ConsoleUI/ModListScreen.cs index 1585a0f3f4..e4a4c9dd09 100644 --- a/ConsoleUI/ModListScreen.cs +++ b/ConsoleUI/ModListScreen.cs @@ -483,7 +483,7 @@ private bool ScanForMods() regMgr.ScanUnmanagedFiles(); } catch (InconsistentKraken ex) { // Warn about inconsistent state - RaiseError(Properties.Resources.ModListScanBad, ex.InconsistenciesPretty); + RaiseError(Properties.Resources.ModListScanBad, ex.Message); } return true; } diff --git a/Core/Registry/RegistryManager.cs b/Core/Registry/RegistryManager.cs index 6b5d40f32a..c97a7811e0 100644 --- a/Core/Registry/RegistryManager.cs +++ b/Core/Registry/RegistryManager.cs @@ -90,11 +90,11 @@ private RegistryManager(string path, GameInstance inst, RepositoryDataManager re // automated tools do not care that no one picked a Scatterer config if (gameInstance.User.Headless) { - log.InfoFormat("Loaded registry with inconsistencies:\r\n\r\n{0}", kraken.InconsistenciesPretty); + log.InfoFormat("Loaded registry with inconsistencies:\r\n\r\n{0}", kraken.Message); } else { - log.ErrorFormat("Loaded registry with inconsistencies:\r\n\r\n{0}", kraken.InconsistenciesPretty); + log.ErrorFormat("Loaded registry with inconsistencies:\r\n\r\n{0}", kraken.Message); } } } diff --git a/Core/Types/Kraken.cs b/Core/Types/Kraken.cs index 5ce1e7bc61..3b8dfd7b94 100644 --- a/Core/Types/Kraken.cs +++ b/Core/Types/Kraken.cs @@ -199,35 +199,24 @@ public TooManyModsProvideKraken(string requested, /// public class InconsistentKraken : Kraken { - public string InconsistenciesPretty - { - get - { - return String.Join(Environment.NewLine, - new string[] { Properties.Resources.KrakenInconsistenciesHeader } - .Concat(inconsistencies.Select(msg => $"* {msg}"))); - } - } - public InconsistentKraken(ICollection inconsistencies, Exception innerException = null) - : base(null, innerException) + : base(string.Join(Environment.NewLine, + new string[] { Properties.Resources.KrakenInconsistenciesHeader } + .Concat(inconsistencies.Select(msg => $"* {msg}"))), + innerException) { this.inconsistencies = inconsistencies; } public InconsistentKraken(string inconsistency, Exception innerException = null) - : base(null, innerException) - { - inconsistencies = new List { inconsistency }; - } + : this(new List { inconsistency }, innerException) + { } public string ShortDescription => string.Join("; ", inconsistencies); public override string ToString() - { - return InconsistenciesPretty + Environment.NewLine + Environment.NewLine + StackTrace; - } + => Message + Environment.NewLine + Environment.NewLine + StackTrace; private readonly ICollection inconsistencies; } diff --git a/GUI/Main/MainInstall.cs b/GUI/Main/MainInstall.cs index 3cb7e49073..1fbebdced2 100644 --- a/GUI/Main/MainInstall.cs +++ b/GUI/Main/MainInstall.cs @@ -382,7 +382,7 @@ private void PostInstallMods(object sender, RunWorkerCompletedEventArgs e) break; case InconsistentKraken exc: - currentUser.RaiseMessage(exc.InconsistenciesPretty); + currentUser.RaiseMessage(exc.Message); break; case CancelledActionKraken exc: From ec3ec5e8cf9f969b4c3c6b865d4891acf98c943b Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Tue, 26 Sep 2023 14:21:11 -0500 Subject: [PATCH 04/22] Performance monitoring tools --- Core/Logging.cs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/Core/Logging.cs b/Core/Logging.cs index fa4c5efdd5..eb890e8d2d 100644 --- a/Core/Logging.cs +++ b/Core/Logging.cs @@ -1,6 +1,8 @@ using System; using System.IO; using System.Reflection; +using System.Diagnostics; + using log4net; using log4net.Config; using log4net.Core; @@ -40,5 +42,36 @@ public static void Initialize() } } } + +#if DEBUG + + public static void WithTimeElapsed(Action elapsedCallback, + Action toMeasure) + { + var sw = new Stopwatch(); + sw.Start(); + + toMeasure(); + + sw.Stop(); + elapsedCallback(sw.Elapsed); + } + + public static T WithTimeElapsed(Action elapsedCallback, + Func toMeasure) + { + var sw = new Stopwatch(); + sw.Start(); + + T val = toMeasure(); + + sw.Stop(); + elapsedCallback(sw.Elapsed); + + return val; + } + +#endif + } } From db2e75d444b59d214262e84def5413ad5e677c93 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 22:52:34 -0500 Subject: [PATCH 05/22] Make tag index thread-safe --- Core/Registry/Registry.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Core/Registry/Registry.cs b/Core/Registry/Registry.cs index 5f472cdc9d..3d3c03e580 100644 --- a/Core/Registry/Registry.cs +++ b/Core/Registry/Registry.cs @@ -712,9 +712,12 @@ public Dictionary Tags { get { - if (tags == null) + lock (tagMutex) { - BuildTagIndex(); + if (tags == null) + { + BuildTagIndex(); + } } return tags; } @@ -725,14 +728,19 @@ public HashSet Untagged { get { - if (untagged == null) + lock (tagMutex) { - BuildTagIndex(); + if (untagged == null) + { + BuildTagIndex(); + } } return untagged; } } + private object tagMutex = new object(); + /// /// Assemble a mapping from tags to modules /// From 2b3b9803d0f54675800888b2472aa2eb8b9dab86 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 15:49:07 -0500 Subject: [PATCH 06/22] Parse large JSON module dictionaries in parallel --- ConsoleUI/GameInstanceListScreen.cs | 26 +++++-- ConsoleUI/SplashScreen.cs | 36 +++++++++- ConsoleUI/Toolkit/Symbols.cs | 8 +++ .../JsonParallelDictionaryConverter.cs | 49 ++++++++++++++ Core/Extensions/EnumerableExtensions.cs | 26 +++++++ Core/Registry/InstalledModule.cs | 7 +- Core/Registry/Registry.cs | 1 + Core/Repositories/ProgressImmediate.cs | 19 ++++++ .../ProgressScalePercentsByFileSize.cs | 67 +++++++++++++++++++ Core/Repositories/ReadProgressStream.cs | 11 +-- Core/Repositories/RepositoryData.cs | 27 +++++++- Core/Repositories/RepositoryDataManager.cs | 9 +-- Core/Types/CkanModule.cs | 6 ++ Core/Types/ModuleInstallDescriptor.cs | 6 ++ 14 files changed, 280 insertions(+), 18 deletions(-) create mode 100644 Core/Converters/JsonParallelDictionaryConverter.cs create mode 100644 Core/Repositories/ProgressImmediate.cs create mode 100644 Core/Repositories/ProgressScalePercentsByFileSize.cs diff --git a/ConsoleUI/GameInstanceListScreen.cs b/ConsoleUI/GameInstanceListScreen.cs index dc2d8c3427..c487d7f9b7 100644 --- a/ConsoleUI/GameInstanceListScreen.cs +++ b/ConsoleUI/GameInstanceListScreen.cs @@ -2,6 +2,8 @@ using System.IO; using System.Collections.Generic; using System.ComponentModel; +using System.Linq; + using CKAN.Versioning; using CKAN.ConsoleUI.Toolkit; @@ -75,7 +77,9 @@ public GameInstanceListScreen(GameInstanceManager mgr, RepositoryDataManager rep new List() ); - if (TryGetInstance(theme, instanceList.Selection, repoData, (ConsoleTheme th) => { d.Run(th, (ConsoleTheme thm) => {}); })) { + if (TryGetInstance(theme, instanceList.Selection, repoData, + (ConsoleTheme th) => { d.Run(th, (ConsoleTheme thm) => {}); }, + null)) { try { manager.SetCurrentInstance(instanceList.Selection.Name); } catch (Exception ex) { @@ -108,7 +112,9 @@ public GameInstanceListScreen(GameInstanceManager mgr, RepositoryDataManager rep string.Format(Properties.Resources.InstanceListLoadingInstance, instanceList.Selection.Name), new List() ); - TryGetInstance(theme, instanceList.Selection, repoData, (ConsoleTheme th) => { d.Run(theme, (ConsoleTheme thm) => {}); }); + TryGetInstance(theme, instanceList.Selection, repoData, + (ConsoleTheme th) => { d.Run(theme, (ConsoleTheme thm) => {}); }, + null); // Still launch the screen even if the load fails, // because you need to be able to fix the name/path. LaunchSubScreen(theme, new GameInstanceEditScreen(manager, repoData, instanceList.Selection)); @@ -170,10 +176,15 @@ protected override string MenuTip() /// Game instance /// Repository data manager providing info from repos /// Function that shows a loading message + /// Function to call with progress updates 0-100 /// /// True if successfully loaded, false if it's locked or the registry was corrupted, etc. /// - public static bool TryGetInstance(ConsoleTheme theme, GameInstance ksp, RepositoryDataManager repoData, Action render) + public static bool TryGetInstance(ConsoleTheme theme, + GameInstance ksp, + RepositoryDataManager repoData, + Action render, + IProgress progress) { bool retry; do { @@ -183,7 +194,10 @@ public static bool TryGetInstance(ConsoleTheme theme, GameInstance ksp, Reposito // Show loading message render(theme); // Try to get the lock; this will throw if another instance is in there - RegistryManager.Instance(ksp, repoData); + var regMgr = RegistryManager.Instance(ksp, repoData); + repoData.Prepopulate(regMgr.registry.Repositories.Values.ToList(), + progress); + var compat = regMgr.registry.CompatibleModules(ksp.VersionCriteria()); } catch (RegistryInUseKraken k) { @@ -215,7 +229,9 @@ public static bool TryGetInstance(ConsoleTheme theme, GameInstance ksp, Reposito } catch (Exception e) { ConsoleMessageDialog errd = new ConsoleMessageDialog( - string.Format(Properties.Resources.InstanceListLoadingError, Path.Combine(ksp.CkanDir(), "registry.json"), e.Message), + string.Format(Properties.Resources.InstanceListLoadingError, + Path.Combine(ksp.CkanDir(), "registry.json").Replace('/', Path.DirectorySeparatorChar), + e.ToString()), new List() { Properties.Resources.OK } ); errd.Run(theme); diff --git a/ConsoleUI/SplashScreen.cs b/ConsoleUI/SplashScreen.cs index ff7601c762..92bd0b7ae6 100644 --- a/ConsoleUI/SplashScreen.cs +++ b/ConsoleUI/SplashScreen.cs @@ -28,7 +28,10 @@ public bool Run(ConsoleTheme theme) { // If there's a default instance, try to get the lock for it. GameInstance ksp = manager.CurrentInstance ?? manager.GetPreferredInstance(); - if (ksp != null && !GameInstanceListScreen.TryGetInstance(theme, ksp, repoData, (ConsoleTheme th) => Draw(th, false))) { + if (ksp != null + && !GameInstanceListScreen.TryGetInstance(theme, ksp, repoData, + (ConsoleTheme th) => Draw(th, false), + new Progress(p => drawProgressBar(theme, 22, 20, p)))) { Console.ResetColor(); Console.Clear(); Console.CursorVisible = true; @@ -94,6 +97,35 @@ private void Draw(ConsoleTheme theme, bool pressAny = false) } } + private void drawProgressBar(ConsoleTheme theme, int y, int w, int percent) + { + lock (progBarMutex) + { + try { + var doubleWidth = percent * (w - 2) / 50; + if (doubleWidth > lastProgDblW) + { + int lp = (Console.WindowWidth - w) / 2; + var bar = new string(Symbols.fullBlock, percent * (w - 2) / 100); + if ((doubleWidth & 1) == 1) + { + // "Cheat" an extra half character to make the bar more precise + bar += Symbols.leftHalfBlock; + } + // This can throw if the screen is too small + Console.SetCursorPosition(lp, y); + Console.ForegroundColor = theme.SplashNormalFg; + Console.Write("["); + Console.ForegroundColor = theme.SplashAccentFg; + Console.Write(bar.PadRight(w - 2, ' ')); + Console.ForegroundColor = theme.SplashNormalFg; + Console.Write("]"); + lastProgDblW = doubleWidth; + } + } catch { } + } + } + private void drawCentered(int y, string val) { int lp = (Console.WindowWidth - val.Length) / 2; @@ -106,6 +138,8 @@ private void drawCentered(int y, string val) private GameInstanceManager manager; private RepositoryDataManager repoData; + private int lastProgDblW = -1; + private object progBarMutex = new object(); } } diff --git a/ConsoleUI/Toolkit/Symbols.cs b/ConsoleUI/Toolkit/Symbols.cs index 7105eda7a6..0588060ca2 100644 --- a/ConsoleUI/Toolkit/Symbols.cs +++ b/ConsoleUI/Toolkit/Symbols.cs @@ -128,6 +128,14 @@ public static class Symbols { /// and the letters in the splash screen ASCII art /// public static readonly char lowerHalfBlock = cp437c(0xdc); + /// + /// Full block used for most of a progress bar + /// + public static readonly char fullBlock = cp437c(0xdb); + /// + /// Left half block used for right edge of odd width progress bar + /// + public static readonly char leftHalfBlock = cp437c(0xdd); private static char cp437c(byte num) { return dosCodePage.GetChars(new byte[] {num})[0]; } private static string cp437s(byte num) { return $"{cp437c(num)}"; } diff --git a/Core/Converters/JsonParallelDictionaryConverter.cs b/Core/Converters/JsonParallelDictionaryConverter.cs new file mode 100644 index 0000000000..09d0c97cd6 --- /dev/null +++ b/Core/Converters/JsonParallelDictionaryConverter.cs @@ -0,0 +1,49 @@ +using System; +using System.Linq; +using System.Collections.Generic; +using System.Collections.Concurrent; + +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; + +using CKAN.Extensions; + +namespace CKAN +{ + /// + /// A converter that loads a dictionary in parallel, + /// use with large collections of complex objects for a speed boost + /// + public class JsonParallelDictionaryConverter : JsonConverter + { + public override object ReadJson(JsonReader reader, + Type objectType, + object existingValue, + JsonSerializer serializer) + => ParseWithProgress(JObject.Load(reader) + .Properties() + .ToArray(), + serializer); + + private object ParseWithProgress(JProperty[] properties, + JsonSerializer serializer) + => Partitioner.Create(properties, true) + .AsParallel() + .WithMergeOptions(ParallelMergeOptions.NotBuffered) + .Select(prop => new KeyValuePair( + prop.Name, + prop.Value.ToObject())) + .WithProgress(properties.Length, + serializer.Context.Context as IProgress) + .ToDictionary(); + + public override bool CanWrite => false; + public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) + { + throw new NotImplementedException(); + } + + // Only convert when we're an explicit attribute + public override bool CanConvert(Type object_type) => false; + } +} diff --git a/Core/Extensions/EnumerableExtensions.cs b/Core/Extensions/EnumerableExtensions.cs index 0541187449..cff26ae632 100644 --- a/Core/Extensions/EnumerableExtensions.cs +++ b/Core/Extensions/EnumerableExtensions.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Threading; namespace CKAN.Extensions { @@ -34,6 +35,31 @@ public static Dictionary ToDictionary(this IEnumerable pairs.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); + public static Dictionary ToDictionary(this ParallelQuery> pairs) + => pairs.ToDictionary(kvp => kvp.Key, + kvp => kvp.Value); + + // https://stackoverflow.com/a/55591477/2422988 + public static ParallelQuery WithProgress(this ParallelQuery source, + long totalCount, + IProgress progress) + { + long count = 0; + int prevPercent = -1; + return progress == null + ? source + : source.Select(item => + { + var percent = (int)(100 * Interlocked.Increment(ref count) / totalCount); + if (percent > prevPercent) + { + progress.Report(percent); + prevPercent = percent; + } + return item; + }); + } + public static IEnumerable Memoize(this IEnumerable source) { if (source == null) diff --git a/Core/Registry/InstalledModule.cs b/Core/Registry/InstalledModule.cs index a12e74baed..2b80f68e47 100644 --- a/Core/Registry/InstalledModule.cs +++ b/Core/Registry/InstalledModule.cs @@ -146,10 +146,15 @@ private InstalledModule() [OnDeserialized] private void DeSerialisationFixes(StreamingContext context) { + if (installed_files == null) + { + installed_files = new Dictionary(); + } if (Platform.IsWindows) { // We need case insensitive path matching on Windows - installed_files = new Dictionary(installed_files, StringComparer.OrdinalIgnoreCase); + installed_files = new Dictionary(installed_files, + StringComparer.OrdinalIgnoreCase); } } diff --git a/Core/Registry/Registry.cs b/Core/Registry/Registry.cs index 3d3c03e580..ff6222c664 100644 --- a/Core/Registry/Registry.cs +++ b/Core/Registry/Registry.cs @@ -42,6 +42,7 @@ public class Registry : IEnlistmentNotification, IRegistryQuerier private Dictionary installed_dlls; [JsonProperty] + [JsonConverter(typeof(JsonParallelDictionaryConverter))] private Dictionary installed_modules; // filename (case insensitive on Windows) => module diff --git a/Core/Repositories/ProgressImmediate.cs b/Core/Repositories/ProgressImmediate.cs new file mode 100644 index 0000000000..ae62de473b --- /dev/null +++ b/Core/Repositories/ProgressImmediate.cs @@ -0,0 +1,19 @@ +using System; + +namespace CKAN +{ + public class ProgressImmediate : IProgress + { + public ProgressImmediate(Action action) + { + this.action = action; + } + + public void Report(T val) + { + action(val); + } + + private readonly Action action; + } +} diff --git a/Core/Repositories/ProgressScalePercentsByFileSize.cs b/Core/Repositories/ProgressScalePercentsByFileSize.cs new file mode 100644 index 0000000000..599b8165cc --- /dev/null +++ b/Core/Repositories/ProgressScalePercentsByFileSize.cs @@ -0,0 +1,67 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +namespace CKAN +{ + /// + /// Accepts progress updates in terms of percentage of one file within a group + /// and translates them into percentages across the whole operation. + /// + public class ProgressScalePercentsByFileSizes : IProgress + { + /// + /// Initialize an percent-to-scaled-percent progress translator + /// + /// The upstream progress object expecting percentages + /// Sequence of sizes of files in our group + public ProgressScalePercentsByFileSizes(IProgress percentProgress, + IEnumerable sizes) + { + this.percentProgress = percentProgress; + this.sizes = sizes.ToArray(); + totalSize = this.sizes.Sum(); + } + + /// + /// The IProgress member called when we advance within the current file + /// + /// How far into the current file we are + public void Report(int currentFilePercent) + { + if (basePercent < 100 && currentIndex < sizes.Length) + { + var percent = basePercent + (int)(currentFilePercent * sizes[currentIndex] / totalSize); + // Only report each percentage once, to avoid spamming UI calls + if (percent > lastPercent) + { + percentProgress?.Report(percent); + lastPercent = percent; + } + } + } + + /// + /// Call this when you move on from one file to the next + /// + public void NextFile() + { + doneSize += sizes[currentIndex]; + basePercent = (int)(100 * doneSize / totalSize); + ++currentIndex; + if (basePercent > lastPercent) + { + percentProgress?.Report(basePercent); + lastPercent = basePercent; + } + } + + private IProgress percentProgress; + private long[] sizes; + private long totalSize; + private long doneSize = 0; + private int currentIndex = 0; + private int basePercent = 0; + private int lastPercent = -1; + } +} diff --git a/Core/Repositories/ReadProgressStream.cs b/Core/Repositories/ReadProgressStream.cs index 9011a4676a..2fd2fa5534 100644 --- a/Core/Repositories/ReadProgressStream.cs +++ b/Core/Repositories/ReadProgressStream.cs @@ -20,11 +20,14 @@ public ReadProgressStream(Stream stream, IProgress progress) public override int Read(byte[] buffer, int offset, int count) { int amountRead = base.Read(buffer, offset, count); - long newProgress = Position; - if (newProgress > lastProgress) + if (progress != null) { - progress?.Report(newProgress); - lastProgress = newProgress; + long newProgress = Position; + if (newProgress > lastProgress) + { + progress?.Report(newProgress); + lastProgress = newProgress; + } } return amountRead; } diff --git a/Core/Repositories/RepositoryData.cs b/Core/Repositories/RepositoryData.cs index 43ac5d7881..5f71e2e0c2 100644 --- a/Core/Repositories/RepositoryData.cs +++ b/Core/Repositories/RepositoryData.cs @@ -3,6 +3,8 @@ using System.Text; using System.Linq; using System.Collections.Generic; +using System.Collections.Concurrent; +using System.Runtime.Serialization; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -26,6 +28,7 @@ public class RepositoryData /// The available modules from this repository /// [JsonProperty("available_modules", NullValueHandling = NullValueHandling.Ignore)] + [JsonConverter(typeof(JsonParallelDictionaryConverter))] public readonly Dictionary AvailableModules; /// @@ -92,19 +95,37 @@ public void SaveTo(string path) /// Load a previously cached repo data object from JSON on disk /// /// Filename of the JSON file to load + /// Progress notifier to receive updates of percent completion of this file /// A repo data object or null if loading fails - public static RepositoryData FromJson(string path, IProgress progress) + public static RepositoryData FromJson(string path, IProgress progress) { try { + long fileSize = new FileInfo(path).Length; log.DebugFormat("Trying to load repository data from {0}", path); // Ain't OOP grand?! using (var stream = File.Open(path, FileMode.Open)) - using (var progressStream = new ReadProgressStream(stream, progress)) + using (var progressStream = new ReadProgressStream( + stream, + progress == null + ? null + // Treat JSON parsing as the first 50% + : new ProgressImmediate(p => progress.Report((int)(50 * p / fileSize))))) using (var reader = new StreamReader(progressStream)) using (var jStream = new JsonTextReader(reader)) { - return new JsonSerializer().Deserialize(jStream); + var settings = new JsonSerializerSettings() + { + Context = new StreamingContext( + StreamingContextStates.Other, + progress == null + ? null + : new ProgressImmediate(p => + // Treat CkanModule creation as the last 50% + progress.Report(50 + p / 2))), + }; + return JsonSerializer.Create(settings) + .Deserialize(jStream); } } catch (Exception exc) diff --git a/Core/Repositories/RepositoryDataManager.cs b/Core/Repositories/RepositoryDataManager.cs index 0c1362b651..bca211b80a 100644 --- a/Core/Repositories/RepositoryDataManager.cs +++ b/Core/Repositories/RepositoryDataManager.cs @@ -100,7 +100,7 @@ public void Prepopulate(List repos, IProgress percentProgress) new FileInfo(tuple.Item2).Length)) .ToList(); // Translate from file group offsets to percent - var progress = new ProgressFilesOffsetsToPercent( + var progress = new ProgressScalePercentsByFileSizes( percentProgress, reposAndSizes.Select(tuple => tuple.Item2)); foreach (var repo in reposAndSizes.Select(tuple => tuple.Item1)) { @@ -258,10 +258,11 @@ private RepositoryData GetRepoData(Repository repo) ? data : LoadRepoData(repo, null); - private RepositoryData LoadRepoData(Repository repo, IProgress progress) + private RepositoryData LoadRepoData(Repository repo, IProgress progress) { - log.DebugFormat("Looking for data in {0}", GetRepoDataPath(repo)); - var data = RepositoryData.FromJson(GetRepoDataPath(repo), progress); + var path = GetRepoDataPath(repo); + log.DebugFormat("Looking for data in {0}", path); + var data = RepositoryData.FromJson(path, progress); if (data != null) { log.Debug("Found it! Adding..."); diff --git a/Core/Types/CkanModule.cs b/Core/Types/CkanModule.cs index 9136e7dd26..5636ebe1f1 100644 --- a/Core/Types/CkanModule.cs +++ b/Core/Types/CkanModule.cs @@ -6,6 +6,7 @@ using System.Runtime.Serialization; using System.Text; using System.Text.RegularExpressions; +using System.Reflection; using Autofac; using log4net; @@ -281,6 +282,11 @@ internal CkanModule() // We don't have this passed in, so we'll ask the service locator // directly. Yuck. _comparator = ServiceLocator.Container.Resolve(); + download_content_type = typeof(CkanModule).GetTypeInfo() + .GetDeclaredField("download_content_type") + .GetCustomAttribute() + .Value + .ToString(); } /// diff --git a/Core/Types/ModuleInstallDescriptor.cs b/Core/Types/ModuleInstallDescriptor.cs index a194e745f1..48e9d9a352 100644 --- a/Core/Types/ModuleInstallDescriptor.cs +++ b/Core/Types/ModuleInstallDescriptor.cs @@ -7,6 +7,7 @@ using System.Text; using System.Text.RegularExpressions; using System.Runtime.CompilerServices; +using System.Reflection; using ICSharpCode.SharpZipLib.Zip; using Newtonsoft.Json; @@ -109,6 +110,11 @@ internal void DeSerialisationFixes(StreamingContext like_i_could_care) [JsonConstructor] private ModuleInstallDescriptor() { + install_to = typeof(ModuleInstallDescriptor).GetTypeInfo() + .GetDeclaredField("install_to") + .GetCustomAttribute() + .Value + .ToString(); } /// From 4276e2091fe8a9f18b6fac56d174ce27cff408d5 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 16:43:16 -0500 Subject: [PATCH 07/22] Parallelize loading of Versions tab --- GUI/Controls/ModInfoTabs/Versions.cs | 90 +++++++++++++++------------- 1 file changed, 48 insertions(+), 42 deletions(-) diff --git a/GUI/Controls/ModInfoTabs/Versions.cs b/GUI/Controls/ModInfoTabs/Versions.cs index a6f7bda121..bc5b89bdc7 100644 --- a/GUI/Controls/ModInfoTabs/Versions.cs +++ b/GUI/Controls/ModInfoTabs/Versions.cs @@ -2,8 +2,10 @@ using System.Linq; using System.ComponentModel; using System.Collections.Generic; +using System.Collections.Concurrent; using System.Drawing; using System.Windows.Forms; +using System.Threading; using System.Threading.Tasks; using Autofac; @@ -36,6 +38,8 @@ public GUIMod SelectedModule { if (!(visibleGuiModule?.Equals(value) ?? value?.Equals(visibleGuiModule) ?? true)) { + // Stop background loading of row colors + cancelTokenSrc?.Cancel(); // Listen for property changes (we only care about GUIMod.SelectedMod) if (visibleGuiModule != null) { @@ -55,9 +59,10 @@ public GUIMod SelectedModule } } - private RepositoryDataManager repoData; - private GUIMod visibleGuiModule; - private bool ignoreItemCheck; + private RepositoryDataManager repoData; + private GUIMod visibleGuiModule; + private bool ignoreItemCheck; + private CancellationTokenSource cancelTokenSrc; private void VersionsListView_ItemCheck(object sender, ItemCheckEventArgs e) { @@ -201,51 +206,52 @@ private ListViewItem[] getItems(GUIMod gmod, List versions) [ForbidGUICalls] private void checkInstallable(ListViewItem[] items) { - GameInstance currentInstance = Main.Instance.Manager.CurrentInstance; - IRegistryQuerier registry = RegistryManager.Instance(currentInstance, repoData).registry; - bool latestCompatibleVersionAlreadyFound = false; - var installer = new ModuleInstaller( - currentInstance, - Main.Instance.Manager.Cache, - Main.Instance.currentUser); - foreach (ListViewItem item in items) - { - if (item.ListView == null) - { - // User switched to another mod, quit - break; - } - if (installable(installer, item.Tag as CkanModule, registry)) - { - if (!latestCompatibleVersionAlreadyFound) - { - latestCompatibleVersionAlreadyFound = true; - Util.Invoke(this, () => - { - item.BackColor = Color.Green; - item.ForeColor = Color.White; - }); - } - else - { - Util.Invoke(this, () => - { - item.BackColor = Color.LightGreen; - }); - } - } - } - Util.Invoke(this, () => - { - UseWaitCursor = false; - }); + var currentInstance = Main.Instance.Manager.CurrentInstance; + var registry = RegistryManager.Instance(currentInstance, repoData).registry; + var installer = new ModuleInstaller(currentInstance, + Main.Instance.Manager.Cache, + Main.Instance.currentUser); + ListViewItem latestCompatible = null; + // Load balance the items so they're processed roughly in-order instead of blocks + Partitioner.Create(items, true) + // Distribute across cores + .AsParallel() + // Abort when they switch to another mod + .WithCancellation(cancelTokenSrc.Token) + // Return them as they're processed + .WithMergeOptions(ParallelMergeOptions.NotBuffered) + // Slow step to be performed across multiple cores + .Where(item => installable(installer, item.Tag as CkanModule, registry)) + // Jump back to GUI thread for the updates for each compatible item + .ForAll(item => Util.Invoke(this, () => + { + if (latestCompatible == null || item.Index < latestCompatible.Index) + { + if (latestCompatible != null) + { + // Revert color of previous best guess + latestCompatible.BackColor = Color.LightGreen; + latestCompatible.ForeColor = SystemColors.ControlText; + } + latestCompatible = item; + item.BackColor = Color.Green; + item.ForeColor = Color.White; + } + else + { + item.BackColor = Color.LightGreen; + } + })); + Util.Invoke(this, () => UseWaitCursor = false); } private void Refresh(GUIMod gmod) { + // checkInstallable needs this to stop background threads on switch to another mod + cancelTokenSrc = new CancellationTokenSource(); var startingModule = gmod; var items = getItems(gmod, getVersions(gmod)); - Util.Invoke(this, () => + Util.AsyncInvoke(this, () => { VersionsListView.BeginUpdate(); VersionsListView.Items.Clear(); From 0a128058ec0ccaaf9be120e81be310be6cf1e482 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 17:06:13 -0500 Subject: [PATCH 08/22] Parallelize loading of CompatibilitySorter --- Core/Extensions/EnumerableExtensions.cs | 4 + Core/Registry/CompatibilitySorter.cs | 187 +++++++++++------------- Core/Repositories/AvailableModule.cs | 13 +- 3 files changed, 98 insertions(+), 106 deletions(-) diff --git a/Core/Extensions/EnumerableExtensions.cs b/Core/Extensions/EnumerableExtensions.cs index cff26ae632..5f17794157 100644 --- a/Core/Extensions/EnumerableExtensions.cs +++ b/Core/Extensions/EnumerableExtensions.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Collections.Concurrent; using System.Linq; using System.Threading; @@ -39,6 +40,9 @@ public static Dictionary ToDictionary(this ParallelQuery pairs.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); + public static ConcurrentDictionary ToConcurrentDictionary(this IEnumerable> pairs) + => new ConcurrentDictionary(pairs); + // https://stackoverflow.com/a/55591477/2422988 public static ParallelQuery WithProgress(this ParallelQuery source, long totalCount, diff --git a/Core/Registry/CompatibilitySorter.cs b/Core/Registry/CompatibilitySorter.cs index b5d5fa9024..00f85bd435 100644 --- a/Core/Registry/CompatibilitySorter.cs +++ b/Core/Registry/CompatibilitySorter.cs @@ -1,4 +1,6 @@ +using System; using System.Collections.Generic; +using System.Collections.Concurrent; using System.Linq; using log4net; @@ -33,12 +35,35 @@ public CompatibilitySorter(GameVersionCriteria crit this.installed = installed; this.dlls = dlls; this.dlc = dlc; - var merged = available.SelectMany(dict => dict) - .GroupBy(kvp => kvp.Key, - kvp => kvp.Value) - .ToDictionary(grp => grp.Key, - grp => AvailableModule.Merge(grp.ToList())); - PartitionModules(merged, CompatibleProviders(crit, providers)); + + // Running these independent prep steps in parallel isn't faster (they're each already parallel) + + // Mapping from identifiers to trivially compatible mods providing those identifiers + log.Debug("Calculating compatible provider mapping"); + var compatProv = CompatibleProviders(crit, providers); + + // Split mods into compatible, incompatible, and indeterminate + log.Debug("Partitioning modules by compatibility"); + var groups = getCompatGroups(available).ToArray(); + + Compatible = groups.FirstOrDefault(tuple => tuple.Item1 == true) + ?.Item2 + ?? new ConcurrentDictionary(); + + Incompatible = groups.FirstOrDefault(tuple => tuple.Item1 == false) + ?.Item2 + ?? new ConcurrentDictionary(); + + // Mods that might be compatible or incompatible based on their dependencies + var indeterminate = groups.FirstOrDefault(tuple => tuple.Item1 == null) + ?.Item2 + ?? new ConcurrentDictionary(); + + // Sort out the complexly [in]compatible mods + indeterminate.AsParallel() + .ForAll(kvp => CheckDepends(kvp.Key, kvp.Value, + compatProv, new Stack())); + log.Debug("Done partitioning modules by compatibility"); } /// @@ -49,8 +74,7 @@ public CompatibilitySorter(GameVersionCriteria crit /// /// Mods that are compatible with our versions /// - public readonly SortedDictionary Compatible - = new SortedDictionary(); + public readonly ConcurrentDictionary Compatible; public ICollection LatestCompatible { @@ -67,8 +91,7 @@ public ICollection LatestCompatible /// /// Mods that are incompatible with our versions /// - public readonly SortedDictionary Incompatible - = new SortedDictionary(); + public readonly ConcurrentDictionary Incompatible; public ICollection LatestIncompatible { @@ -82,17 +105,6 @@ public ICollection LatestIncompatible } } - /// - /// Mods that might be compatible or incompatible based on their dependencies - /// - private readonly SortedDictionary Indeterminate = new SortedDictionary(); - - /// - /// Mods for which we have an active call to CheckDepends right now - /// in the call stack, used to avoid infinite recursion on circular deps. - /// - private readonly Stack Investigating = new Stack(); - private readonly Dictionary installed; private readonly HashSet dlls; private readonly IDictionary dlc; @@ -111,76 +123,40 @@ public ICollection LatestIncompatible private Dictionary> CompatibleProviders( GameVersionCriteria crit, Dictionary> providers) - { - log.Debug("Calculating compatible provider mapping"); - var compat = new Dictionary>(); - foreach (var (ident, availMods) in providers) - { - var compatGroups = availMods - .GroupBy(availMod => availMod.AllAvailable() - .Any(ckm => !ckm.IsDLC - && ckm.ProvidesList.Contains(ident) - && ckm.IsCompatible(crit))) - .ToDictionary(grp => grp.Key, - grp => grp); - if (!compatGroups.ContainsKey(false)) - { - // Everything is compatible, just re-use the same HashSet - compat.Add(ident, availMods); - } - else if (compatGroups.TryGetValue(true, out var compatGroup)) - { - // Some are compatible, put them in a new HashSet - compat.Add(ident, compatGroup.ToHashSet()); - } - // Else if nothing compatible, just skip this one - } - log.Debug("Done calculating compatible provider mapping"); - return compat; - } + => providers + .AsParallel() + .Select(kvp => new KeyValuePair>( + kvp.Key, + kvp.Value.Where(availMod => availMod.AllAvailable() + .Any(ckm => !ckm.IsDLC + && ckm.ProvidesList.Contains(kvp.Key) + && ckm.IsCompatible(crit))) + .ToHashSet())) + .Where(kvp => kvp.Value.Count > 0) + .ToDictionary(); - /// - /// Split the given mods into compatible and incompatible. - /// Handles all levels of dependencies. - /// - /// All mods available from registry - /// Mapping from identifiers to mods providing those identifiers - private void PartitionModules(Dictionary available, - Dictionary> providers) - { - log.Debug("Partitioning modules by compatibility"); - // First get the ones that are trivially [in]compatible. - foreach (var kvp in available) - { - if (kvp.Value.AllAvailable().All(m => !m.IsCompatible(CompatibleVersions))) - { - // No versions compatible == incompatible - log.DebugFormat("Trivially incompatible: {0}", kvp.Key); - Incompatible.Add(kvp.Key, kvp.Value); - } - else if (kvp.Value.AllAvailable().All(m => m.depends == null)) - { - // No dependencies == compatible - log.DebugFormat("Trivially compatible: {0}", kvp.Key); - Compatible.Add(kvp.Key, kvp.Value); - } - else - { - // Need to investigate this one more later - log.DebugFormat("Trivially indeterminate: {0}", kvp.Key); - Indeterminate.Add(kvp.Key, kvp.Value); - } - } - log.Debug("Trivial mods done, moving on to indeterminates"); - // We'll be modifying `indeterminate` during this loop, so `foreach` is out - while (Indeterminate.Count > 0) - { - var kvp = Indeterminate.First(); - log.DebugFormat("Checking: {0}", kvp.Key); - CheckDepends(kvp.Key, kvp.Value, providers); - } - log.Debug("Done partitioning modules by compatibility"); - } + private IEnumerable>> getCompatGroups( + IEnumerable> available) + // Merge AvailableModules with duplicate identifiers + => available.SelectMany(dict => dict) + .GroupBy(kvp => kvp.Key, + kvp => kvp.Value) + .ToDictionary(grp => grp.Key, + grp => AvailableModule.Merge(grp)) + // Group into trivially [in]compatible (false/true) and indeterminate (null) + .AsParallel() + .GroupBy(kvp => kvp.Value.AllAvailable() + .All(m => !m.IsCompatible(CompatibleVersions)) + // No versions compatible == incompatible + ? (bool?)false + : kvp.Value.AllAvailable() + .All(m => m.depends == null) + // No dependencies == compatible + ? (bool?)true + // Need to investigate this one more later + : (bool?)null) + .Select(grp => new Tuple>( + grp.Key, grp.ToConcurrentDictionary())); /// /// Move an indeterminate module to Compatible or Incompatible @@ -189,9 +165,18 @@ private void PartitionModules(Dictionary avail /// Identifier of the module to check /// The module to check /// Mapping from identifiers to mods providing those identifiers - private void CheckDepends(string identifier, AvailableModule am, Dictionary> providers) + /// Mods for which we have an active call to CheckDepends right now in the call stack, used to avoid infinite recursion on circular deps + private void CheckDepends(string identifier, + AvailableModule am, + Dictionary> providers, + Stack investigating) { - Investigating.Push(identifier); + if (Compatible.ContainsKey(identifier) || Incompatible.ContainsKey(identifier)) + { + // Already checked this one on another branch, don't repeat + return; + } + investigating.Push(identifier); foreach (CkanModule m in am.AllAvailable().Where(m => m.IsCompatible(CompatibleVersions))) { log.DebugFormat("What about {0}?", m.version); @@ -201,7 +186,7 @@ private void CheckDepends(string identifier, AvailableModule am, Dictionary kvp.Value.Module), dlls, dlc)) + if (rel.MatchesAny(installed.Select(kvp => kvp.Value.Module).ToList(), dlls, dlc)) { // Matches a DLL or DLC, cool foundCompat = true; @@ -221,15 +206,15 @@ private void CheckDepends(string identifier, AvailableModule am, Dictionary diff --git a/Core/Repositories/AvailableModule.cs b/Core/Repositories/AvailableModule.cs index f116972d21..a74f072e03 100644 --- a/Core/Repositories/AvailableModule.cs +++ b/Core/Repositories/AvailableModule.cs @@ -53,10 +53,15 @@ internal void DeserialisationFixes(StreamingContext context) Debug.Assert(module_version.Values.All(m => identifier.Equals(m.identifier))); } - public static AvailableModule Merge(IList availMods) - => availMods.Count == 1 ? availMods.First() - : new AvailableModule(availMods.First().identifier, - availMods.Reverse().SelectMany(am => am.AllAvailable())); + /// + /// Generate a new AvailableModule given its CkanModules + /// + /// Sequence of mods to be contained, expected to be IGrouping<>, so it should support O(1) Count(), even though IEnumerable<> in general does not + /// + public static AvailableModule Merge(IEnumerable availMods) + => availMods.Count() == 1 ? availMods.First() + : new AvailableModule(availMods.First().identifier, + availMods.Reverse().SelectMany(am => am.AllAvailable())); // The map of versions -> modules, that's what we're about! // First element is the oldest version, last is the newest. From 89fe1d35ed922ad9cd7fdda0bed8a746755c84bb Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 18:41:20 -0500 Subject: [PATCH 09/22] Fix RelationshipResolver reporting conflicts with null --- Core/Registry/Registry.cs | 4 +- Core/Relationships/RelationshipResolver.cs | 29 +++----- Core/Relationships/SanityChecker.cs | 85 ++++++++++------------ Core/Types/Kraken.cs | 20 ++--- Tests/Core/Relationships/SanityChecker.cs | 4 +- 5 files changed, 63 insertions(+), 79 deletions(-) diff --git a/Core/Registry/Registry.cs b/Core/Registry/Registry.cs index ff6222c664..fc62479203 100644 --- a/Core/Registry/Registry.cs +++ b/Core/Registry/Registry.cs @@ -1181,9 +1181,9 @@ public static IEnumerable FindReverseDependencies( var brokenDeps = SanityChecker.FindUnsatisfiedDepends(hypothetical, dlls, dlc); if (satisfiedFilter != null) { - brokenDeps.RemoveAll(kvp => satisfiedFilter(kvp.Value)); + brokenDeps.RemoveAll(kvp => satisfiedFilter(kvp.Item2)); } - var brokenIdents = brokenDeps.Select(x => x.Key.identifier).ToHashSet(); + var brokenIdents = brokenDeps.Select(x => x.Item1.identifier).ToHashSet(); if (modulesToInstall != null) { diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index c3e194e273..fe12a35539 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -119,8 +119,8 @@ private void AddModulesToInstall(IEnumerable modules) { if (options.proceed_with_inconsistencies) { - conflicts.Add(new KeyValuePair(listed_mod, module)); - conflicts.Add(new KeyValuePair(module, listed_mod)); + conflicts.Add(new ModPair(listed_mod, module)); + conflicts.Add(new ModPair(module, listed_mod)); } else { @@ -150,20 +150,11 @@ private void AddModulesToInstall(IEnumerable modules) registry.InstalledDlls, registry.InstalledDlc); } - catch (BadRelationshipsKraken k) + catch (BadRelationshipsKraken k) when (options.without_enforce_consistency) { - // Add to this.conflicts (catches conflicting DLLs and DLCs that the above loops miss) - foreach (var kvp in k.Conflicts) - { - conflicts.Add(new KeyValuePair( - kvp.Key, null - )); - } - if (!options.without_enforce_consistency) - { - // Only re-throw if caller asked for consistency enforcement - throw; - } + conflicts.AddRange(k.Conflicts.Select(kvp => new ModPair(kvp.Item1, kvp.Item3)) + .Where(kvp => !conflicts.Contains(kvp)) + .ToArray()); } } @@ -285,8 +276,8 @@ private void ResolveStanza(IEnumerable stanza, Selection CkanModule module = modlist.Values.FirstOrDefault(m => descriptor.ContainsAny(new string[] { m.identifier })); if (options.proceed_with_inconsistencies) { - conflicts.Add(new KeyValuePair(module, reason.Parent)); - conflicts.Add(new KeyValuePair(reason.Parent, module)); + conflicts.Add(new ModPair(module, reason.Parent)); + conflicts.Add(new ModPair(reason.Parent, module)); continue; } else @@ -310,8 +301,8 @@ private void ResolveStanza(IEnumerable stanza, Selection CkanModule module = installed_modules.FirstOrDefault(m => descriptor.ContainsAny(new string[] { m.identifier })); if (options.proceed_with_inconsistencies) { - conflicts.Add(new KeyValuePair(module, reason.Parent)); - conflicts.Add(new KeyValuePair(reason.Parent, module)); + conflicts.Add(new ModPair(module, reason.Parent)); + conflicts.Add(new ModPair(reason.Parent, module)); continue; } else diff --git a/Core/Relationships/SanityChecker.cs b/Core/Relationships/SanityChecker.cs index 1611866bf4..bb0e845d7e 100644 --- a/Core/Relationships/SanityChecker.cs +++ b/Core/Relationships/SanityChecker.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; @@ -6,6 +7,9 @@ namespace CKAN { + using modRelPair = Tuple; + using modRelList = List>; + /// /// Sanity checks on what mods we have installed, or may install. /// @@ -20,9 +24,8 @@ public static void EnforceConsistency(IEnumerable modules IEnumerable dlls = null, IDictionary dlc = null) { - List> unmetDepends; - List> conflicts; - if (!CheckConsistency(modules, dlls, dlc, out unmetDepends, out conflicts)) + if (!CheckConsistency(modules, dlls, dlc, + out modRelList unmetDepends, out modRelList conflicts)) { throw new BadRelationshipsKraken(unmetDepends, conflicts); } @@ -38,17 +41,16 @@ public static bool IsConsistent(IEnumerable modules, => CheckConsistency(modules, dlls, dlc, out var _, out var _); - private static bool CheckConsistency( - IEnumerable modules, - IEnumerable dlls, - IDictionary dlc, - out List> UnmetDepends, - out List> Conflicts) + private static bool CheckConsistency(IEnumerable modules, + IEnumerable dlls, + IDictionary dlc, + out modRelList UnmetDepends, + out modRelList Conflicts) { - modules = modules?.Memoize(); + var modList = modules?.ToList(); var dllSet = dlls?.ToHashSet(); - UnmetDepends = FindUnsatisfiedDepends(modules?.ToList(), dllSet, dlc); - Conflicts = FindConflicting( modules, dllSet, dlc); + UnmetDepends = FindUnsatisfiedDepends(modList, dllSet, dlc); + Conflicts = FindConflicting(modList, dllSet, dlc); return !UnmetDepends.Any() && !Conflicts.Any(); } @@ -62,15 +64,13 @@ private static bool CheckConsistency( /// List of dependencies that aren't satisfied represented as pairs. /// Each Key is the depending module, and each Value is the relationship. /// - public static List> FindUnsatisfiedDepends( - ICollection modules, - HashSet dlls, - IDictionary dlc) + public static modRelList FindUnsatisfiedDepends(ICollection modules, + HashSet dlls, + IDictionary dlc) => (modules?.Where(m => m.depends != null) - .SelectMany(m => m.depends.Select(dep => - new KeyValuePair(m, dep))) - .Where(kvp => !kvp.Value.MatchesAny(modules, dlls, dlc)) - ?? Enumerable.Empty>()) + .SelectMany(m => m.depends.Select(dep => new modRelPair(m, dep, null))) + .Where(kvp => !kvp.Item2.MatchesAny(modules, dlls, dlc)) + ?? Enumerable.Empty()) .ToList(); /// @@ -83,32 +83,25 @@ public static List> FindUnsatis /// List of conflicts represented as pairs. /// Each Key is the depending module, and each Value is the relationship. /// - public static List> FindConflicting( - IEnumerable modules, - HashSet dlls, - IDictionary dlc) - { - var confl = new List>(); - if (modules != null) - { - modules = modules.Memoize(); - foreach (CkanModule m in modules.Where(m => m.conflicts != null)) - { - // Remove self from the list, so we're only comparing to OTHER modules. - // Also remove other versions of self, to avoid conflicts during upgrades. - var others = modules.Where(other => other.identifier != m.identifier).Memoize(); - foreach (RelationshipDescriptor dep in m.conflicts) - { - if (dep.MatchesAny(others, dlls, dlc)) - { - confl.Add(new KeyValuePair(m, dep)); - } - } - } - } - return confl; - } - + private static modRelList FindConflicting(List modules, + HashSet dlls, + IDictionary dlc) + => (modules?.Where(m => m.conflicts != null) + .SelectMany(m => FindConflictingWith( + m, + modules.Where(other => other.identifier != m.identifier) + .ToList(), + dlls, dlc)) + ?? Enumerable.Empty()) + .ToList(); + private static IEnumerable FindConflictingWith(CkanModule module, + List otherMods, + HashSet dlls, + IDictionary dlc) + => module.conflicts.Select(rel => rel.MatchesAny(otherMods, dlls, dlc, out CkanModule other) + ? new modRelPair(module, rel, other) + : null) + .Where(pair => pair != null); } } diff --git a/Core/Types/Kraken.cs b/Core/Types/Kraken.cs index 3b8dfd7b94..dd7971635d 100644 --- a/Core/Types/Kraken.cs +++ b/Core/Types/Kraken.cs @@ -7,6 +7,8 @@ namespace CKAN { + using modRelList = List>; + /// /// Our application exceptions are called Krakens. /// @@ -227,25 +229,23 @@ public override string ToString() public class BadRelationshipsKraken : InconsistentKraken { public BadRelationshipsKraken( - ICollection> depends, - ICollection> conflicts + modRelList depends, + modRelList conflicts ) : base( - (depends?.Select(dep => string.Format(Properties.Resources.KrakenMissingDependency, dep.Key, dep.Value)) + (depends?.Select(dep => string.Format(Properties.Resources.KrakenMissingDependency, dep.Item1, dep.Item2)) ?? new string[] {} ).Concat( - conflicts?.Select(conf => string.Format(Properties.Resources.KrakenConflictsWith, conf.Key, conf.Value)) + conflicts?.Select(conf => string.Format(Properties.Resources.KrakenConflictsWith, conf.Item1, conf.Item2)) ?? new string[] {} ).ToArray() ) { - Depends = depends?.ToList() - ?? new List>(); - Conflicts = conflicts?.ToList() - ?? new List>(); + Depends = depends ?? new modRelList(); + Conflicts = conflicts ?? new modRelList(); } - public readonly List> Depends; - public readonly List> Conflicts; + public readonly modRelList Depends; + public readonly modRelList Conflicts; } /// diff --git a/Tests/Core/Relationships/SanityChecker.cs b/Tests/Core/Relationships/SanityChecker.cs index 97759e5eaf..9de023949d 100644 --- a/Tests/Core/Relationships/SanityChecker.cs +++ b/Tests/Core/Relationships/SanityChecker.cs @@ -129,7 +129,7 @@ public void FindUnsatisfiedDepends() mods.Add(registry.LatestAvailable("CustomBiomes", null)); Assert.Contains( "CustomBiomesData", - CKAN.SanityChecker.FindUnsatisfiedDepends(mods, dlls, dlc).Select(kvp => kvp.Value.ToString()).ToList(), + CKAN.SanityChecker.FindUnsatisfiedDepends(mods, dlls, dlc).Select(kvp => kvp.Item2.ToString()).ToList(), "Missing CustomBiomesData" ); @@ -141,7 +141,7 @@ public void FindUnsatisfiedDepends() Assert.Contains( "CustomBiomes", - CKAN.SanityChecker.FindUnsatisfiedDepends(mods, dlls, dlc).Select(kvp => kvp.Value.ToString()).ToList(), + CKAN.SanityChecker.FindUnsatisfiedDepends(mods, dlls, dlc).Select(kvp => kvp.Item2.ToString()).ToList(), "Missing CustomBiomes" ); } From 0d7725ad6df7f28a534d1989f55a7e15cba55967 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 19:19:11 -0500 Subject: [PATCH 10/22] Make RelationshipResolver.ReasonsFor return empty instead of throwing if no reasons --- Core/Relationships/RelationshipResolver.cs | 14 +++----------- Tests/Core/Relationships/RelationshipResolver.cs | 6 +++--- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index fe12a35539..da0d9240fd 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -603,17 +603,9 @@ public Dictionary ConflictList public bool IsConsistent => !conflicts.Any(); public List ReasonsFor(CkanModule mod) - { - if (mod == null) throw new ArgumentNullException(); - if (!reasons.ContainsKey(mod) && !ModList().Contains(mod)) - { - throw new ArgumentException(string.Format( - Properties.Resources.RelationshipResolverModNotInList, - mod.identifier)); - } - - return reasons[mod]; - } + => reasons.TryGetValue(mod, out List r) + ? r + : new List(); /// /// Indicates whether a module should be considered auto-installed in this change set. diff --git a/Tests/Core/Relationships/RelationshipResolver.cs b/Tests/Core/Relationships/RelationshipResolver.cs index 16ad38174f..68d202a59f 100644 --- a/Tests/Core/Relationships/RelationshipResolver.cs +++ b/Tests/Core/Relationships/RelationshipResolver.cs @@ -856,7 +856,7 @@ public void Constructor_ReverseDependencyConflictsLatest_ChoosesOlderVersion() } [Test] - public void ReasonFor_WithModsNotInList_ThrowsArgumentException() + public void ReasonFor_WithModsNotInList_Empty() { var mod = generator.GeneratorRandomModule(); @@ -869,8 +869,8 @@ public void ReasonFor_WithModsNotInList_ThrowsArgumentException() var relationship_resolver = new RelationshipResolver(list, null, options, registry, null); var mod_not_in_resolver_list = generator.GeneratorRandomModule(); - CollectionAssert.DoesNotContain(relationship_resolver.ModList(),mod_not_in_resolver_list); - Assert.Throws(() => relationship_resolver.ReasonsFor(mod_not_in_resolver_list)); + CollectionAssert.DoesNotContain(relationship_resolver.ModList(), mod_not_in_resolver_list); + Assert.IsEmpty(relationship_resolver.ReasonsFor(mod_not_in_resolver_list)); } } From 2873ca8f4d935fcee7488be2f8dd26cbb7168c63 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 22:15:42 -0500 Subject: [PATCH 11/22] Parallelize creation of GUIMod objects --- GUI/Controls/ManageMods.cs | 41 +++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/GUI/Controls/ManageMods.cs b/GUI/Controls/ManageMods.cs index c3da94eeee..b9f61ef918 100644 --- a/GUI/Controls/ManageMods.cs +++ b/GUI/Controls/ManageMods.cs @@ -1174,24 +1174,29 @@ private bool _UpdateModsList(Dictionary old_modules = null) Main.Instance.Wait.AddLogMessage(Properties.Resources.MainModListLoadingInstalled); var versionCriteria = Main.Instance.CurrentInstance.VersionCriteria(); - var gui_mods = new HashSet(); - gui_mods.UnionWith( - registry.InstalledModules - .Where(instMod => !instMod.Module.IsDLC) - .Select(instMod => new GUIMod(instMod, repoData, registry, versionCriteria, null, - Main.Instance.configuration.HideEpochs, Main.Instance.configuration.HideV))); - Main.Instance.Wait.AddLogMessage(Properties.Resources.MainModListLoadingAvailable); - gui_mods.UnionWith( - registry.CompatibleModules(versionCriteria) - .Where(m => !m.IsDLC) - .Select(m => new GUIMod(m, repoData, registry, versionCriteria, null, - Main.Instance.configuration.HideEpochs, Main.Instance.configuration.HideV))); - Main.Instance.Wait.AddLogMessage(Properties.Resources.MainModListLoadingIncompatible); - gui_mods.UnionWith( - registry.IncompatibleModules(versionCriteria) - .Where(m => !m.IsDLC) - .Select(m => new GUIMod(m, repoData, registry, versionCriteria, true, - Main.Instance.configuration.HideEpochs, Main.Instance.configuration.HideV))); + + var gui_mods = registry.InstalledModules + .AsParallel() + .Where(instMod => !instMod.Module.IsDLC) + .Select(instMod => new GUIMod( + instMod, repoData, registry, versionCriteria, null, + Main.Instance.configuration.HideEpochs, + Main.Instance.configuration.HideV)) + .Concat(registry.CompatibleModules(versionCriteria) + .AsParallel() + .Where(m => !m.IsDLC) + .Select(m => new GUIMod( + m, repoData, registry, versionCriteria, null, + Main.Instance.configuration.HideEpochs, + Main.Instance.configuration.HideV))) + .Concat(registry.IncompatibleModules(versionCriteria) + .AsParallel() + .Where(m => !m.IsDLC) + .Select(m => new GUIMod( + m, repoData, registry, versionCriteria, true, + Main.Instance.configuration.HideEpochs, + Main.Instance.configuration.HideV))) + .ToHashSet(); Main.Instance.Wait.AddLogMessage(Properties.Resources.MainModListPreservingNew); var toNotify = new HashSet(); From 0f6966628c6f787040bb28ed5a91e1df4fe2306e Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 22:16:55 -0500 Subject: [PATCH 12/22] Parallelize creation of grid rows --- GUI/Model/ModList.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/GUI/Model/ModList.cs b/GUI/Model/ModList.cs index 0061448946..ed749e4b5b 100644 --- a/GUI/Model/ModList.cs +++ b/GUI/Model/ModList.cs @@ -272,9 +272,9 @@ public IEnumerable ConstructModList(IReadOnlyCollection { Modules = modules; var changes = mc?.ToList(); - full_list_of_mod_rows = Modules.ToDictionary( - gm => gm.Identifier, - gm => MakeRow(gm, changes, instanceName, game)); + full_list_of_mod_rows = Modules.AsParallel() + .ToDictionary(gm => gm.Identifier, + gm => MakeRow(gm, changes, instanceName, game)); HasAnyInstalled = Modules.Any(m => m.IsInstalled); return full_list_of_mod_rows.Values; } From a6f6e107582c5800caa414bbc4201ede3a66e443 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 22:14:42 -0500 Subject: [PATCH 13/22] Parallelize filtering of rows --- GUI/Controls/ManageMods.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/GUI/Controls/ManageMods.cs b/GUI/Controls/ManageMods.cs index b9f61ef918..2499b8c205 100644 --- a/GUI/Controls/ManageMods.cs +++ b/GUI/Controls/ManageMods.cs @@ -1121,13 +1121,10 @@ private void _UpdateFilters() var registry = RegistryManager.Instance(Main.Instance.CurrentInstance, repoData).registry; ModGrid.Rows.Clear(); - foreach (var row in rows) - { - var mod = ((GUIMod) row.Tag); - var inst = Main.Instance.CurrentInstance; - row.Visible = mainModList.IsVisible(mod, inst.Name, inst.game, registry); - } - + var instName = Main.Instance.CurrentInstance.Name; + var instGame = Main.Instance.CurrentInstance.game; + rows.AsParallel().ForAll(row => + row.Visible = mainModList.IsVisible((GUIMod)row.Tag, instName, instGame, registry)); ApplyHeaderGlyphs(); ModGrid.Rows.AddRange(Sort(rows.Where(row => row.Visible)).ToArray()); From 9e704f2c0ccd7ac85109f90005fe87bd00f3c66e Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 22:39:32 -0500 Subject: [PATCH 14/22] Use collection params instead of converting to collection --- Core/Registry/IRegistryQuerier.cs | 18 +-- Core/Registry/Registry.cs | 11 +- Core/Relationships/RelationshipResolver.cs | 56 +++++---- Core/Repositories/AvailableModule.cs | 9 +- Core/Types/RelationshipDescriptor.cs | 139 ++++++++------------- GUI/Controls/ModInfoTabs/Relationships.cs | 13 +- 6 files changed, 107 insertions(+), 139 deletions(-) diff --git a/Core/Registry/IRegistryQuerier.cs b/Core/Registry/IRegistryQuerier.cs index 695ae53a48..683aa34529 100644 --- a/Core/Registry/IRegistryQuerier.cs +++ b/Core/Registry/IRegistryQuerier.cs @@ -42,7 +42,11 @@ public interface IRegistryQuerier /// If no ksp_version is provided, the latest module for *any* KSP version is returned. /// Throws if asked for a non-existent module. /// - CkanModule LatestAvailable(string identifier, GameVersionCriteria ksp_version, RelationshipDescriptor relationship_descriptor = null); + CkanModule LatestAvailable(string identifier, + GameVersionCriteria ksp_version, + RelationshipDescriptor relationship_descriptor = null, + ICollection installed = null, + ICollection toInstall = null); /// /// Returns the max game version that is compatible with the given mod. @@ -63,13 +67,11 @@ public interface IRegistryQuerier /// Returns an empty list if nothing is available for our system, which includes if no such module exists. /// If no KSP version is provided, the latest module for *any* KSP version is given. /// - List LatestAvailableWithProvides( - string identifier, - GameVersionCriteria ksp_version, - RelationshipDescriptor relationship_descriptor = null, - IEnumerable installed = null, - IEnumerable toInstall = null - ); + List LatestAvailableWithProvides(string identifier, + GameVersionCriteria ksp_version, + RelationshipDescriptor relationship_descriptor = null, + ICollection installed = null, + ICollection toInstall = null); /// /// Checks the sanity of the registry, to ensure that all dependencies are met, diff --git a/Core/Registry/Registry.cs b/Core/Registry/Registry.cs index fc62479203..2d9b5343cd 100644 --- a/Core/Registry/Registry.cs +++ b/Core/Registry/Registry.cs @@ -609,10 +609,13 @@ private AvailableModule[] getAvail(string identifier) public CkanModule LatestAvailable( string identifier, GameVersionCriteria gameVersion, - RelationshipDescriptor relationshipDescriptor = null) + RelationshipDescriptor relationshipDescriptor = null, + ICollection installed = null, + ICollection toInstall = null) { log.DebugFormat("Finding latest available for {0}", identifier); - return getAvail(identifier)?.Select(am => am.Latest(gameVersion, relationshipDescriptor)) + return getAvail(identifier)?.Select(am => am.Latest(gameVersion, relationshipDescriptor, + installed, toInstall)) .Where(m => m != null) .OrderByDescending(m => m.version) .FirstOrDefault(); @@ -793,8 +796,8 @@ public List LatestAvailableWithProvides( string identifier, GameVersionCriteria gameVersion, RelationshipDescriptor relationship_descriptor = null, - IEnumerable installed = null, - IEnumerable toInstall = null) + ICollection installed = null, + ICollection toInstall = null) { if (providers == null) { diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index da0d9240fd..4579e13797 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -328,8 +328,8 @@ private void ResolveStanza(IEnumerable stanza, Selection candidates = descriptor .LatestAvailableWithProvides(registry, versionCrit, new CkanModule[0]) .Where(mod => !modlist.ContainsKey(mod.identifier) - && descriptor1.WithinBounds(mod) - && MightBeInstallable(mod, null, new CkanModule[0])) + && descriptor1.WithinBounds(mod) + && MightBeInstallable(mod)) .ToList(); } @@ -462,6 +462,13 @@ private void Add(CkanModule module, SelectionReason reason) } } + private bool MightBeInstallable(CkanModule module, + CkanModule stanzaSource = null, + ICollection installed = null) + => MightBeInstallable(module, stanzaSource, + installed ?? new List(), + new List()); + /// /// Tests that a module might be able to be installed via checking if dependencies /// exist for current version. @@ -471,43 +478,42 @@ private void Add(CkanModule module, SelectionReason reason) /// The list of installed modules in the current resolver state /// For internal use /// Whether its dependencies are compatible with the current game version - private bool MightBeInstallable(CkanModule module, CkanModule stanzaSource = null, - IEnumerable installed = null, List compatible = null) + private bool MightBeInstallable(CkanModule module, + CkanModule stanzaSource, + ICollection installed, + List parentCompat) { if (module.IsDLC) - return false; - if (module.depends == null) - return true; - if (compatible == null) { - compatible = new List(); + return false; } - else if (compatible.Contains(module.identifier)) + if (module.depends == null) { return true; } // When checking the dependencies we assume that this module is installable // in case a dependent depends on it - compatible.Add(module.identifier); + var compatible = parentCompat.Append(module.identifier).ToList(); + var dlls = registry.InstalledDlls.ToHashSet(); + var dlcs = registry.InstalledDlc; - var toInstall = stanzaSource != null ? new List {stanzaSource} : null; + var toInstall = stanzaSource != null + ? new List { stanzaSource } + : null; - // Get list of lists of dependency choices - var needed = module.depends + // Note, .AsParallel() breaks this, too many threads for recursion + return (parentCompat.Count > 0 ? (IEnumerable)module.depends + : module.depends.AsParallel()) // Skip dependencies satisfied by installed modules - .Where(depend => !depend.MatchesAny(installed, null, null)) + .Where(rel => !rel.MatchesAny(installed, dlls, dlcs)) // ... or by modules that are about to be installed - .Select(depend => depend.LatestAvailableWithProvides(registry, versionCrit, 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, stanzaSource, installed, compatible))); - compatible.Remove(module.identifier); - return installable; + .Select(rel => rel.LatestAvailableWithProvides(registry, versionCrit, + installed, toInstall)) + // We need every dependency to have at least one possible module + .All(need => need.Any(mod => compatible.Contains(mod.identifier) + || MightBeInstallable(mod, stanzaSource, + installed, compatible))); } diff --git a/Core/Repositories/AvailableModule.cs b/Core/Repositories/AvailableModule.cs index a74f072e03..2a3d6a59b6 100644 --- a/Core/Repositories/AvailableModule.cs +++ b/Core/Repositories/AvailableModule.cs @@ -102,8 +102,8 @@ private void Add(CkanModule module) public CkanModule Latest( GameVersionCriteria ksp_version = null, RelationshipDescriptor relationship = null, - IEnumerable installed = null, - IEnumerable toInstall = null) + ICollection installed = null, + ICollection toInstall = null) { IEnumerable modules = module_version.Values.Reverse(); if (relationship != null) @@ -125,9 +125,8 @@ public CkanModule Latest( return modules.FirstOrDefault(); } - private static bool DependsAndConflictsOK(CkanModule module, IEnumerable others) + private static bool DependsAndConflictsOK(CkanModule module, ICollection others) { - others = others.Memoize(); if (module.depends != null) { foreach (RelationshipDescriptor rel in module.depends) @@ -140,7 +139,7 @@ private static bool DependsAndConflictsOK(CkanModule module, IEnumerable m.identifier != module.identifier).Memoize(); + var othersMinusSelf = others.Where(m => m.identifier != module.identifier).ToList(); if (module.conflicts != null) { // Skip self-conflicts (but catch other modules providing self) diff --git a/Core/Types/RelationshipDescriptor.cs b/Core/Types/RelationshipDescriptor.cs index 320e64d528..385875f504 100644 --- a/Core/Types/RelationshipDescriptor.cs +++ b/Core/Types/RelationshipDescriptor.cs @@ -13,27 +13,25 @@ namespace CKAN { public abstract class RelationshipDescriptor : IEquatable { - public bool MatchesAny( - IEnumerable modules, - HashSet dlls, - IDictionary dlc) + public bool MatchesAny(ICollection modules, + HashSet dlls, + IDictionary dlc) => MatchesAny(modules, dlls, dlc, out CkanModule _); - public abstract bool MatchesAny( - IEnumerable modules, - HashSet dlls, - IDictionary dlc, - out CkanModule matched); + public abstract bool MatchesAny(ICollection modules, + HashSet dlls, + IDictionary dlc, + out CkanModule matched); public abstract bool WithinBounds(CkanModule otherModule); public abstract List LatestAvailableWithProvides( - IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable installed = null, - IEnumerable toInstall = null); + IRegistryQuerier registry, GameVersionCriteria crit, ICollection installed = null, + ICollection toInstall = null); public abstract CkanModule ExactMatch( - IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable installed = null, - IEnumerable toInstall = null); + IRegistryQuerier registry, GameVersionCriteria crit, ICollection installed = null, + ICollection toInstall = null); public abstract bool Equals(RelationshipDescriptor other); @@ -71,8 +69,11 @@ public class ModuleRelationshipDescriptor : RelationshipDescriptor public ModuleVersion version; public override bool WithinBounds(CkanModule otherModule) - => otherModule.ProvidesList.Contains(name) - && WithinBounds(otherModule.version); + // See if the real thing is there + => otherModule.identifier == name ? WithinBounds(otherModule.version) + // See if anyone else "provides" the target name + // Note that versions can't be checked for "provides" + : (otherModule.provides?.Contains(name) ?? false); /// /// Returns if the other version satisfies this RelationshipDescriptor. @@ -101,14 +102,11 @@ public bool WithinBounds(ModuleVersion other) /// /// true if any of the modules match this descriptor, false otherwise. /// - public override bool MatchesAny( - IEnumerable modules, - HashSet dlls, - IDictionary dlc, - out CkanModule matched) + public override bool MatchesAny(ICollection modules, + HashSet dlls, + IDictionary dlc, + out CkanModule matched) { - modules = modules?.AsCollection(); - // DLLs are considered to match any version if (dlls != null && dlls.Contains(name)) { @@ -116,59 +114,28 @@ public override bool MatchesAny( return true; } - if (modules != null) + // .AsParallel() makes this slower, too many threads + matched = modules?.FirstOrDefault(WithinBounds); + if (matched != null) { - // See if anyone else "provides" the target name - // Note that versions can't be checked for "provides" clauses - var matches = modules - .Where(m => - m.identifier != name - && m.provides != null - && m.provides.Contains(name)) - .ToList(); - if (matches.Any()) - { - matched = matches.FirstOrDefault(); - return true; - } - - // See if the real thing is there - foreach (var m in modules.Where(m => m.identifier == name)) - { - if (WithinBounds(m)) - { - matched = m; - return true; - } - } - } - - if (dlc != null) - { - foreach (var d in dlc.Where(i => i.Key == name)) - { - if (WithinBounds(d.Value)) - { - matched = null; - return true; - } - } + return true; } - matched = null; - return false; + return dlc != null && dlc.TryGetValue(name, out ModuleVersion dlcVer) + && WithinBounds(dlcVer); } - public override List LatestAvailableWithProvides( - IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable installed = null, - IEnumerable toInstall = null) + public override List LatestAvailableWithProvides(IRegistryQuerier registry, + GameVersionCriteria crit, + ICollection installed = null, + ICollection toInstall = null) => registry.LatestAvailableWithProvides(name, crit, this, installed, toInstall); - public override CkanModule ExactMatch( - IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable installed = null, - IEnumerable toInstall = null) - => registry.LatestAvailableWithProvides(name, crit, this, installed, toInstall) - .FirstOrDefault(mod => mod.identifier == name); + public override CkanModule ExactMatch(IRegistryQuerier registry, + GameVersionCriteria crit, + ICollection installed = null, + ICollection toInstall = null) + => registry.LatestAvailable(name, crit, this, installed, toInstall); public override bool Equals(RelationshipDescriptor other) => Equals(other as ModuleRelationshipDescriptor); @@ -225,36 +192,28 @@ public class AnyOfRelationshipDescriptor : RelationshipDescriptor public override bool WithinBounds(CkanModule otherModule) => any_of?.Any(r => r.WithinBounds(otherModule)) ?? false; - public override bool MatchesAny( - IEnumerable modules, - HashSet dlls, - IDictionary dlc, - out CkanModule matched) + public override bool MatchesAny(ICollection modules, + HashSet dlls, + IDictionary dlc, + out CkanModule matched) { - if (any_of != null) - { - foreach (RelationshipDescriptor rel in any_of) - { - if (rel.MatchesAny(modules, dlls, dlc, out CkanModule whatMatched)) - { - matched = whatMatched; - return true; - } - } - } - matched = null; - return false; + matched = any_of?.AsParallel() + .Select(rel => rel.MatchesAny(modules, dlls, dlc, out CkanModule whatMached) + ? whatMached + : null) + .FirstOrDefault(m => m != null); + return matched != null; } public override List LatestAvailableWithProvides( - IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable installed = null, - IEnumerable toInstall = null) + IRegistryQuerier registry, GameVersionCriteria crit, ICollection installed = null, + ICollection toInstall = null) => any_of?.SelectMany(r => r.LatestAvailableWithProvides(registry, crit, installed, toInstall)).Distinct().ToList(); // Exact match is not possible for any_of public override CkanModule ExactMatch( - IRegistryQuerier registry, GameVersionCriteria crit, IEnumerable installed = null, - IEnumerable toInstall = null) + IRegistryQuerier registry, GameVersionCriteria crit, ICollection installed = null, + ICollection toInstall = null) => null; public override bool Equals(RelationshipDescriptor other) diff --git a/GUI/Controls/ModInfoTabs/Relationships.cs b/GUI/Controls/ModInfoTabs/Relationships.cs index 8549ffbf05..15f78d2363 100644 --- a/GUI/Controls/ModInfoTabs/Relationships.cs +++ b/GUI/Controls/ModInfoTabs/Relationships.cs @@ -265,12 +265,11 @@ private IEnumerable ForwardRelationships(IRegistryQuerier registry, Tr private TreeNode findDependencyShallow(IRegistryQuerier registry, RelationshipDescriptor relDescr, RelationshipType relationship, GameVersionCriteria crit) { // Check if this dependency is installed - if (relDescr.MatchesAny( - registry.InstalledModules.Select(im => im.Module), - new HashSet(registry.InstalledDlls), - // Maybe it's a DLC? - registry.InstalledDlc, - out CkanModule matched)) + if (relDescr.MatchesAny(registry.InstalledModules.Select(im => im.Module).ToList(), + registry.InstalledDlls.ToHashSet(), + // Maybe it's a DLC? + registry.InstalledDlc, + out CkanModule matched)) { return matched != null ? indexedNode(registry, matched, relationship, relDescr, crit) @@ -281,7 +280,7 @@ private TreeNode findDependencyShallow(IRegistryQuerier registry, RelationshipDe List dependencyModules = relDescr.LatestAvailableWithProvides( registry, crit, // Ignore conflicts with installed mods - Enumerable.Empty()); + new List()); if (dependencyModules.Count == 0) { // Nothing found, don't return a node From dd190687e6682d1018582b85e3d14b861c748543 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 23:06:42 -0500 Subject: [PATCH 15/22] Parallelize changeset sorting --- Core/Relationships/RelationshipResolver.cs | 92 +++++++++------------- 1 file changed, 39 insertions(+), 53 deletions(-) diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index 4579e13797..1071a2aa6c 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -519,68 +519,54 @@ private bool MightBeInstallable(CkanModule module, /// /// Returns a list of all modules to install to satisfy the changes required. - /// Each mod should be after its dependencies and before its reverse dependencies. - /// Circular dependencies throw a wrench into that, but for now we treat them the same - /// and let the chips fall where they may. + /// Each mod is after its dependencies and before its reverse dependencies. /// public IEnumerable ModList() + => modlist.Values + .Distinct() + .AsParallel() + // Put user choices at the bottom; .OrderBy(bool) -> false first + .OrderBy(m => ReasonsFor(m).Any(r => r is SelectionReason.UserRequested)) + // Put dependencies before dependers + .ThenByDescending(totalDependers) + // Resolve ties in name order + .ThenBy(m => m.name); + + // The more nodes of the reverse-dependency graph we can paint, the higher up in the list it goes + private int totalDependers(CkanModule module) + => allDependers(module).Count(); + + private static bool AnyRelationship(SelectionReason r) + => r is SelectionReason.Depends + || r is SelectionReason.Recommended + || r is SelectionReason.Suggested; + + private IEnumerable BreadthFirstSearch(IEnumerable startingGroup, + Func, IEnumerable> getNextGroup) { - var sortedDepsFirst = new List(); - try + var found = startingGroup.ToHashSet(); + var toSearch = new Queue(found); + + while (toSearch.Count > 0) { - var modules = modlist.Values - .Distinct() - .OrderBy(m => m.depends?.Count ?? 0) - .ThenBy(m => m.name); - foreach (var module in modules) + var searching = toSearch.Dequeue(); + yield return searching; + foreach (var nextNode in getNextGroup(searching, found)) { - try - { - var reasons = ReasonsFor(module); - var userReq = reasons.Any(r => r is SelectionReason.UserRequested); - var index = reasons - // Ignore non-dependencies - .Where(r => - !(r is SelectionReason.UserRequested - || r is SelectionReason.Installed - || (userReq - // Ignore if depending mod is not user requested - && ReasonsFor(r.Parent).All(parentR => - !(parentR is SelectionReason.UserRequested))))) - .Select(r => sortedDepsFirst.IndexOf(r.Parent)) - // Ignore parents not added yet - .Except(Enumerable.Repeat(-1, 1)) - // Put it at the earliest point where a depender needs it - // (throws into below catch block if empty) - .Min(); - log.DebugFormat("Parent found: {0}, {1}", index, module); - sortedDepsFirst.Insert(index, module); - } - catch (ArgumentException) - { - // ReasonsFor throws this for mods without reasons, just add it to the end - log.DebugFormat("Reasons for parent not found: {0}", module); - sortedDepsFirst.Add(module); - } - catch (InvalidOperationException) - { - // No index, just append - log.DebugFormat("Valid parent not added yet: {0}", module); - sortedDepsFirst.Add(module); - } - } - if (sortedDepsFirst.Count > 0) - { - log.DebugFormat("Returning sorted: {0}", string.Join(", ", sortedDepsFirst)); + found.Add(nextNode); + toSearch.Enqueue(nextNode); } } - catch (Exception ex) - { - log.Debug("Failed to get mod list", ex); - } - return sortedDepsFirst; } + private IEnumerable allDependers(CkanModule module, + Func followReason = null) + => BreadthFirstSearch(Enumerable.Repeat(module, 1), + (searching, found) => + ReasonsFor(searching).Where(followReason ?? AnyRelationship) + .Select(r => r.Parent) + .Except(found)); + /// /// Returns a dictionary consisting of KeyValuePairs containing conflicting mods. /// From 97ba9b03d365b1ad2edb8e8180fb5b2faab58137 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Fri, 6 Oct 2023 13:05:56 -0500 Subject: [PATCH 16/22] Parallelize JSON parsing of downloaded repo data --- Core/Extensions/EnumerableExtensions.cs | 23 ++- Core/Repositories/RepositoryData.cs | 185 +++++++++++++++--------- 2 files changed, 141 insertions(+), 67 deletions(-) diff --git a/Core/Extensions/EnumerableExtensions.cs b/Core/Extensions/EnumerableExtensions.cs index 5f17794157..3c9974e18c 100644 --- a/Core/Extensions/EnumerableExtensions.cs +++ b/Core/Extensions/EnumerableExtensions.cs @@ -141,6 +141,7 @@ public static IEnumerable TraverseNodes(this T start, Func getNext) } #if NET45 + /// /// Make pairs out of the elements of two sequences /// @@ -149,7 +150,6 @@ public static IEnumerable TraverseNodes(this T start, Func getNext) /// Sequence of pairs of one element from seq1 and one from seq2 public static IEnumerable> Zip(this IEnumerable seq1, IEnumerable seq2) => seq1.Zip(seq2, (item1, item2) => new Tuple(item1, item2)); -#endif /// /// Enable a `foreach` over a sequence of tuples @@ -179,6 +179,24 @@ public static void Deconstruct(this Tuple tuple, item3 = tuple.Item3; } + /// + /// Enable a `foreach` over a sequence of tuples + /// + /// A tuple to deconstruct + /// Set to the first value from the tuple + /// Set to the second value from the tuple + public static void Deconstruct(this Tuple tuple, + out T1 item1, + out T2 item2, + out T3 item3, + out T4 item4) + { + item1 = tuple.Item1; + item2 = tuple.Item2; + item3 = tuple.Item3; + item4 = tuple.Item4; + } + /// /// Enable a `foreach` over a sequence of key value pairs /// @@ -190,6 +208,9 @@ public static void Deconstruct(this KeyValuePair kvp, out T1 key key = kvp.Key; val = kvp.Value; } + +#endif + } /// diff --git a/Core/Repositories/RepositoryData.cs b/Core/Repositories/RepositoryData.cs index 5f71e2e0c2..3352ff3daf 100644 --- a/Core/Repositories/RepositoryData.cs +++ b/Core/Repositories/RepositoryData.cs @@ -16,9 +16,21 @@ using CKAN.Versioning; using CKAN.Games; +using CKAN.Extensions; namespace CKAN { + using ArchiveEntry = Tuple, + GameVersion[], + Repository[], + long>; + + using ArchiveList = Tuple, + SortedDictionary, + GameVersion[], + Repository[]>; + /// /// Represents everything we retrieve from one metadata repository /// @@ -51,6 +63,17 @@ public class RepositoryData [JsonProperty("repositories", NullValueHandling = NullValueHandling.Ignore)] public readonly Repository[] Repositories; + private RepositoryData(Dictionary modules, + SortedDictionary counts, + GameVersion[] versions, + Repository[] repos) + { + AvailableModules = modules; + DownloadCounts = counts; + KnownGameVersions = versions; + Repositories = repos; + } + /// /// Instantiate a repo data object /// @@ -62,13 +85,18 @@ public RepositoryData(IEnumerable modules, SortedDictionary counts, IEnumerable versions, IEnumerable repos) + : this(modules?.GroupBy(m => m.identifier) + .ToDictionary(grp => grp.Key, + grp => new AvailableModule(grp.Key, grp)), + counts ?? new SortedDictionary(), + (versions ?? Enumerable.Empty()).ToArray(), + (repos ?? Enumerable.Empty()).ToArray()) + { + } + + [JsonConstructor] + private RepositoryData() { - AvailableModules = modules?.GroupBy(m => m.identifier) - .ToDictionary(grp => grp.Key, - grp => new AvailableModule(grp.Key, grp)); - DownloadCounts = counts ?? new SortedDictionary(); - KnownGameVersions = (versions ?? Enumerable.Empty()).ToArray(); - Repositories = (repos ?? Enumerable.Empty()).ToArray(); } /// @@ -160,19 +188,31 @@ private static RepositoryData FromTarGz(string path, IGame game, IProgress using (var gzipStream = new GZipInputStream(progressStream)) using (var tarStream = new TarInputStream(gzipStream, Encoding.UTF8)) { - var modules = new List(); - SortedDictionary counts = null; - GameVersion[] versions = null; - Repository[] repos = null; + (List modules, + SortedDictionary counts, + GameVersion[] versions, + Repository[] repos) = AggregateArchiveEntries(archiveEntriesFromTar(tarStream, game)); + return new RepositoryData(modules, counts, versions, repos); + } + } + + private static ParallelQuery archiveEntriesFromTar(TarInputStream tarStream, IGame game) + => Partitioner.Create(getTarEntries(tarStream)) + .AsParallel() + .Select(tuple => getArchiveEntry(tuple.Item1.Name, + () => tuple.Item2, + game, + tarStream.Position)); - TarEntry entry; - while ((entry = tarStream.GetNextEntry()) != null) + private static IEnumerable> getTarEntries(TarInputStream tarStream) + { + TarEntry entry; + while ((entry = tarStream.GetNextEntry()) != null) + { + if (!entry.Name.EndsWith(".frozen")) { - ProcessFileEntry(entry.Name, () => tarStreamString(tarStream, entry), - game, inputStream.Position, progress, - ref modules, ref counts, ref versions, ref repos); + yield return new Tuple(entry, tarStreamString(tarStream, entry)); } - return new RepositoryData(modules, counts, versions, repos); } } @@ -205,61 +245,74 @@ private static RepositoryData FromZip(string path, IGame game, IProgress p using (var progressStream = new ReadProgressStream(inputStream, progress)) using (var zipfile = new ZipFile(progressStream)) { - var modules = new List(); - SortedDictionary counts = null; - GameVersion[] versions = null; - Repository[] repos = null; - - foreach (ZipEntry entry in zipfile) - { - ProcessFileEntry(entry.Name, () => new StreamReader(zipfile.GetInputStream(entry)).ReadToEnd(), - game, entry.Offset, progress, - ref modules, ref counts, ref versions, ref repos); - } + (List modules, + SortedDictionary counts, + GameVersion[] versions, + Repository[] repos) = AggregateArchiveEntries(archiveEntriesFromZip(zipfile, game)); zipfile.Close(); return new RepositoryData(modules, counts, versions, repos); } } - private static void ProcessFileEntry(string filename, - Func getContents, - IGame game, - long position, - IProgress progress, - ref List modules, - ref SortedDictionary counts, - ref GameVersion[] versions, - ref Repository[] repos) - { - if (filename.EndsWith("download_counts.json")) - { - counts = JsonConvert.DeserializeObject>(getContents()); - } - else if (filename.EndsWith("builds.json")) - { - versions = game.ParseBuildsJson(JToken.Parse(getContents())); - } - else if (filename.EndsWith("repositories.json")) - { - repos = JObject.Parse(getContents())["repositories"] - .ToObject(); - } - else if (filename.EndsWith(".ckan")) - { - log.DebugFormat("Reading CKAN data from {0}", filename); - CkanModule module = ProcessRegistryMetadataFromJSON(getContents(), filename); - if (module != null) - { - modules.Add(module); - } - } - else - { - // Skip things we don't want - log.DebugFormat("Skipping archive entry {0}", filename); - } - progress?.Report(position); - } + private static ParallelQuery archiveEntriesFromZip(ZipFile zipfile, IGame game) + => zipfile.Cast() + .ToArray() + .AsParallel() + .Select(entry => getArchiveEntry( + entry.Name, + () => new StreamReader(zipfile.GetInputStream(entry)).ReadToEnd(), + game, + entry.Offset)); + + private static ArchiveList AggregateArchiveEntries(ParallelQuery entries) + => entries.Aggregate(new ArchiveList(new List(), null, null, null), + (subtotal, item) => + item == null + ? subtotal + : new ArchiveList( + item.Item1 == null + ? subtotal.Item1 + : subtotal.Item1.Append(item.Item1).ToList(), + subtotal.Item2 ?? item.Item2, + subtotal.Item3 ?? item.Item3, + subtotal.Item4 ?? item.Item4), + (total, subtotal) + => new ArchiveList(total.Item1.Concat(subtotal.Item1).ToList(), + total.Item2 ?? subtotal.Item2, + total.Item3 ?? subtotal.Item3, + total.Item4 ?? subtotal.Item4), + total => total); + + private static ArchiveEntry getArchiveEntry(string filename, + Func getContents, + IGame game, + long position) + => filename.EndsWith(".ckan") + ? new ArchiveEntry(ProcessRegistryMetadataFromJSON(getContents(), filename), + null, + null, + null, + position) + : filename.EndsWith("download_counts.json") + ? new ArchiveEntry(null, + JsonConvert.DeserializeObject>(getContents()), + null, + null, + position) + : filename.EndsWith("builds.json") + ? new ArchiveEntry(null, + null, + game.ParseBuildsJson(JToken.Parse(getContents())), + null, + position) + : filename.EndsWith("repositories.json") + ? new ArchiveEntry(null, + null, + null, + JObject.Parse(getContents())["repositories"] + .ToObject(), + position) + : null; private static CkanModule ProcessRegistryMetadataFromJSON(string metadata, string filename) { From d4971195c59982672b2b8b87179789114638f57b Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 23:22:23 -0500 Subject: [PATCH 17/22] Indicate which user-selected mods are affected by a conflict --- Core/ModuleInstaller.cs | 4 +- Core/Properties/Resources.Designer.cs | 3 + Core/Properties/Resources.resx | 1 + Core/Relationships/RelationshipResolver.cs | 83 ++++++++++++------- GUI/Controls/ManageMods.cs | 7 +- GUI/Model/ModList.cs | 7 +- .../Relationships/RelationshipResolver.cs | 9 +- 7 files changed, 75 insertions(+), 39 deletions(-) diff --git a/Core/ModuleInstaller.cs b/Core/ModuleInstaller.cs index 7035148e14..2feac6f5c9 100644 --- a/Core/ModuleInstaller.cs +++ b/Core/ModuleInstaller.cs @@ -1497,9 +1497,7 @@ IRegistryQuerier registry } else { - string problems = resolver.ConflictList.Values - .Aggregate((a, b) => $"{a}, {b}"); - log.Debug($"Can't install {request}: {problems}"); + log.DebugFormat("Can't install {0}: {1}", request, string.Join("; ", resolver.ConflictDescriptions)); return false; } } diff --git a/Core/Properties/Resources.Designer.cs b/Core/Properties/Resources.Designer.cs index a93d0ab9a6..e599e5cf64 100644 --- a/Core/Properties/Resources.Designer.cs +++ b/Core/Properties/Resources.Designer.cs @@ -554,6 +554,9 @@ internal static string KrakenNotEnoughSpace { internal static string RelationshipResolverConflictsWith { get { return (string)(ResourceManager.GetObject("RelationshipResolverConflictsWith", resourceCulture)); } } + internal static string RelationshipResolverConflictingModDescription { + get { return (string)(ResourceManager.GetObject("RelationshipResolverConflictingModDescription", resourceCulture)); } + } internal static string RelationshipResolverRequiredButResolver { get { return (string)(ResourceManager.GetObject("RelationshipResolverRequiredButResolver", resourceCulture)); } } diff --git a/Core/Properties/Resources.resx b/Core/Properties/Resources.resx index ad7957c401..c209ce6d10 100644 --- a/Core/Properties/Resources.resx +++ b/Core/Properties/Resources.resx @@ -288,6 +288,7 @@ Need to store {3} to {1}, but only {2} is available! Free up space on that device or change your settings to use another location. {0} conflicts with {1} + {0} (needed for: {1}) {0} required, but an incompatible version is in the resolver {0} required, but an incompatible version is installed an unmanaged DLL or DLC diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index 1071a2aa6c..3411789289 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -97,17 +97,17 @@ public static RelationshipResolverOptions DependsOnlyOpts() /// Add modules to consideration of the relationship resolver. /// /// Modules to attempt to install - private void AddModulesToInstall(IEnumerable modules) + private void AddModulesToInstall(CkanModule[] modules) { - //Count may need to do a full enumeration. Might as well convert to array - var ckan_modules = modules as CkanModule[] ?? modules.ToArray(); - log.DebugFormat("Processing relationships for {0} modules", ckan_modules.Count()); + log.DebugFormat("Processing relationships for {0} modules", modules.Length); // Start by figuring out what versions we're installing, and then // adding them to the list. This *must* be pre-populated with all // user-specified modules, as they may be supplying things that provide // virtual packages. - foreach (CkanModule module in ckan_modules) + + // This part can't be parallel, to preserve order of processing + foreach (CkanModule module in modules) { log.DebugFormat("Preparing to resolve relationships for {0} {1}", module.identifier, module.version); @@ -235,15 +235,16 @@ private void Resolve(CkanModule module, /// /// See RelationshipResolverOptions for further adjustments that can be made. /// - private void ResolveStanza(IEnumerable stanza, SelectionReason reason, - RelationshipResolverOptions options, bool soft_resolve = false, - IEnumerable old_stanza = null) + private void ResolveStanza(List stanza, + SelectionReason reason, + RelationshipResolverOptions options, + bool soft_resolve = false, + IEnumerable old_stanza = null) { if (stanza == null) { return; } - stanza = stanza.Memoize(); foreach (RelationshipDescriptor descriptor in stanza) { @@ -402,12 +403,25 @@ private void ResolveStanza(IEnumerable stanza, Selection { throw new InconsistentKraken(string.Format( Properties.Resources.RelationshipResolverConflictsWith, - conflicting_mod, candidate)); + conflictingModDescription(conflicting_mod, null), + conflictingModDescription(candidate, reason.Parent))); } } } } + private string conflictingModDescription(CkanModule mod, CkanModule parent) + => mod == null + ? Properties.Resources.RelationshipResolverAnUnmanaged + : parent == null && ReasonsFor(mod).Any(r => r is SelectionReason.UserRequested + || r is SelectionReason.Installed) + // No parenthetical needed if it's user requested + ? mod.ToString() + // Explain why we're looking at this mod + : string.Format(Properties.Resources.RelationshipResolverConflictingModDescription, + mod.ToString(), + string.Join(", ", UserReasonsFor(parent ?? mod).Select(m => m.ToString()))); + /// /// Adds the specified module to the list of modules we're installing. /// This also adds its provides list to what we have available. @@ -516,7 +530,6 @@ private bool MightBeInstallable(CkanModule module, installed, compatible))); } - /// /// Returns a list of all modules to install to satisfy the changes required. /// Each mod is after its dependencies and before its reverse dependencies. @@ -569,28 +582,35 @@ private IEnumerable allDependers(CkanModule module, /// /// Returns a dictionary consisting of KeyValuePairs containing conflicting mods. + /// The keys are the mods that the user chose that led to the conflict being in the list! + /// Use this for coloring/labelling rows, use ConflictDescriptions for reporting the conflicts to the user. /// public Dictionary ConflictList - { - get - { - return conflicts - .GroupBy(kvp => kvp.Key) - .ToDictionary( - group => group.Key, - group => group.Select(kvp => kvp.Value) - .Where(v => v != null) - .Distinct()) - .ToDictionary( - kvp => kvp.Key, - kvp => string.Format( + => conflicts + .SelectMany(kvp => UserReasonsFor(kvp.Key).Select(k => new KeyValuePair(k, kvp))) + .GroupBy(kvp => kvp.Key) + .ToDictionary(group => group.Key, + group => string.Join(", ", group.Select(kvp => kvp.Value) + .Distinct() + .Select(origKvp => string.Format( + Properties.Resources.RelationshipResolverConflictsWith, + conflictingModDescription(origKvp.Key, null), + conflictingModDescription(origKvp.Value, null))))); + + /// + /// Return descriptions of all the current conflicts. + /// Avoids duplicates and explains why dependencies are in the list. + /// Use for reporting the conflicts to the user, use ConflictsList for coloring rows. + /// + /// + public IEnumerable ConflictDescriptions + => conflicts.Where(kvp => kvp.Value == null + // Pick the pair with the least distantly selected one first + || totalDependers(kvp.Key) < totalDependers(kvp.Value)) + .Select(kvp => string.Format( Properties.Resources.RelationshipResolverConflictsWith, - kvp.Key, ( - kvp.Value.Count() == 0 - ? Properties.Resources.RelationshipResolverAnUnmanaged - : string.Join(", ", kvp.Value)))); - } - } + conflictingModDescription(kvp.Key, null), + conflictingModDescription(kvp.Value, null))); public bool IsConsistent => !conflicts.Any(); @@ -599,6 +619,9 @@ public List ReasonsFor(CkanModule mod) ? r : new List(); + public IEnumerable UserReasonsFor(CkanModule mod) + => allDependers(mod).Intersect(user_requested_mods); + /// /// Indicates whether a module should be considered auto-installed in this change set. /// A mod is auto-installed if it is in the list because it's a dependency diff --git a/GUI/Controls/ManageMods.cs b/GUI/Controls/ManageMods.cs index 2499b8c205..9b9d9d276a 100644 --- a/GUI/Controls/ManageMods.cs +++ b/GUI/Controls/ManageMods.cs @@ -1685,7 +1685,12 @@ public void UpdateChangeSetAndConflicts(GameInstance inst, IRegistryQuerier regi item => item.Value); if (new_conflicts.Count > 0) { - Main.Instance.AddStatusMessage(string.Join(";", new_conflicts.Values)); + Main.Instance.AddStatusMessage(string.Join("; ", tuple.Item3)); + } + else + { + // Clear the conflict area if no conflicts + Main.Instance.AddStatusMessage(""); } } catch (DependencyNotSatisfiedKraken k) diff --git a/GUI/Model/ModList.cs b/GUI/Model/ModList.cs index ed749e4b5b..70ad6a1c21 100644 --- a/GUI/Model/ModList.cs +++ b/GUI/Model/ModList.cs @@ -121,7 +121,7 @@ public static SavedSearch FilterToSavedSearch(GUIModFilter filter, ModuleTag tag /// /// /// The version of the current game instance - public Tuple, Dictionary> ComputeFullChangeSetFromUserChangeSet( + public Tuple, Dictionary, List> ComputeFullChangeSetFromUserChangeSet( IRegistryQuerier registry, HashSet changeSet, GameVersionCriteria version) { var modules_to_install = new List(); @@ -192,7 +192,7 @@ public Tuple, Dictionary> ComputeFull conflictOptions, registry, version); // Replace Install entries in changeset with the ones from resolver to get all the reasons - return new Tuple, Dictionary>( + return new Tuple, Dictionary, List>( changeSet.Where(ch => !(ch.ChangeType is GUIModChangeType.Install)) .OrderBy(ch => ch.Mod.identifier) .Union(resolver.ModList() @@ -200,7 +200,8 @@ public Tuple, Dictionary> ComputeFull .Except(upgrading) .Where(m => !m.IsMetapackage) .Select(m => new ModChange(m, GUIModChangeType.Install, resolver.ReasonsFor(m)))), - resolver.ConflictList); + resolver.ConflictList, + resolver.ConflictDescriptions.ToList()); } /// diff --git a/Tests/Core/Relationships/RelationshipResolver.cs b/Tests/Core/Relationships/RelationshipResolver.cs index 68d202a59f..da01c83447 100644 --- a/Tests/Core/Relationships/RelationshipResolver.cs +++ b/Tests/Core/Relationships/RelationshipResolver.cs @@ -1051,10 +1051,12 @@ public void UninstallingConflictingModule_InstallingRecursiveDependencies_Resolv modulesToRemove = new List(); options.proceed_with_inconsistencies = false; - Assert.Throws(() => + var exception = Assert.Throws(() => { resolver = new RelationshipResolver(modulesToInstall, modulesToRemove, options, registry, null); }); + Assert.AreEqual($"{avp} conflicts with {eveDefaultConfig}", + exception.ShortDescription); // Scenario 2 - Try installing AVP, expect no exception for proceed_with_inconsistencies=true, but a conflict list @@ -1064,7 +1066,10 @@ public void UninstallingConflictingModule_InstallingRecursiveDependencies_Resolv { resolver = new RelationshipResolver(modulesToInstall, modulesToRemove, options, registry, null); }); - CollectionAssert.AreEquivalent(new List {avp, eveDefaultConfig}, resolver.ConflictList.Keys); + CollectionAssert.AreEquivalent(modulesToInstall, + resolver.ConflictList.Keys); + CollectionAssert.AreEquivalent(new List {$"{avp} conflicts with {eveDefaultConfig}"}, + resolver.ConflictList.Values); // Scenario 3 - Try uninstalling eveDefaultConfig and installing avp, should work and result in no conflicts From ff1fdec67a1d7de9cf4032fcda875d26df96eb1b Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 27 Sep 2023 20:58:34 -0500 Subject: [PATCH 18/22] Indicate when a removal is because a dependency was removed --- Core/Properties/Resources.Designer.cs | 3 +++ Core/Properties/Resources.resx | 1 + Core/Relationships/RelationshipResolver.cs | 6 ++++++ GUI/Model/ModList.cs | 3 ++- 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Core/Properties/Resources.Designer.cs b/Core/Properties/Resources.Designer.cs index e599e5cf64..4fd2327533 100644 --- a/Core/Properties/Resources.Designer.cs +++ b/Core/Properties/Resources.Designer.cs @@ -578,6 +578,9 @@ internal static string RelationshipResolverInstalledReason { internal static string RelationshipResolverUserReason { get { return (string)(ResourceManager.GetObject("RelationshipResolverUserReason", resourceCulture)); } } + internal static string RelationshipResolverDependencyRemoved { + get { return (string)(ResourceManager.GetObject("RelationshipResolverDependencyRemoved", resourceCulture)); } + } internal static string RelationshipResolverNoLongerUsedReason { get { return (string)(ResourceManager.GetObject("RelationshipResolverNoLongerUsedReason", resourceCulture)); } } diff --git a/Core/Properties/Resources.resx b/Core/Properties/Resources.resx index c209ce6d10..69990be6ef 100644 --- a/Core/Properties/Resources.resx +++ b/Core/Properties/Resources.resx @@ -296,6 +296,7 @@ Free up space on that device or change your settings to use another location. Mod {0} is not in the list Currently installed Requested by user + Dependency removed Auto-installed, depending modules removed Replacing {0} Suggested by {0} diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index 3411789289..19900c424f 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -776,6 +776,12 @@ public override string Reason => Properties.Resources.RelationshipResolverUserReason; } + public class DependencyRemoved : SelectionReason + { + public override string Reason + => Properties.Resources.RelationshipResolverDependencyRemoved; + } + public class NoLongerUsed : SelectionReason { public override string Reason diff --git a/GUI/Model/ModList.cs b/GUI/Model/ModList.cs index 70ad6a1c21..03835b283e 100644 --- a/GUI/Model/ModList.cs +++ b/GUI/Model/ModList.cs @@ -174,7 +174,8 @@ public Tuple, Dictionary, List Date: Tue, 26 Sep 2023 14:25:14 -0500 Subject: [PATCH 19/22] Fix relationships tab stop sign in recommendations --- GUI/Controls/ManageMods.cs | 4 +++- GUI/Main/Main.cs | 1 + GUI/Main/MainHistory.cs | 7 ++++++- GUI/Model/GUIMod.cs | 18 +++++++++--------- Tests/GUI/Model/GUIMod.cs | 9 ++++++--- Tests/GUI/Model/ModList.cs | 12 ++++++++---- 6 files changed, 33 insertions(+), 18 deletions(-) diff --git a/GUI/Controls/ManageMods.cs b/GUI/Controls/ManageMods.cs index 9b9d9d276a..22ea2b39b2 100644 --- a/GUI/Controls/ManageMods.cs +++ b/GUI/Controls/ManageMods.cs @@ -1681,7 +1681,9 @@ public void UpdateChangeSetAndConflicts(GameInstance inst, IRegistryQuerier regi var tuple = mainModList.ComputeFullChangeSetFromUserChangeSet(registry, user_change_set, gameVersion); full_change_set = tuple.Item1.ToList(); new_conflicts = tuple.Item2.ToDictionary( - item => new GUIMod(item.Key, repoData, registry, gameVersion), + item => new GUIMod(item.Key, repoData, registry, gameVersion, null, + Main.Instance.configuration.HideEpochs, + Main.Instance.configuration.HideV), item => item.Value); if (new_conflicts.Count > 0) { diff --git a/GUI/Main/Main.cs b/GUI/Main/Main.cs index 5ba7fb9dc6..8f3e616be9 100644 --- a/GUI/Main/Main.cs +++ b/GUI/Main/Main.cs @@ -770,6 +770,7 @@ private void ShowSelectionModInfo(ListView.SelectedListViewItemCollection select repoData, RegistryManager.Instance(CurrentInstance, repoData).registry, CurrentInstance.VersionCriteria(), + null, configuration.HideEpochs, configuration.HideV); } diff --git a/GUI/Main/MainHistory.cs b/GUI/Main/MainHistory.cs index c47edb3926..97486744d0 100644 --- a/GUI/Main/MainHistory.cs +++ b/GUI/Main/MainHistory.cs @@ -41,7 +41,12 @@ private void InstallationHistory_Done() private void InstallationHistory_OnSelectedModuleChanged(CkanModule m) { - ActiveModInfo = new GUIMod(m, repoData, RegistryManager.Instance(CurrentInstance, repoData).registry, CurrentInstance.VersionCriteria()); + ActiveModInfo = new GUIMod(m, repoData, + RegistryManager.Instance(CurrentInstance, repoData).registry, + CurrentInstance.VersionCriteria(), + null, + configuration.HideEpochs, + configuration.HideV); } } diff --git a/GUI/Model/GUIMod.cs b/GUI/Model/GUIMod.cs index ced5a1faff..52c390a915 100644 --- a/GUI/Model/GUIMod.cs +++ b/GUI/Model/GUIMod.cs @@ -133,9 +133,9 @@ public GUIMod(InstalledModule instMod, RepositoryDataManager repoDataMgr, IRegistryQuerier registry, GameVersionCriteria current_game_version, - bool? incompatible = null, - bool hideEpochs = false, - bool hideV = false) + bool? incompatible, + bool hideEpochs, + bool hideV) : this(instMod.Module, repoDataMgr, registry, current_game_version, incompatible, hideEpochs, hideV) { IsInstalled = true; @@ -166,9 +166,9 @@ public GUIMod(CkanModule mod, RepositoryDataManager repoDataMgr, IRegistryQuerier registry, GameVersionCriteria current_game_version, - bool? incompatible = null, - bool hideEpochs = false, - bool hideV = false) + bool? incompatible, + bool hideEpochs, + bool hideV) : this(mod.identifier, repoDataMgr, registry, current_game_version, incompatible, hideEpochs, hideV) { Mod = mod; @@ -210,9 +210,9 @@ public GUIMod(string identifier, RepositoryDataManager repoDataMgr, IRegistryQuerier registry, GameVersionCriteria current_game_version, - bool? incompatible = null, - bool hideEpochs = false, - bool hideV = false) + bool? incompatible, + bool hideEpochs, + bool hideV) { Identifier = identifier; IsAutodetected = registry.IsAutodetected(identifier); diff --git a/Tests/GUI/Model/GUIMod.cs b/Tests/GUI/Model/GUIMod.cs index 0ff61401f0..31145be7e5 100644 --- a/Tests/GUI/Model/GUIMod.cs +++ b/Tests/GUI/Model/GUIMod.cs @@ -32,7 +32,8 @@ public void NewGuiModsAreNotSelectedForUpgrade() var registry = new Registry(repoData.Manager, repo.repo); var ckan_mod = registry.GetModuleByVersion("kOS", "0.14"); - var mod = new GUIMod(ckan_mod, repoData.Manager, registry, manager.CurrentInstance.VersionCriteria()); + var mod = new GUIMod(ckan_mod, repoData.Manager, registry, manager.CurrentInstance.VersionCriteria(), + null, false, false); Assert.False(mod.IsUpgradeChecked); } } @@ -58,7 +59,8 @@ public void HasUpdateReturnsTrueWhenUpdateAvailable() registry.RegisterModule(old_version, Enumerable.Empty(), null, false); - var mod = new GUIMod(old_version, repoData.Manager, registry, tidy.KSP.VersionCriteria()); + var mod = new GUIMod(old_version, repoData.Manager, registry, tidy.KSP.VersionCriteria(), + null, false, false); Assert.True(mod.HasUpdate); } } @@ -91,7 +93,8 @@ public void GameCompatibility_OutOfOrderGameVersions_TrueMaxVersion() CkanModule prevVersion = registry.GetModuleByVersion("OutOfOrderMod", "1.1.0"); // Act - GUIMod m = new GUIMod(mainVersion, repoData.Manager, registry, tidy.KSP.VersionCriteria(), false); + GUIMod m = new GUIMod(mainVersion, repoData.Manager, registry, tidy.KSP.VersionCriteria(), + null, false, false); // Assert Assert.AreEqual("1.4.2", m.GameCompatibilityVersion.ToString()); diff --git a/Tests/GUI/Model/ModList.cs b/Tests/GUI/Model/ModList.cs index 9ed8fd62ce..606eec54e9 100644 --- a/Tests/GUI/Model/ModList.cs +++ b/Tests/GUI/Model/ModList.cs @@ -46,7 +46,8 @@ public void IsVisible_WithAllAndNoNameFilter_ReturnsTrueForCompatible() var item = new ModList(); Assert.That(item.IsVisible( - new GUIMod(ckan_mod, repoData.Manager, registry, manager.CurrentInstance.VersionCriteria()), + new GUIMod(ckan_mod, repoData.Manager, registry, manager.CurrentInstance.VersionCriteria(), + null, false, false), manager.CurrentInstance.Name, manager.CurrentInstance.game, registry)); @@ -83,8 +84,10 @@ public void ConstructModList_NumberOfRows_IsEqualToNumberOfMods() var mod_list = main_mod_list.ConstructModList( new List { - new GUIMod(TestData.FireSpitterModule(), repoData.Manager, registry, manager.CurrentInstance.VersionCriteria()), - new GUIMod(TestData.kOS_014_module(), repoData.Manager, registry, manager.CurrentInstance.VersionCriteria()) + new GUIMod(TestData.FireSpitterModule(), repoData.Manager, registry, manager.CurrentInstance.VersionCriteria(), + null, false, false), + new GUIMod(TestData.kOS_014_module(), repoData.Manager, registry, manager.CurrentInstance.VersionCriteria(), + null, false, false) }, manager.CurrentInstance.Name, manager.CurrentInstance.game @@ -182,7 +185,8 @@ public void InstallAndSortByCompat_WithAnyCompat_NoCrash() Assert.IsNotNull(modList); var modules = repoData.Manager.GetAllAvailableModules(Enumerable.Repeat(repo.repo, 1)) - .Select(mod => new GUIMod(mod.Latest(), repoData.Manager, registry, instance.KSP.VersionCriteria())) + .Select(mod => new GUIMod(mod.Latest(), repoData.Manager, registry, instance.KSP.VersionCriteria(), + null, false, false)) .ToList(); listGui.Rows.AddRange(modList.ConstructModList(modules, null, instance.KSP.game).ToArray()); From 07a545ea04b3bacf07a5ef4bf2e40c71bb89d541 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Thu, 28 Sep 2023 09:17:12 -0500 Subject: [PATCH 20/22] Obey suppress_recommendations for CmdLine installs --- Core/Relationships/RelationshipResolver.cs | 37 +++++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index 19900c424f..ebc8d4ea99 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -193,8 +193,7 @@ private void Resolve(CkanModule module, // Even though we may resolve top-level suggests for our module, // we don't install suggestions all the way down unless with_all_suggests // is true. - var sub_options = (RelationshipResolverOptions) options.Clone(); - sub_options.with_suggests = false; + var sub_options = options.WithoutSuggestions(); old_stanza = old_stanza?.Memoize(); @@ -239,13 +238,14 @@ private void ResolveStanza(List stanza, SelectionReason reason, RelationshipResolverOptions options, bool soft_resolve = false, - IEnumerable old_stanza = null) + IEnumerable old_stanza = null) { if (stanza == null) { return; } + var orig_options = options; foreach (RelationshipDescriptor descriptor in stanza) { log.DebugFormat("Considering {0}", descriptor.ToString()); @@ -255,6 +255,7 @@ private void ResolveStanza(List stanza, log.DebugFormat("Skipping {0} because get_recommenders option is set"); continue; } + options = orig_options.OptionsFor(descriptor); // If we already have this dependency covered, // resolve its relationships if we haven't already. @@ -676,7 +677,7 @@ private void AddReason(CkanModule module, SelectionReason reason) // TODO: It would be lovely to get rid of the `without` fields, // and replace them with `with` fields. Humans suck at inverting // cases in their heads. - public class RelationshipResolverOptions : ICloneable + public class RelationshipResolverOptions { /// /// If true, add recommended mods, and their recommendations. @@ -732,7 +733,33 @@ public class RelationshipResolverOptions : ICloneable /// public bool get_recommenders = false; - public object Clone() => MemberwiseClone(); + public RelationshipResolverOptions OptionsFor(RelationshipDescriptor descr) + => descr.suppress_recommendations ? WithoutRecommendations() : this; + + private RelationshipResolverOptions WithoutRecommendations() + { + if (with_recommends || with_all_suggests || with_suggests) + { + var newOptions = (RelationshipResolverOptions)MemberwiseClone(); + newOptions.with_recommends = false; + newOptions.with_all_suggests = false; + newOptions.with_suggests = false; + return newOptions; + } + return this; + } + + public RelationshipResolverOptions WithoutSuggestions() + { + if (with_suggests) + { + var newOptions = (RelationshipResolverOptions)MemberwiseClone(); + newOptions.with_suggests = false; + return newOptions; + } + return this; + } + } /// From ff7ec5468eba05f1c3e18a1e39121492e1266b8d Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Tue, 3 Oct 2023 17:40:22 -0500 Subject: [PATCH 21/22] Improve recommendations performance --- Core/ModuleInstaller.cs | 230 +++++------------- Core/Registry/IRegistryQuerier.cs | 2 +- Core/Relationships/RelationshipResolver.cs | 191 ++++++++++----- .../ChooseRecommendedMods.Designer.cs | 1 - GUI/Controls/ChooseRecommendedMods.cs | 139 +++++------ GUI/Controls/ModInfoTabs/Versions.cs | 19 +- GUI/Main/MainChangeset.cs | 2 +- GUI/Main/MainInstall.cs | 2 +- GUI/Main/MainRecommendations.cs | 2 +- GUI/Main/MainTrayIcon.cs | 2 +- .../Relationships/RelationshipResolver.cs | 6 +- 11 files changed, 275 insertions(+), 321 deletions(-) diff --git a/Core/ModuleInstaller.cs b/Core/ModuleInstaller.cs index 2feac6f5c9..d11e3c5f1f 100644 --- a/Core/ModuleInstaller.cs +++ b/Core/ModuleInstaller.cs @@ -1048,7 +1048,7 @@ public void Upgrade(IEnumerable modules, var resolver = new RelationshipResolver( modules, modules.Select(m => registry.InstalledModule(m.identifier)?.Module).Where(m => m != null), - RelationshipResolver.DependsOnlyOpts(), + RelationshipResolverOptions.DependsOnlyOpts(), registry, ksp.VersionCriteria() ); @@ -1292,20 +1292,6 @@ private void DownloadModules(IEnumerable mods, IDownloader downloade } } - private static readonly RelationshipResolverOptions RecommenderOptions = new RelationshipResolverOptions() - { - // Only look at depends - with_recommends = false, - - // Don't throw anything - without_toomanyprovides_kraken = true, - without_enforce_consistency = true, - proceed_with_inconsistencies = true, - - // Skip relationships with suppress_recommendations==true - get_recommenders = true, - }; - /// /// Looks for optional related modules that could be installed alongside the given modules /// @@ -1317,148 +1303,50 @@ private void DownloadModules(IEnumerable mods, IDownloader downloade /// /// true if anything found, false otherwise /// - public bool FindRecommendations( - HashSet sourceModules, - List toInstall, - Registry registry, - out Dictionary>> recommendations, - out Dictionary> suggestions, - out Dictionary> supporters) + public bool FindRecommendations(HashSet sourceModules, + List toInstall, + Registry registry, + out Dictionary>> recommendations, + out Dictionary> suggestions, + out Dictionary> supporters) { - // Get all dependencies except where suppress_recommendations==true - var resolver = new RelationshipResolver(sourceModules, null, RecommenderOptions, - registry, ksp.VersionCriteria()); - var recommenders = resolver.ModList().ToList(); - - var dependersIndex = getDependersIndex(recommenders, registry, toInstall); - var instList = toInstall.ToList(); - recommendations = new Dictionary>>(); - suggestions = new Dictionary>(); - supporters = new Dictionary>(); - foreach (CkanModule mod in recommenders.Where(m => m.recommends != null)) - { - foreach (RelationshipDescriptor rel in mod.recommends) - { - List providers = rel.LatestAvailableWithProvides( - registry, ksp.VersionCriteria()); - int i = 0; - foreach (CkanModule provider in providers) - { - if (!registry.IsInstalled(provider.identifier) - && !toInstall.Any(m => m.identifier == provider.identifier) - && dependersIndex.TryGetValue(provider, out List dependers) - && (provider.IsDLC || CanInstall(RelationshipResolver.DependsOnlyOpts(), - instList.Concat(new List() { provider }).ToList(), registry))) - { - dependersIndex.Remove(provider); - recommendations.Add( - provider, - new Tuple>( - !provider.IsDLC && (i == 0 || provider.identifier == (rel as ModuleRelationshipDescriptor)?.name), - dependers)); - ++i; - } - } - } - } - foreach (CkanModule mod in recommenders.Where(m => m.suggests != null)) - { - foreach (RelationshipDescriptor rel in mod.suggests) - { - List providers = rel.LatestAvailableWithProvides( - registry, ksp.VersionCriteria()); - foreach (CkanModule provider in providers) - { - if (!registry.IsInstalled(provider.identifier) - && !toInstall.Any(m => m.identifier == provider.identifier) - && dependersIndex.TryGetValue(provider, out List dependers) - && (provider.IsDLC || CanInstall(RelationshipResolver.DependsOnlyOpts(), - instList.Concat(new List() { provider }).ToList(), registry))) - { - dependersIndex.Remove(provider); - suggestions.Add(provider, dependers); - } - } - } - } - - // Find installable modules with "supports" relationships - var candidates = registry.CompatibleModules(ksp.VersionCriteria()) - .Where(mod => !registry.IsInstalled(mod.identifier) - && !toInstall.Any(m => m.identifier == mod.identifier)) - .Where(m => m?.supports != null) - .Except(recommendations.Keys) - .Except(suggestions.Keys); - // Find each module that "supports" something we're installing - foreach (CkanModule mod in candidates) - { - foreach (RelationshipDescriptor rel in mod.supports) - { - if (rel.MatchesAny(recommenders, null, null)) - { - var name = (rel as ModuleRelationshipDescriptor)?.name; - if (!string.IsNullOrEmpty(name)) - { - if (supporters.TryGetValue(mod, out HashSet others)) - { - others.Add(name); - } - else - { - supporters.Add(mod, new HashSet() { name }); - } - } - } - } - } - supporters.RemoveWhere(kvp => - !CanInstall( - RelationshipResolver.DependsOnlyOpts(), - instList.Concat(new List() { kvp.Key }).ToList(), - registry)); - - return recommendations.Any() || suggestions.Any() || supporters.Any(); - } - - // Build up the list of who recommends what - private Dictionary> getDependersIndex( - IEnumerable sourceModules, - IRegistryQuerier registry, - List toExclude) - { - Dictionary> dependersIndex = new Dictionary>(); - foreach (CkanModule mod in sourceModules) - { - foreach (List relations in new List>() { mod.recommends, mod.suggests }) - { - if (relations != null) - { - foreach (RelationshipDescriptor rel in relations) - { - List providers = rel.LatestAvailableWithProvides( - registry, ksp.VersionCriteria()); - foreach (CkanModule provider in providers) - { - if (!registry.IsInstalled(provider.identifier) - && !toExclude.Any(m => m.identifier == provider.identifier)) - { - if (dependersIndex.TryGetValue(provider, out List dependers)) - { - // Add the dependent mod to the list of reasons this dependency is shown. - dependers.Add(mod.identifier); - } - else - { - // Add a new entry if this provider isn't listed yet. - dependersIndex.Add(provider, new List() { mod.identifier }); - } - } - } - } - } - } - } - return dependersIndex; + var crit = ksp.VersionCriteria(); + var resolver = new RelationshipResolver(sourceModules, null, + RelationshipResolverOptions.KitchenSinkOpts(), + registry, crit); + var recommenders = resolver.Dependencies().ToHashSet(); + + recommendations = resolver.Recommendations(recommenders) + .ToDictionary(m => m, + m => new Tuple>( + resolver.ReasonsFor(m) + .Any(r => (r as SelectionReason.Recommended) + ?.ProvidesIndex == 0), + resolver.ReasonsFor(m) + .Where(r => r is SelectionReason.Recommended rec + && recommenders.Contains(rec.Parent)) + .Select(r => r.Parent.identifier) + .ToList())); + suggestions = resolver.Suggestions(recommenders, + recommendations.Keys.ToList()) + .ToDictionary(m => m, + m => resolver.ReasonsFor(m) + .Where(r => r is SelectionReason.Suggested sug + && recommenders.Contains(sug.Parent)) + .Select(r => r.Parent.identifier) + .ToList()); + + var opts = RelationshipResolverOptions.DependsOnlyOpts(); + supporters = resolver.Supporters(recommenders, + toInstall.Concat(recommendations.Keys) + .Concat(suggestions.Keys)) + .Where(kvp => CanInstall(toInstall.Append(kvp.Key).ToList(), + opts, registry, crit)) + .ToDictionary(); + + return recommendations.Count > 0 + || suggestions.Count > 0 + || supporters.Count > 0; } /// @@ -1471,27 +1359,23 @@ private Dictionary> getDependersIndex( /// /// True if it's possible to install these mods, false otherwise /// - public bool CanInstall( - RelationshipResolverOptions opts, - List toInstall, - IRegistryQuerier registry - ) + public bool CanInstall(List toInstall, + RelationshipResolverOptions opts, + IRegistryQuerier registry, + GameVersionCriteria crit) { - string request = toInstall.Select(m => m.identifier).Aggregate((a, b) => $"{a}, {b}"); + string request = string.Join(", ", toInstall.Select(m => m.identifier)); try { - RelationshipResolver resolver = new RelationshipResolver( - toInstall, - toInstall.Select(m => registry.InstalledModule(m.identifier)?.Module).Where(m => m != null), - opts, registry, ksp.VersionCriteria() - ); + var installed = toInstall.Select(m => registry.InstalledModule(m.identifier)?.Module) + .Where(m => m != null); + var resolver = new RelationshipResolver(toInstall, installed, opts, registry, crit); - if (resolver.ModList().Count() >= toInstall.Count(m => !m.IsMetapackage)) + var resolverModList = resolver.ModList().ToList(); + if (resolverModList.Count >= toInstall.Count(m => !m.IsMetapackage)) { // We can install with no further dependencies - string recipe = resolver.ModList() - .Select(m => m.identifier) - .Aggregate((a, b) => $"{a}, {b}"); + string recipe = string.Join(", ", resolverModList.Select(m => m.identifier)); log.Debug($"Installable: {request}: {recipe}"); return true; } @@ -1504,10 +1388,10 @@ IRegistryQuerier registry catch (TooManyModsProvideKraken k) { // One of the dependencies is virtual - foreach (CkanModule mod in k.modules) + foreach (var mod in k.modules) { // Try each option recursively to see if any are successful - if (CanInstall(opts, toInstall.Concat(new List() { mod }).ToList(), registry)) + if (CanInstall(toInstall.Append(mod).ToList(), opts, registry, crit)) { // Child call will emit debug output, so we don't need to here return true; diff --git a/Core/Registry/IRegistryQuerier.cs b/Core/Registry/IRegistryQuerier.cs index 683aa34529..fa3b56ee20 100644 --- a/Core/Registry/IRegistryQuerier.cs +++ b/Core/Registry/IRegistryQuerier.cs @@ -362,7 +362,7 @@ private static IEnumerable FindRemovableAutoInstalled( var autoInstIds = autoInstMods.Select(im => im.Module.identifier).ToHashSet(); // Need to get the full changeset for this to work as intended - RelationshipResolverOptions opts = RelationshipResolver.DependsOnlyOpts(); + RelationshipResolverOptions opts = RelationshipResolverOptions.DependsOnlyOpts(); opts.without_toomanyprovides_kraken = true; opts.without_enforce_consistency = true; opts.proceed_with_inconsistencies = true; diff --git a/Core/Relationships/RelationshipResolver.cs b/Core/Relationships/RelationshipResolver.cs index ebc8d4ea99..7756af6bd5 100644 --- a/Core/Relationships/RelationshipResolver.cs +++ b/Core/Relationships/RelationshipResolver.cs @@ -69,30 +69,6 @@ private RelationshipResolver(RelationshipResolverOptions options, } } - /// - /// Returns the default options for relationship resolution. - /// - public static RelationshipResolverOptions DefaultOpts() - // TODO: This should just be able to return a new RelationshipResolverOptions - // and the defaults in the class definition should do the right thing. - => new RelationshipResolverOptions - { - with_recommends = true, - with_suggests = false, - with_all_suggests = false, - }; - - /// - /// Options to install without recommendations. - /// - public static RelationshipResolverOptions DependsOnlyOpts() - => new RelationshipResolverOptions - { - with_recommends = false, - with_suggests = false, - with_all_suggests = false, - }; - /// /// Add modules to consideration of the relationship resolver. /// @@ -190,15 +166,10 @@ private void Resolve(CkanModule module, alreadyResolved.Add(module); } - // Even though we may resolve top-level suggests for our module, - // we don't install suggestions all the way down unless with_all_suggests - // is true. - var sub_options = options.WithoutSuggestions(); - old_stanza = old_stanza?.Memoize(); log.DebugFormat("Resolving dependencies for {0}", module.identifier); - ResolveStanza(module.depends, new SelectionReason.Depends(module), sub_options, false, old_stanza); + ResolveStanza(module.depends, new SelectionReason.Depends(module), options, false, old_stanza); // TODO: RR currently conducts a depth-first resolution of requirements. While we do the // right thing in processing all dependencies first, then recommends, and then suggests, @@ -211,13 +182,19 @@ private void Resolve(CkanModule module, if (options.with_recommends) { log.DebugFormat("Resolving recommends for {0}", module.identifier); - ResolveStanza(module.recommends, new SelectionReason.Recommended(module), sub_options, true, old_stanza); + ResolveStanza(module.recommends, new SelectionReason.Recommended(module, 0), + options.get_recommenders ? options.WithoutRecommendations() + : options.WithoutSuggestions(), + true, old_stanza); } if (options.with_suggests || options.with_all_suggests) { log.DebugFormat("Resolving suggests for {0}", module.identifier); - ResolveStanza(module.suggests, new SelectionReason.Suggested(module), sub_options, true, old_stanza); + ResolveStanza(module.suggests, new SelectionReason.Suggested(module), + options.get_recommenders ? options.WithoutRecommendations() + : options.WithoutSuggestions(), + true, old_stanza); } } @@ -252,7 +229,7 @@ private void ResolveStanza(List stanza, if (options.get_recommenders && descriptor.suppress_recommendations) { - log.DebugFormat("Skipping {0} because get_recommenders option is set"); + log.DebugFormat("Skipping {0} because get_recommenders option is set", descriptor.ToString()); continue; } options = orig_options.OptionsFor(descriptor); @@ -339,10 +316,10 @@ private void ResolveStanza(List stanza, { if (!soft_resolve) { - log.InfoFormat("Dependency on {0} found but it is not listed in the index, or not available for your version of KSP.", descriptor.ToString()); + log.InfoFormat("Dependency on {0} found but it is not listed in the index, or not available for your game version.", descriptor.ToString()); throw new DependencyNotSatisfiedKraken(reason.Parent, descriptor.ToString()); } - log.InfoFormat("{0} is recommended/suggested but it is not listed in the index, or not available for your version of KSP.", descriptor.ToString()); + log.InfoFormat("{0} is recommended/suggested but it is not listed in the index, or not available for your game version.", descriptor.ToString()); continue; } if (candidates.Count > 1) @@ -350,6 +327,16 @@ private void ResolveStanza(List stanza, // Oh no, too many to pick from! if (options.without_toomanyprovides_kraken) { + if (options.get_recommenders) + { + for (int i = 0; i < candidates.Count; ++i) + { + var cand = candidates[i]; + Add(cand, reason is SelectionReason.Recommended rec + ? rec.WithIndex(i) + : reason); + } + } continue; } @@ -388,25 +375,22 @@ private void ResolveStanza(List stanza, Add(candidate, reason); Resolve(candidate, options, stanza); } + else if (options.proceed_with_inconsistencies) + { + Add(candidate, reason); + conflicts.Add(new ModPair(conflicting_mod, candidate)); + conflicts.Add(new ModPair(candidate, conflicting_mod)); + } else if (soft_resolve) { log.InfoFormat("{0} would cause conflicts, excluding it from consideration", candidate); } else { - if (options.proceed_with_inconsistencies) - { - Add(candidate, reason); - conflicts.Add(new ModPair(conflicting_mod, candidate)); - conflicts.Add(new ModPair(candidate, conflicting_mod)); - } - else - { - throw new InconsistentKraken(string.Format( - Properties.Resources.RelationshipResolverConflictsWith, - conflictingModDescription(conflicting_mod, null), - conflictingModDescription(candidate, reason.Parent))); - } + throw new InconsistentKraken(string.Format( + Properties.Resources.RelationshipResolverConflictsWith, + conflictingModDescription(conflicting_mod, null), + conflictingModDescription(candidate, reason.Parent))); } } } @@ -581,6 +565,47 @@ private IEnumerable allDependers(CkanModule module, .Select(r => r.Parent) .Except(found)); + public IEnumerable Dependencies() + => BreadthFirstSearch(user_requested_mods, + (searching, found) => + modlist.Values + .Except(found) + .Where(m => ReasonsFor(m).Any(r => r is SelectionReason.Depends + && r.Parent == searching))); + + public IEnumerable Recommendations(HashSet dependencies) + => modlist.Values.Except(dependencies) + .Where(m => ReasonsFor(m).Any(r => r is SelectionReason.Recommended + && dependencies.Contains(r.Parent))) + .OrderByDescending(totalDependers); + + public IEnumerable Suggestions(HashSet dependencies, + List recommendations) + => modlist.Values.Except(dependencies) + .Except(recommendations) + .Where(m => ReasonsFor(m).Any(r => r is SelectionReason.Suggested + && dependencies.Contains(r.Parent))) + .OrderByDescending(totalDependers); + + public ParallelQuery>> Supporters( + HashSet supported, + IEnumerable toExclude) + => registry.CompatibleModules(versionCrit) + .Except(toExclude) + .AsParallel() + // Find installable modules with "supports" relationships + .Where(mod => !registry.IsInstalled(mod.identifier) + && mod.supports != null) + // Find each module that "supports" something we're installing + .Select(mod => new KeyValuePair>( + mod, + mod.supports + .Where(rel => rel.MatchesAny(supported, null, null)) + .Select(rel => (rel as ModuleRelationshipDescriptor)?.name) + .Where(name => !string.IsNullOrEmpty(name)) + .ToHashSet())) + .Where(kvp => kvp.Value.Count > 0); + /// /// Returns a dictionary consisting of KeyValuePairs containing conflicting mods. /// The keys are the mods that the user chose that led to the conflict being in the list! @@ -679,6 +704,40 @@ private void AddReason(CkanModule module, SelectionReason reason) // cases in their heads. public class RelationshipResolverOptions { + /// + /// Default options for relationship resolution. + /// + public static RelationshipResolverOptions DefaultOpts() + => new RelationshipResolverOptions(); + + /// + /// Options to install without recommendations. + /// + public static RelationshipResolverOptions DependsOnlyOpts() + => new RelationshipResolverOptions() + { + with_recommends = false, + with_suggests = false, + with_all_suggests = false, + }; + + /// + /// Options to find all dependencies, recommendations, and suggestions + /// of anything in the changeset (except when suppress_recommendations==true), + /// without throwing exceptions, so the calling code can decide what to do about conflicts + /// + public static RelationshipResolverOptions KitchenSinkOpts() + => new RelationshipResolverOptions() + { + with_recommends = true, + with_suggests = true, + with_all_suggests = true, + without_toomanyprovides_kraken = true, + without_enforce_consistency = true, + proceed_with_inconsistencies = true, + get_recommenders = true, + }; + /// /// If true, add recommended mods, and their recommendations. /// @@ -736,7 +795,7 @@ public class RelationshipResolverOptions public RelationshipResolverOptions OptionsFor(RelationshipDescriptor descr) => descr.suppress_recommendations ? WithoutRecommendations() : this; - private RelationshipResolverOptions WithoutRecommendations() + public RelationshipResolverOptions WithoutRecommendations() { if (with_recommends || with_all_suggests || with_suggests) { @@ -770,8 +829,7 @@ public abstract class SelectionReason { // Currently assumed to exist for any relationship other than UserRequested or Installed public virtual CkanModule Parent { get; protected set; } - public abstract string Reason { get; } - public virtual string DescribeWith(IEnumerable others) => Reason; + public virtual string DescribeWith(IEnumerable others) => ToString(); public class Installed : SelectionReason { @@ -784,7 +842,7 @@ public override CkanModule Parent } } - public override string Reason + public override string ToString() => Properties.Resources.RelationshipResolverInstalledReason; } @@ -799,19 +857,19 @@ public override CkanModule Parent } } - public override string Reason + public override string ToString() => Properties.Resources.RelationshipResolverUserReason; } public class DependencyRemoved : SelectionReason { - public override string Reason + public override string ToString() => Properties.Resources.RelationshipResolverDependencyRemoved; } public class NoLongerUsed : SelectionReason { - public override string Reason + public override string ToString() => Properties.Resources.RelationshipResolverNoLongerUsedReason; } @@ -823,7 +881,7 @@ public Replacement(CkanModule module) Parent = module; } - public override string Reason + public override string ToString() => string.Format(Properties.Resources.RelationshipResolverReplacementReason, Parent.name); } @@ -835,7 +893,7 @@ public Suggested(CkanModule module) Parent = module; } - public override string Reason + public override string ToString() => string.Format(Properties.Resources.RelationshipResolverSuggestedReason, Parent.name); } @@ -847,7 +905,7 @@ public Depends(CkanModule module) Parent = module; } - public override string Reason + public override string ToString() => string.Format(Properties.Resources.RelationshipResolverDependsReason, Parent.name); public override string DescribeWith(IEnumerable others) @@ -857,13 +915,22 @@ public override string DescribeWith(IEnumerable others) public sealed class Recommended : SelectionReason { - public Recommended(CkanModule module) + public Recommended(CkanModule module, int providesIndex) { - if (module == null) throw new ArgumentNullException(); - Parent = module; + if (module == null) + { + throw new ArgumentNullException(); + } + Parent = module; + ProvidesIndex = providesIndex; } - public override string Reason + public readonly int ProvidesIndex; + + public Recommended WithIndex(int providesIndex) + => new Recommended(Parent, providesIndex); + + public override string ToString() => string.Format(Properties.Resources.RelationshipResolverRecommendedReason, Parent.name); } } diff --git a/GUI/Controls/ChooseRecommendedMods.Designer.cs b/GUI/Controls/ChooseRecommendedMods.Designer.cs index 3e4a6ac06e..8032a65f24 100644 --- a/GUI/Controls/ChooseRecommendedMods.Designer.cs +++ b/GUI/Controls/ChooseRecommendedMods.Designer.cs @@ -73,7 +73,6 @@ private void InitializeComponent() this.RecommendedModsListView.UseCompatibleStateImageBehavior = false; this.RecommendedModsListView.View = System.Windows.Forms.View.Details; this.RecommendedModsListView.SelectedIndexChanged += new System.EventHandler(RecommendedModsListView_SelectedIndexChanged); - this.RecommendedModsListView.ItemChecked += new System.Windows.Forms.ItemCheckedEventHandler(RecommendedModsListView_ItemChecked); this.RecommendedModsListView.Groups.Add(this.RecommendationsGroup); this.RecommendedModsListView.Groups.Add(this.SuggestionsGroup); this.RecommendedModsListView.Groups.Add(this.SupportedByGroup); diff --git a/GUI/Controls/ChooseRecommendedMods.cs b/GUI/Controls/ChooseRecommendedMods.cs index cafafb25b3..a1ef0cbc77 100644 --- a/GUI/Controls/ChooseRecommendedMods.cs +++ b/GUI/Controls/ChooseRecommendedMods.cs @@ -19,25 +19,30 @@ public ChooseRecommendedMods() } [ForbidGUICalls] - public void LoadRecommendations( - IRegistryQuerier registry, - List toInstall, HashSet toUninstall, - GameVersionCriteria GameVersion, NetModuleCache cache, - Dictionary>> recommendations, - Dictionary> suggestions, - Dictionary> supporters - ) + public void LoadRecommendations(IRegistryQuerier registry, + List toInstall, + HashSet toUninstall, + GameVersionCriteria versionCrit, + NetModuleCache cache, + Dictionary>> recommendations, + Dictionary> suggestions, + Dictionary> supporters) { this.registry = registry; this.toInstall = toInstall; this.toUninstall = toUninstall; - this.GameVersion = GameVersion; + this.versionCrit = versionCrit; Util.Invoke(this, () => { RecommendedModsToggleCheckbox.Checked = true; + RecommendedModsListView.BeginUpdate(); + RecommendedModsListView.ItemChecked -= RecommendedModsListView_ItemChecked; RecommendedModsListView.Items.AddRange( getRecSugRows(cache, recommendations, suggestions, supporters).ToArray()); MarkConflicts(); + RecommendedModsListView.EndUpdate(); + // Don't set this before AddRange, it will fire for every row we add! + RecommendedModsListView.ItemChecked += RecommendedModsListView_ItemChecked; }); } @@ -54,12 +59,7 @@ public HashSet Wait() } public ListView.SelectedListViewItemCollection SelectedItems - { - get - { - return RecommendedModsListView.SelectedItems; - } - } + => RecommendedModsListView.SelectedItems; public event Action OnSelectedItemsChanged; @@ -67,10 +67,7 @@ public ListView.SelectedListViewItemCollection SelectedItems private void RecommendedModsListView_SelectedIndexChanged(object sender, EventArgs e) { - if (OnSelectedItemsChanged != null) - { - OnSelectedItemsChanged(RecommendedModsListView.SelectedItems); - } + OnSelectedItemsChanged?.Invoke(RecommendedModsListView.SelectedItems); } private void RecommendedModsListView_ItemChecked(object sender, ItemCheckedEventArgs e) @@ -93,7 +90,15 @@ private void MarkConflicts() { try { - var conflicts = FindConflicts(); + var resolver = new RelationshipResolver( + RecommendedModsListView.CheckedItems + .Cast() + .Select(item => item.Tag as CkanModule) + .Concat(toInstall) + .Distinct(), + toUninstall, + conflictOptions, registry, versionCrit); + var conflicts = resolver.ConflictList; foreach (var item in RecommendedModsListView.Items.Cast() // Apparently ListView handes AddRange by: // 1. Expanding the Items list to the new size by filling it with nulls @@ -106,15 +111,13 @@ private void MarkConflicts() : Color.Empty; } RecommendedModsContinueButton.Enabled = !conflicts.Any(); - if (OnConflictFound != null) - { - OnConflictFound(conflicts.Any() ? conflicts.First().Value : ""); - } + OnConflictFound?.Invoke(string.Join("; ", resolver.ConflictDescriptions)); } catch (DependencyNotSatisfiedKraken k) { - var row = RecommendedModsListView.Items.Cast() - .FirstOrDefault(it => (it?.Tag as CkanModule) == k.parent); + var row = RecommendedModsListView.Items + .Cast() + .FirstOrDefault(it => (it?.Tag as CkanModule) == k.parent); if (row != null) { row.BackColor = Color.LightCoral; @@ -132,47 +135,36 @@ private void MarkConflicts() with_recommends = false }; - private Dictionary FindConflicts() - => new RelationshipResolver( - RecommendedModsListView.CheckedItems.Cast() - .Select(item => item.Tag as CkanModule) - .Concat(toInstall) - .Distinct(), - toUninstall, - conflictOptions, registry, GameVersion - ).ConflictList; - private IEnumerable getRecSugRows( - NetModuleCache cache, + NetModuleCache cache, Dictionary>> recommendations, - Dictionary> suggestions, - Dictionary> supporters) - { - foreach (var kvp in recommendations) - { - yield return getRecSugItem(cache, kvp.Key, string.Join(", ", kvp.Value.Item2), - RecommendationsGroup, kvp.Value.Item1); - } - - foreach (var kvp in suggestions) - { - yield return getRecSugItem(cache, kvp.Key, string.Join(", ", kvp.Value), - SuggestionsGroup, false); - } - - foreach (var kvp in supporters - .ToDictionary(kvp => kvp.Key, kvp => string.Join(", ", kvp.Value.OrderBy(s => s))) - .OrderBy(kvp => kvp.Value) - ) - { - yield return getRecSugItem(cache, kvp.Key, string.Join(", ", kvp.Value), - SupportedByGroup, false); - } - } - - private ListViewItem getRecSugItem(NetModuleCache cache, CkanModule module, string descrip, ListViewGroup group, bool check) - { - return new ListViewItem(new string[] + Dictionary> suggestions, + Dictionary> supporters) + => recommendations.Select(kvp => getRecSugItem(cache, + kvp.Key, + string.Join(", ", kvp.Value.Item2), + RecommendationsGroup, + kvp.Value.Item1)) + .OrderBy(item => item.SubItems[1].Text) + .Concat(suggestions.Select(kvp => getRecSugItem(cache, + kvp.Key, + string.Join(", ", kvp.Value), + SuggestionsGroup, + false)) + .OrderBy(item => item.SubItems[1].Text)) + .Concat(supporters.Select(kvp => getRecSugItem(cache, + kvp.Key, + string.Join(", ", kvp.Value.OrderBy(s => s)), + SupportedByGroup, + false)) + .OrderBy(item => item.SubItems[1].Text)); + + private ListViewItem getRecSugItem(NetModuleCache cache, + CkanModule module, + string descrip, + ListViewGroup group, + bool check) + => new ListViewItem(new string[] { module.IsDLC ? module.name : cache.DescribeAvailability(module), descrip, @@ -183,40 +175,43 @@ private ListViewItem getRecSugItem(NetModuleCache cache, CkanModule module, stri Checked = check, Group = group }; - } private void RecommendedModsToggleCheckbox_CheckedChanged(object sender, EventArgs e) { var state = ((CheckBox)sender).Checked; RecommendedModsListView.BeginUpdate(); + RecommendedModsListView.ItemChecked -= RecommendedModsListView_ItemChecked; foreach (ListViewItem item in RecommendedModsListView.Items) { if (item.Checked != state) item.Checked = state; } + MarkConflicts(); RecommendedModsListView.EndUpdate(); + RecommendedModsListView.ItemChecked += RecommendedModsListView_ItemChecked; } private void RecommendedModsCancelButton_Click(object sender, EventArgs e) { task?.SetResult(null); RecommendedModsListView.Items.Clear(); + RecommendedModsListView.ItemChecked -= RecommendedModsListView_ItemChecked; } private void RecommendedModsContinueButton_Click(object sender, EventArgs e) { - task?.SetResult( - RecommendedModsListView.CheckedItems.Cast() - .Select(item => item.Tag as CkanModule) - .ToHashSet() - ); + task?.SetResult(RecommendedModsListView.CheckedItems + .Cast() + .Select(item => item.Tag as CkanModule) + .ToHashSet()); RecommendedModsListView.Items.Clear(); + RecommendedModsListView.ItemChecked -= RecommendedModsListView_ItemChecked; } private IRegistryQuerier registry; private List toInstall; private HashSet toUninstall; - private GameVersionCriteria GameVersion; + private GameVersionCriteria versionCrit; private TaskCompletionSource> task; } diff --git a/GUI/Controls/ModInfoTabs/Versions.cs b/GUI/Controls/ModInfoTabs/Versions.cs index bc5b89bdc7..e8f87a395d 100644 --- a/GUI/Controls/ModInfoTabs/Versions.cs +++ b/GUI/Controls/ModInfoTabs/Versions.cs @@ -95,11 +95,20 @@ private void VersionsListView_ItemCheck(object sender, ItemCheckEventArgs e) } [ForbidGUICalls] - private bool installable(ModuleInstaller installer, CkanModule module, IRegistryQuerier registry) - => module.IsCompatible(Main.Instance.CurrentInstance.VersionCriteria()) - && installer.CanInstall(RelationshipResolver.DependsOnlyOpts(), - new List() { module }, - registry); + private static bool installable(ModuleInstaller installer, + CkanModule module, + IRegistryQuerier registry) + => installable(installer, module, registry, Main.Instance.CurrentInstance.VersionCriteria()); + + [ForbidGUICalls] + private static bool installable(ModuleInstaller installer, + CkanModule module, + IRegistryQuerier registry, + GameVersionCriteria crit) + => module.IsCompatible(crit) + && installer.CanInstall(new List() { module }, + RelationshipResolverOptions.DependsOnlyOpts(), + registry, crit); private bool allowInstall(CkanModule module) { diff --git a/GUI/Main/MainChangeset.cs b/GUI/Main/MainChangeset.cs index e878c2ba2a..0f472d2776 100644 --- a/GUI/Main/MainChangeset.cs +++ b/GUI/Main/MainChangeset.cs @@ -47,7 +47,7 @@ private void Changeset_OnConfirmChanges(List changeset) // Include all removes and upgrades || ch.ChangeType != GUIModChangeType.Install) .ToList(), - RelationshipResolver.DependsOnlyOpts())); + RelationshipResolverOptions.DependsOnlyOpts())); } catch (InvalidOperationException) { diff --git a/GUI/Main/MainInstall.cs b/GUI/Main/MainInstall.cs index 1fbebdced2..937e22a212 100644 --- a/GUI/Main/MainInstall.cs +++ b/GUI/Main/MainInstall.cs @@ -55,7 +55,7 @@ public void InstallModuleDriver(IRegistryQuerier registry, IEnumerable> supporters new InstallArgument( result.Select(mod => new ModChange(mod, GUIModChangeType.Install)) .ToList(), - RelationshipResolver.DependsOnlyOpts())); + RelationshipResolverOptions.DependsOnlyOpts())); } } else diff --git a/GUI/Main/MainTrayIcon.cs b/GUI/Main/MainTrayIcon.cs index f7a2e244b8..5548ff607b 100644 --- a/GUI/Main/MainTrayIcon.cs +++ b/GUI/Main/MainTrayIcon.cs @@ -147,7 +147,7 @@ private void minimizeNotifyIcon_BalloonTipClicked(object sender, EventArgs e) RegistryManager.Instance(CurrentInstance, repoData).registry, CurrentInstance.VersionCriteria()) .ToList(), - RelationshipResolver.DependsOnlyOpts()) + RelationshipResolverOptions.DependsOnlyOpts()) ); } #endregion diff --git a/Tests/Core/Relationships/RelationshipResolver.cs b/Tests/Core/Relationships/RelationshipResolver.cs index da01c83447..cfe6111efc 100644 --- a/Tests/Core/Relationships/RelationshipResolver.cs +++ b/Tests/Core/Relationships/RelationshipResolver.cs @@ -23,7 +23,7 @@ public class RelationshipResolverTests [SetUp] public void Setup() { - options = RelationshipResolver.DefaultOpts(); + options = RelationshipResolverOptions.DefaultOpts(); generator = new RandomModuleGenerator(new Random(0451)); //Sanity checker means even incorrect RelationshipResolver logic was passing options.without_enforce_consistency = true; @@ -33,7 +33,7 @@ public void Setup() public void Constructor_WithoutModules_AlwaysReturns() { var registry = CKAN.Registry.Empty(); - options = RelationshipResolver.DefaultOpts(); + options = RelationshipResolverOptions.DefaultOpts(); Assert.DoesNotThrow(() => new RelationshipResolver(new List(), null, options, registry, null)); } @@ -984,7 +984,7 @@ public void AutodetectedCanSatisfyRelationships() CkanModule mod = generator.GeneratorRandomModule(depends: depends); new RelationshipResolver( - new CkanModule[] { mod }, null, RelationshipResolver.DefaultOpts(), + new CkanModule[] { mod }, null, RelationshipResolverOptions.DefaultOpts(), registry, new GameVersionCriteria(GameVersion.Parse("1.0.0"))); } } From 4c10b6821e606f494457faa9fdac5ff71146fa4e Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Sat, 7 Oct 2023 09:20:19 -0500 Subject: [PATCH 22/22] Remove redundant pre-install ZIP validation --- Core/ModuleInstaller.cs | 6 +- Core/Net/NetFileCache.cs | 128 ------------------ Core/Net/NetModuleCache.cs | 99 ++++++++++++-- Netkan/Services/CachingHttpService.cs | 2 +- Tests/Core/Cache.cs | 12 +- Tests/Core/ModuleInstallerTests.cs | 4 +- .../Net/NetAsyncModulesDownloaderTests.cs | 16 +-- 7 files changed, 110 insertions(+), 157 deletions(-) diff --git a/Core/ModuleInstaller.cs b/Core/ModuleInstaller.cs index d11e3c5f1f..7101427d1c 100644 --- a/Core/ModuleInstaller.cs +++ b/Core/ModuleInstaller.cs @@ -102,7 +102,7 @@ public static string CachedOrDownload(CkanModule module, NetModuleCache cache, s filename = CkanModule.StandardName(module.identifier, module.version); } - string full_path = cache.GetCachedZip(module); + string full_path = cache.GetCachedFilename(module); if (full_path == null) { return Download(module, filename, cache); @@ -264,7 +264,7 @@ private void Install(CkanModule module, bool autoInstalled, Registry registry, r } // Find ZIP in the cache if we don't already have it. - filename = filename ?? Cache.GetCachedZip(module); + filename = filename ?? Cache.GetCachedFilename(module); // If we *still* don't have a file, then kraken bitterly. if (filename == null) @@ -1284,7 +1284,7 @@ public static IEnumerable PrioritizedHosts(IEnumerable urls) /// private void DownloadModules(IEnumerable mods, IDownloader downloader) { - List downloads = mods.Where(module => !Cache.IsCachedZip(module)).ToList(); + List downloads = mods.Where(module => !Cache.IsCached(module)).ToList(); if (downloads.Count > 0) { diff --git a/Core/Net/NetFileCache.cs b/Core/Net/NetFileCache.cs index 41f8399485..d582619c52 100644 --- a/Core/Net/NetFileCache.cs +++ b/Core/Net/NetFileCache.cs @@ -11,7 +11,6 @@ using log4net; using ChinhDo.Transactions.FileManager; -using ICSharpCode.SharpZipLib.Zip; using CKAN.Extensions; using CKAN.Versioning; @@ -36,13 +35,6 @@ public class NetFileCache : IDisposable private static readonly Regex cacheFileRegex = new Regex("^[0-9A-F]{8}-", RegexOptions.Compiled); private static readonly ILog log = LogManager.GetLogger(typeof (NetFileCache)); - static NetFileCache() - { - // SharpZibLib 1.1.0 changed this to default to false, but we depend on it for international mods. - // https://github.com/icsharpcode/SharpZipLib/issues/591 - ZipStrings.UseUnicode = true; - } - /// /// Initialize a cache given a GameInstanceManager /// @@ -163,13 +155,6 @@ public bool IsCached(Uri url, out string outFilename) return outFilename != null; } - /// - /// Returns true if our given URL is cached, *and* it passes zip - /// validation tests. Prefer this over IsCached when working with - /// zip files. - /// - public bool IsCachedZip(Uri url) => GetCachedZip(url) != null; - /// /// Returns true if a file matching the given URL is cached, but makes no /// attempts to check if it's even valid. This is very fast. @@ -260,38 +245,6 @@ private string scanDirectory(Dictionary files, string findHash, return null; } - /// - /// Returns the filename for a cached URL, if and only if it - /// passes zipfile validation tests. Prefer this to GetCachedFilename - /// when working with zip files. Returns null if not available, or - /// validation failed. - /// - /// Low level CRC (cyclic redundancy check) checks will be done. - /// This can take time on order of seconds for larger zip files. - /// - public string GetCachedZip(Uri url) - { - string filename = GetCachedFilename(url); - if (string.IsNullOrEmpty(filename)) - { - return null; - } - else - { - string invalidReason; - if (ZipValid(filename, out invalidReason, null)) - { - return filename; - } - else - { - // Purge invalid cache entries - File.Delete(filename); - return null; - } - } - } - /// /// Count the files and bytes in the cache /// @@ -442,87 +395,6 @@ private List allFiles(bool includeInProgress = false) ).ToList(); } - /// - /// Check whether a ZIP file is valid - /// - /// Path to zip file to check - /// Description of problem with the file - /// Callback to notify as we traverse the input, called with percentages from 0 to 100 - /// - /// True if valid, false otherwise. See invalidReason param for explanation. - /// - public static bool ZipValid(string filename, out string invalidReason, IProgress progress) - { - try - { - if (filename != null) - { - using (ZipFile zip = new ZipFile(filename)) - { - string zipErr = null; - // Limit progress updates to 100 per ZIP file - long highestPercent = -1; - // Perform CRC and other checks - if (zip.TestArchive(true, TestStrategy.FindFirstError, - (TestStatus st, string msg) => - { - // This delegate is called as TestArchive proceeds through its - // steps, both routine and abnormal. - // The second parameter is non-null if an error occurred. - if (st != null && !st.EntryValid && !string.IsNullOrEmpty(msg)) - { - // Capture the error string so we can return it - zipErr = string.Format( - Properties.Resources.NetFileCacheZipError, - st.Operation, st.Entry?.Name, msg); - } - else if (st.Entry != null && progress != null) - { - // Report progress - var percent = 100 * st.Entry.ZipFileIndex / zip.Count; - if (percent > highestPercent) - { - progress.Report(percent); - highestPercent = percent; - } - } - })) - { - invalidReason = ""; - return true; - } - else - { - invalidReason = zipErr ?? Properties.Resources.NetFileCacheZipTestArchiveFalse; - return false; - } - } - } - else - { - invalidReason = Properties.Resources.NetFileCacheNullFileName; - return false; - } - } - catch (ZipException ze) - { - // Save the errors someplace useful - invalidReason = ze.Message; - return false; - } - catch (ArgumentException ex) - { - invalidReason = ex.Message; - return false; - } - catch (NotSupportedException nse) when (Platform.IsMono) - { - // SharpZipLib throws this if your locale isn't installed on Mono - invalidReason = string.Format(Properties.Resources.NetFileCacheMonoNotSupported, nse.Message); - return false; - } - } - /// /// Stores the results of a given URL in the cache. /// Description is adjusted to be filesystem-safe and then appended to the file hash when saving. diff --git a/Core/Net/NetModuleCache.cs b/Core/Net/NetModuleCache.cs index dca351a359..b3db6165cb 100644 --- a/Core/Net/NetModuleCache.cs +++ b/Core/Net/NetModuleCache.cs @@ -4,6 +4,8 @@ using System.Threading; using System.Security.Cryptography; +using ICSharpCode.SharpZipLib.Zip; + namespace CKAN { /// @@ -16,6 +18,12 @@ namespace CKAN /// public class NetModuleCache : IDisposable { + static NetModuleCache() + { + // SharpZibLib 1.1.0 changed this to default to false, but we depend on it for international mods. + // https://github.com/icsharpcode/SharpZipLib/issues/591 + ZipStrings.UseUnicode = true; + } /// /// Initialize the cache @@ -66,9 +74,6 @@ public bool IsCached(CkanModule m, out string outFilename) outFilename = null; return false; } - public bool IsCachedZip(CkanModule m) - => m.download?.Any(dlUri => cache.IsCachedZip(dlUri)) - ?? false; public bool IsMaybeCachedZip(CkanModule m) => m.download?.Any(dlUri => cache.IsMaybeCachedZip(dlUri, m.release_date)) ?? false; @@ -76,10 +81,6 @@ public string GetCachedFilename(CkanModule m) => m.download?.Select(dlUri => cache.GetCachedFilename(dlUri, m.release_date)) .Where(filename => filename != null) .FirstOrDefault(); - public string GetCachedZip(CkanModule m) - => m.download?.Select(dlUri => cache.GetCachedZip(dlUri)) - .Where(filename => filename != null) - .FirstOrDefault(); public void GetSizeInfo(out int numFiles, out long numBytes, out long bytesFree) { cache.GetSizeInfo(out numFiles, out numBytes, out bytesFree); @@ -172,8 +173,7 @@ public string DescribeAvailability(CkanModule m) cancelToken.ThrowIfCancellationRequested(); // Check valid CRC - string invalidReason; - if (!NetFileCache.ZipValid(path, out invalidReason, new Progress(percent => + if (!ZipValid(path, out string invalidReason, new Progress(percent => progress?.Report(percent * zipValidPercent / 100)))) { throw new InvalidModuleFileKraken(module, path, string.Format( @@ -216,6 +216,87 @@ public string DescribeAvailability(CkanModule m) return success; } + /// + /// Check whether a ZIP file is valid + /// + /// Path to zip file to check + /// Description of problem with the file + /// Callback to notify as we traverse the input, called with percentages from 0 to 100 + /// + /// True if valid, false otherwise. See invalidReason param for explanation. + /// + public static bool ZipValid(string filename, out string invalidReason, IProgress progress) + { + try + { + if (filename != null) + { + using (ZipFile zip = new ZipFile(filename)) + { + string zipErr = null; + // Limit progress updates to 100 per ZIP file + long highestPercent = -1; + // Perform CRC and other checks + if (zip.TestArchive(true, TestStrategy.FindFirstError, + (TestStatus st, string msg) => + { + // This delegate is called as TestArchive proceeds through its + // steps, both routine and abnormal. + // The second parameter is non-null if an error occurred. + if (st != null && !st.EntryValid && !string.IsNullOrEmpty(msg)) + { + // Capture the error string so we can return it + zipErr = string.Format( + Properties.Resources.NetFileCacheZipError, + st.Operation, st.Entry?.Name, msg); + } + else if (st.Entry != null && progress != null) + { + // Report progress + var percent = 100 * st.Entry.ZipFileIndex / zip.Count; + if (percent > highestPercent) + { + progress.Report(percent); + highestPercent = percent; + } + } + })) + { + invalidReason = ""; + return true; + } + else + { + invalidReason = zipErr ?? Properties.Resources.NetFileCacheZipTestArchiveFalse; + return false; + } + } + } + else + { + invalidReason = Properties.Resources.NetFileCacheNullFileName; + return false; + } + } + catch (ZipException ze) + { + // Save the errors someplace useful + invalidReason = ze.Message; + return false; + } + catch (ArgumentException ex) + { + invalidReason = ex.Message; + return false; + } + catch (NotSupportedException nse) when (Platform.IsMono) + { + // SharpZipLib throws this if your locale isn't installed on Mono + invalidReason = string.Format(Properties.Resources.NetFileCacheMonoNotSupported, nse.Message); + return false; + } + } + /// /// Remove a module's download files from the cache /// diff --git a/Netkan/Services/CachingHttpService.cs b/Netkan/Services/CachingHttpService.cs index 76d7e30da7..ab3c12bc03 100644 --- a/Netkan/Services/CachingHttpService.cs +++ b/Netkan/Services/CachingHttpService.cs @@ -87,7 +87,7 @@ private string DownloadPackage(Uri url, string identifier, DateTime? updated, Ur case FileType.Zip: extension = "zip"; string invalidReason; - if (!NetFileCache.ZipValid(downloadedFile, out invalidReason, null)) + if (!NetModuleCache.ZipValid(downloadedFile, out invalidReason, null)) { log.Debug($"{url} is not a valid ZIP file: {invalidReason}"); File.Delete(downloadedFile); diff --git a/Tests/Core/Cache.cs b/Tests/Core/Cache.cs index 09d42a4a2f..4e66bcaa9f 100644 --- a/Tests/Core/Cache.cs +++ b/Tests/Core/Cache.cs @@ -161,21 +161,21 @@ public void ZipValidation() // We could use any URL, but this one is awesome. <3 Uri url = new Uri("http://kitte.nz/"); - Assert.IsFalse(cache.IsCachedZip(url)); + Assert.IsFalse(cache.IsCached(url)); // Store a bad zip. cache.Store(url, TestData.DogeCoinFlagZipCorrupt()); - // Make sure it's stored, but not valid as a zip + // Make sure it's stored Assert.IsTrue(cache.IsCached(url)); - Assert.IsFalse(cache.IsCachedZip(url)); + // Make sure it's not valid as a zip + Assert.IsFalse(NetModuleCache.ZipValid(cache.GetCachedFilename(url), out string invalidReason, null)); // Store a good zip. cache.Store(url, TestData.DogeCoinFlagZip()); // Make sure it's stored, and valid. Assert.IsTrue(cache.IsCached(url)); - Assert.IsTrue(cache.IsCachedZip(url)); } [Test] @@ -189,7 +189,7 @@ public void ZipValid_ContainsFilenameWithBadChars_NoException() bool valid = false; string reason = ""; Assert.DoesNotThrow(() => - valid = NetFileCache.ZipValid(TestData.ZipWithBadChars, out reason, null)); + valid = NetModuleCache.ZipValid(TestData.ZipWithBadChars, out reason, null)); // The file is considered valid on Linux; // only check the reason if found invalid @@ -212,7 +212,7 @@ public void ZipValid_ContainsFilenameWithUnicodeChars_Valid() string reason = null; Assert.DoesNotThrow(() => - valid = NetFileCache.ZipValid(TestData.ZipWithUnicodeChars, out reason, null)); + valid = NetModuleCache.ZipValid(TestData.ZipWithUnicodeChars, out reason, null)); Assert.IsTrue(valid, reason); } diff --git a/Tests/Core/ModuleInstallerTests.cs b/Tests/Core/ModuleInstallerTests.cs index 7262c3fe3a..3d021014d7 100644 --- a/Tests/Core/ModuleInstallerTests.cs +++ b/Tests/Core/ModuleInstallerTests.cs @@ -407,13 +407,13 @@ public void CanInstallMod() Assert.IsFalse(File.Exists(mod_file_path)); // Copy the zip file to the cache directory. - Assert.IsFalse(manager.Cache.IsCachedZip(TestData.DogeCoinFlag_101_module())); + Assert.IsFalse(manager.Cache.IsCached(TestData.DogeCoinFlag_101_module())); string cache_path = manager.Cache.Store(TestData.DogeCoinFlag_101_module(), TestData.DogeCoinFlagZip(), new Progress(bytes => {})); - Assert.IsTrue(manager.Cache.IsCachedZip(TestData.DogeCoinFlag_101_module())); + Assert.IsTrue(manager.Cache.IsCached(TestData.DogeCoinFlag_101_module())); Assert.IsTrue(File.Exists(cache_path)); var registry = CKAN.RegistryManager.Instance(manager.CurrentInstance, repoData.Manager).registry; diff --git a/Tests/Core/Net/NetAsyncModulesDownloaderTests.cs b/Tests/Core/Net/NetAsyncModulesDownloaderTests.cs index 219a8b9081..0bc51bef3f 100644 --- a/Tests/Core/Net/NetAsyncModulesDownloaderTests.cs +++ b/Tests/Core/Net/NetAsyncModulesDownloaderTests.cs @@ -171,8 +171,8 @@ public void SingleDownload() // Download our module. async.DownloadModules(modules); - // Assert that we have it, and it passes zip validation. - Assert.IsTrue(cache.IsCachedZip(kOS)); + // Assert that we have it (which meansit passed zip validation) + Assert.IsTrue(cache.IsCached(kOS)); } [Test] @@ -189,13 +189,13 @@ public void MultiDownload() modules.Add(kOS); modules.Add(quick_revert); - Assert.IsFalse(cache.IsCachedZip(kOS)); - Assert.IsFalse(cache.IsCachedZip(quick_revert)); + Assert.IsFalse(cache.IsCached(kOS)); + Assert.IsFalse(cache.IsCached(quick_revert)); async.DownloadModules(modules); - Assert.IsTrue(cache.IsCachedZip(kOS)); - Assert.IsTrue(cache.IsCachedZip(quick_revert)); + Assert.IsTrue(cache.IsCached(kOS)); + Assert.IsTrue(cache.IsCached(quick_revert)); } [Test] @@ -210,11 +210,11 @@ public void RandSdownload() modules.Add(rAndS); - Assert.IsFalse(cache.IsCachedZip(rAndS), "Module not yet downloaded"); + Assert.IsFalse(cache.IsCached(rAndS), "Module not yet downloaded"); async.DownloadModules(modules); - Assert.IsTrue(cache.IsCachedZip(rAndS), "Module download successful"); + Assert.IsTrue(cache.IsCached(rAndS), "Module download successful"); } }