Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Obey version properties of conflicts and depends relationships in sanity checks #2339

Merged
merged 1 commit into from
Mar 4, 2018

Conversation

HebaruSan
Copy link
Member

Background

Kopernicus has gotten into a habit of making frequent compatibility breaks with previous versions, with the expectation that planet pack makers will release updates to handle the new version. CKAN users would usually be burned by this, as we upgrade their Kopernicus before the planet packs are ready.

A conscientious planet pack maker may try to version-lock his mod to a specific version of Kopernicus to ensure that an upgrade does not take place until the dependent mod is ready for it.

Problems

If a mod depends on a specific version of another mod, such as this from @Sigma88's StockalikeSolarSystem:

    "depends": [
        {
            "name": "Kopernicus",
            "version": "2:release-1.3.1-3"
        },
        {
            "name": "SigmaDimensions",
            "version": "0.9.6"
        },

... then the correct dependency version will be pulled in at the initial install, but an upgrade command will happily replace it with a later version, breaking the dependency. And CKAN would never notice. This means that even if a planet pack maker goes to the trouble of setting up version-specific metadata, their CKAN users will get burned anyway.

Similarly, a version-specific conflict such as:

    "conflicts": [ { "name": "SigmaBinary-core", "version": "1:v1.6.11" } ],

... is currently applied to all versions of the conflicting mod, so not only can you not install the conflicting version of that mod, you can't install any version of it. This problem rules out the use of this kind of metadata as a way of limiting the allowed versions for recommendations and suggestions.

Cause

In SanityChecker.cs, both conflict checking and dependency checking completely ignore module versions. Conflicts and dependencies are handled solely in terms of their plain identifiers.

// 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.

/// <summary>
/// Given a list of modules, and a set of providers, returns a dictionary of dependencies which have not been met.
/// </summary>
internal static Dictionary<string,List<CkanModule>> FindUnmetDependencies(IEnumerable<CkanModule> modules, HashSet<string> provided)
{
var unmet = new Dictionary<string,List<CkanModule>> ();
// TODO: This doesn't examine versions, it should!

Changes

Now all three of those TODOs are resolved:

  • Conflict checking is split into a standalone function
  • Conflict checking obeys version specifications
  • Dependency checking obeys version specifications

This means that if you try to upgrade a module past the version where another module's depends clause wants it to be, you now get errors:

$ ckan upgrade --all

Upgrading modules...

The following inconsistencies were found:
* StockalikeSolarSystem v0.5.5 has an unsatisfied dependency: Kopernicus 2:release-1.3.1-3 is not installed
* StockalikeSolarSystem v0.5.5 has an unsatisfied dependency: SigmaDimensions 0.9.6 is not installed
* StockalikeSolarSystem v0.5.5 conflicts with SigmaBinary-core 1:v1.6.11

  at CKAN.SanityChecker.EnforceConsistency (System.Collections.Generic.IEnumerable`1[T] modules, System.Collections.Generic.IEnumerable`1[T] dlls) [0x0001e] in <e2a7ca14012c4404a5ab8dd1504e35ee>:0 
  at CKAN.Registry.CheckSanity () [0x00038] in <e2a7ca14012c4404a5ab8dd1504e35ee>:0 
  at CKAN.RegistryManager.Save (System.Boolean enforce_consistency) [0x0001d] in <e2a7ca14012c4404a5ab8dd1504e35ee>:0 
  at CKAN.ModuleInstaller.AddRemove (System.Collections.Generic.IEnumerable`1[T] add, System.Collections.Generic.IEnumerable`1[T] remove, System.Boolean enforceConsistency) [0x0006b] in <e2a7ca14012c4404a5ab8dd1504e35ee>:0 
  at CKAN.ModuleInstaller.Upgrade (System.Collections.Generic.IEnumerable`1[T] modules, CKAN.IDownloader netAsyncDownloader, System.Boolean enforceConsistency) [0x00159] in <e2a7ca14012c4404a5ab8dd1504e35ee>:0 
  at CKAN.CmdLine.Upgrade.RunCommand (CKAN.KSP ksp, System.Object raw_options) [0x00337] in <e2a7ca14012c4404a5ab8dd1504e35ee>:0 

To make this happen, RelationshipDescriptor now has a public MatchesAny function that can be used to see whether the given relationship is satisfied by any of a list of modules. This logic can be used the same way for both dependencies and conflicts.

The TooManyModsProvideKraken and InconsistentKraken messages are now cleaned up to print the previously missing bullet point for the first item in the list, caused by putting the bullet point in the delimiter for String.Join. And where the stack trace previously started on the same line as the last line of the main error message, now there's a blank line between those two sections.

Fixes #2324.
Fixes #2325.

@HebaruSan HebaruSan added Bug Pull request Relationships Issues affecting depends, recommends, etc. and removed Bug labels Mar 3, 2018
@politas politas merged commit 99f0417 into KSP-CKAN:master Mar 4, 2018
politas added a commit that referenced this pull request Mar 4, 2018
@HebaruSan HebaruSan deleted the fix/version-sanity branch March 4, 2018 06:10
@HebaruSan HebaruSan mentioned this pull request Apr 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Relationships Issues affecting depends, recommends, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conflict checking ignores "version" property Upgrade ignores "version" property of "depends"
2 participants