Obey version properties of conflicts and depends relationships in sanity checks

Cmdline/Action/Upgrade.cs

@@ -128,6 +128,11 @@ public int RunCommand(CKAN.KSP ksp, object raw_options)
                 User.RaiseMessage("Module {0} not found", kraken.module);
                 return Exit.ERROR;
             }
+            catch (InconsistentKraken kraken)
+            {
+                User.RaiseMessage(kraken.ToString());
+                return Exit.ERROR;
+            }
             User.RaiseMessage("\r\nDone!\r\n");
 
             return Exit.OK;

Core/Registry/Registry.cs

@@ -1001,9 +1001,8 @@ internal static HashSet<string> FindReverseDependencies(IEnumerable<string> modu
                 log.DebugFormat("Started with {0}, removing {1}, and keeping {2}; our dlls are {3}", string.Join(", ", orig_installed), string.Join(", ", modules_to_remove), string.Join(", ", hypothetical), string.Join(", ", dlls));
 
                 // Find what would break with this configuration.
-                // The Values.SelectMany() flattens our list of broken mods.
-                var broken = new HashSet<string>(SanityChecker.FindUnmetDependencies(hypothetical, dlls)
-                    .Values.SelectMany(x => x).Select(x => x.identifier));
+                var broken = SanityChecker.FindUnsatisfiedDepends(hypothetical, dlls.ToHashSet())
+                    .Select(x => x.Key.identifier).ToHashSet();
 
                 // If nothing else would break, it's just the list of modules we're removing.
                 HashSet<string> to_remove = new HashSet<string>(modules_to_remove);

Core/Relationships/SanityChecker.cs

@@ -9,7 +9,6 @@ namespace CKAN
     /// </summary>
     public static class SanityChecker
     {
-
         private static readonly ILog log = LogManager.GetLogger(typeof(SanityChecker));
 
         /// <summary>
@@ -26,57 +25,17 @@ public static ICollection<string> ConsistencyErrors(IEnumerable<CkanModule> modu
                 return errors;
             }
 
-            Dictionary<string, List<string>> providers = ModulesToProvides(modules, dlls);
-
-            foreach (KeyValuePair<string,List<CkanModule>> entry in FindUnmetDependencies(modules, dlls))
+            foreach (var kvp in FindUnsatisfiedDepends(modules.ToList(), dlls?.ToHashSet()))
             {
-                foreach (CkanModule unhappy_mod in entry.Value)
-                {
-                    // This error can fire if a dependency
-                    // IS listed in the index AND IS available for our version,
-                    // but not installed.
-                    // This happens when `dlls` is Registry.installed_dlls.Keys,
-                    // as in Registry.GetSanityErrors.
-                    errors.Add(string.Format(
-                        "{0} has an unmet dependency: {1} is not installed",
-                        unhappy_mod.identifier,
-                        entry.Key
-                    ));
-                }
+                errors.Add($"{kvp.Key} has an unsatisfied dependency: {kvp.Value} is not installed");
             }
 
             // Conflicts are more difficult. Mods are allowed to conflict with themselves.
             // So we walk all our mod conflicts, find what (if anything) provide those
             // conflicts, and return false if it's not the module we're examining.
-
-            // TODO: This doesn't examine versions. We should!
-            // TODO: It would be great to factor this into its own function, too.
-
-            foreach (CkanModule mod in modules)
+            foreach (var kvp in FindConflicting(modules.ToList(), dlls?.ToHashSet()))
             {
-                // If our mod doesn't conflict with anything, skip it.
-                if (mod.conflicts == null)
-                {
-                    continue;
-                }
-
-                foreach (var conflict in mod.conflicts)
-                {
-                    // If nothing conflicts with us, skip.
-                    if (! providers.ContainsKey(conflict.name))
-                    {
-                        continue;
-                    }
-
-                    // If something does conflict with us, and it's not ourselves, that's a fail.
-                    foreach (string provider in providers[conflict.name])
-                    {
-                        if (provider != mod.identifier)
-                        {
-                            errors.Add(string.Format("{0} conflicts with {1}.", mod.identifier, provider));
-                        }
-                    }
-                }
+                errors.Add($"{kvp.Key} conflicts with {kvp.Value}");
             }
 
             // Return whatever we've found, which could be empty.
@@ -97,7 +56,7 @@ public static void EnforceConsistency(IEnumerable<CkanModule> modules, IEnumerab
                 throw new InconsistentKraken(errors);
             }
         }
-    
+
         /// <summary>
         /// Returns true if the mods supplied can co-exist. This checks depends/pre-depends/conflicts only.
         /// </summary>
@@ -107,93 +66,64 @@ public static bool IsConsistent(IEnumerable<CkanModule> modules, IEnumerable<str
         }
 
         /// <summary>
-        /// Maps a list of modules and dlls to a dictionary of what's provided, and a list of
-        /// identifiers that supply each.
+        /// Find unsatisfied dependencies among the given modules and DLLs.
         /// </summary>
-        // 
-        // Eg: {
-        //          LifeSupport => [ "TACLS", "Snacks" ]
-        //          DogeCoinFlag => [ "DogeCoinFlag" ]
-        // }
-        public static Dictionary<string, List<string>> ModulesToProvides(IEnumerable<CkanModule> modules, IEnumerable<string> dlls = null)
+        /// <param name="modules">List of modules to check</param>
+        /// <param name="dlls">List of DLLs that can also count toward relationships</param>
+        /// <returns>
+        /// List of dependencies that aren't satisfied represented as pairs.
+        /// Each Key is the depending module, and each Value is the relationship.
+        /// </returns>
+        public static List<KeyValuePair<CkanModule, RelationshipDescriptor>>
+            FindUnsatisfiedDepends(IEnumerable<CkanModule> modules, HashSet<string> dlls)
         {
-            var providers = new Dictionary<string, List<string>>();
-
-            if (dlls == null)
-            {
-                dlls = new List<string>();
-            }
-
-            foreach (CkanModule mod in modules)
-            {
-                foreach (string provides in mod.ProvidesList)
-                {
-                    log.DebugFormat("{0} provides {1}", mod, provides);
-                    providers[provides] = providers.ContainsKey(provides) ? providers[provides] : new List<string>();
-                    providers[provides].Add(mod.identifier);
-                }
-            }
-
-            // Add in our DLLs as things we know exist.
-            foreach (string dll in dlls)
+            var unsat = new List<KeyValuePair<CkanModule, RelationshipDescriptor>>();
+            if (modules != null)
             {
-                if (! providers.ContainsKey(dll))
+                foreach (CkanModule m in modules.Where(m => m.depends != null))
                 {
-                    providers[dll] = new List<string>();
+                    foreach (RelationshipDescriptor dep in m.depends)
+                    {
+                        if (!dep.MatchesAny(modules, dlls))
+                        {
+                            unsat.Add(new KeyValuePair<CkanModule, RelationshipDescriptor>(m, dep));
+                        }
+                    }
                 }
-                providers[dll].Add(dll);
             }
-
-            return providers;
-        }
-
-        /// <summary>
-        /// Given a list of modules and optional dlls, returns a dictionary of dependencies which are still not met.
-        /// The dictionary keys are the un-met depdendencies, the values are the modules requesting them.
-        /// </summary>
-        public static Dictionary<string,List<CkanModule>> FindUnmetDependencies(IEnumerable<CkanModule> modules, IEnumerable<string> dlls = null)
-        {
-            return FindUnmetDependencies(modules, ModulesToProvides(modules, dlls));
-        }
-
-        /// <summary>
-        /// Given a list of modules, and a dictionary of providers, returns a dictionary of depdendencies which have not been met.
-        /// </summary>
-        internal static Dictionary<string,List<CkanModule>> FindUnmetDependencies(IEnumerable<CkanModule> modules, Dictionary<string,List<string>> provided)
-        {
-            return FindUnmetDependencies(modules, new HashSet<string> (provided.Keys));
+            return unsat;
         }
 
         /// <summary>
-        /// Given a list of modules, and a set of providers, returns a dictionary of dependencies which have not been met.
+        /// Find conflicts among the given modules and DLLs.
         /// </summary>
-        internal static Dictionary<string,List<CkanModule>> FindUnmetDependencies(IEnumerable<CkanModule> modules, HashSet<string> provided)
+        /// <param name="modules">List of modules to check</param>
+        /// <param name="dlls">List of DLLs that can also count toward relationships</param>
+        /// <returns>
+        /// List of conflicts represented as pairs.
+        /// Each Key is the depending module, and each Value is the relationship.
+        /// </returns>
+        public static List<KeyValuePair<CkanModule, RelationshipDescriptor>>
+            FindConflicting(IEnumerable<CkanModule> modules, HashSet<string> dlls)
         {
-            var unmet = new Dictionary<string,List<CkanModule>> ();
-
-            // TODO: This doesn't examine versions, it should!
-
-            foreach (CkanModule mod in modules)
+            var confl = new List<KeyValuePair<CkanModule, RelationshipDescriptor>>();
+            if (modules != null)
             {
-                // If this module has no dependencies, we're done.
-                if (mod.depends == null)
+                foreach (CkanModule m in modules.Where(m => m.conflicts != null))
                 {
-                    continue;
-                }
-
-                // If it does have dependencies, but we can't find anything that provides them,
-                // add them to our unmet list.
-                foreach (RelationshipDescriptor dep in mod.depends.Where(dep => ! provided.Contains(dep.name)))
-                {
-                    if (!unmet.ContainsKey(dep.name))
+                    // Remove self from the list, so we're only comparing to OTHER modules
+                    var others = modules.Where(other => other != m);
+                    foreach (RelationshipDescriptor dep in m.conflicts)
                     {
-                        unmet[dep.name] = new List<CkanModule>();
+                        if (dep.MatchesAny(others, dlls))
+                        {
+                            confl.Add(new KeyValuePair<CkanModule, RelationshipDescriptor>(m, dep));
+                        }
                     }
-                    unmet[dep.name].Add(mod); // mod needs dep.name, but doesn't have it.
                 }
             }
-
-            return unmet;
+            return confl;
         }
+
     }
-}
+}

Core/Types/CkanModule.cs

@@ -52,6 +52,44 @@ public bool version_within_bounds(Version other_version)
             return false;
         }
 
+        /// <summary>
+        /// Check whether any of the modules in a given list match this descriptor.
+        /// NOTE: Only proper modules can be checked for versions!
+        ///       DLLs match all versions, as do "provides" clauses.
+        /// </summary>
+        /// <param name="modules">Sequence of modules to consider</param>
+        /// <param name="dlls">Sequence of DLLs to consider</param>
+        /// <returns>
+        /// true if any of the modules match this descriptor, false otherwise.
+        /// </returns>
+        public bool MatchesAny(IEnumerable<CkanModule> modules, HashSet<string> dlls)
+        {
+            // DLLs are considered to match any version
+            if (dlls != null && dlls.Contains(name))
+            {
+                return true;
+            }
+            if (modules != null)
+            {
+                // See if anyone else "provides" the target name
+                // Note that versions can't be checked for "provides" clauses
+                if (modules.Any(m =>
+                    m.identifier != name && m.provides != null && m.provides.Contains(name)))
+                {
+                    return true;
+                }
+                // See if the real thing is there
+                foreach (CkanModule m in modules.Where(m => m.identifier == name))
+                {
+                    if (version_within_bounds(m.version))
+                    {
+                        return true;
+                    }
+                }
+            }
+            return false;
+        }
+
         /// <summary>
         /// A user friendly message for what versions satisfies this descriptor.
         /// </summary>
@@ -68,6 +106,27 @@ public string RequiredVersion
             }
         }
 
+        /// <summary>
+        /// Generate a user readable description of the relationship
+        /// </summary>
+        /// <returns>
+        /// Depending on the version properties, one of:
+        /// name
+        /// name version
+        /// name min_version -- max_version
+        /// name min_version or later
+        /// name max_version or earlier
+        /// </returns>
+        public override string ToString()
+        {
+            return
+                  version     != null                        ? $"{name} {version}"
+                : min_version != null && max_version != null ? $"{name} {min_version} -- {max_version}"
+                : min_version != null                        ? $"{name} {min_version} or later"
+                : max_version != null                        ? $"{name} {max_version} or earlier"
+                : name;
+        }
+
     }
 
     public class ResourcesDescriptor

Core/Types/Kraken.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Linq;
 using System.Net;
 using System.Text;
 using System.Collections.Generic;
@@ -154,7 +155,7 @@ public TooManyModsProvideKraken(string requested, List<CkanModule> modules, Exce
         internal static string FormatMessage(string requested, List<CkanModule> modules)
         {
             string oops = string.Format("Too many mods provide {0}:\r\n", requested);
-            return oops + String.Join("\r\n* ", modules);
+            return oops + String.Join("\r\n", modules.Select(m => $"* {m}"));
         }
     }
 
@@ -170,8 +171,7 @@ public string InconsistenciesPretty
         {
             get
             {
-                const string message = "The following inconsistencies were found:\r\n";
-                return message + String.Join("\r\n * ", inconsistencies);
+                return header + String.Join("\r\n", inconsistencies.Select(msg => $"* {msg}"));
             }
         }
 
@@ -189,8 +189,10 @@ public InconsistentKraken(string inconsistency, Exception innerException = null)
 
         public override string ToString()
         {
-            return InconsistenciesPretty + StackTrace;
+            return InconsistenciesPretty + "\r\n\r\n" + StackTrace;
         }
+
+        private const string header = "The following inconsistencies were found:\r\n";
     }
 
     /// <summary>