Upgrade ignores "version" property of "depends" #2324

Sigma88 · 2018-02-26T20:37:56Z

CKAN Version:
1.24.0

KSP Version:
1.3.1

Operating System:
Windows 10

Have you made any manual changes to your GameData folder (i.e., not via CKAN)?
no

Problem

What steps did you take in CKAN?
On a clean installation of KSP 1.3.1 open ckan.exe, select the mod StockalikeSolarSystem and install.

Notice that this mod requires a specific version of other mods to be installed.

Once the installation is complete, click on "Add Available Updates". Then Apply,

What happened?

some of the mods that were installed at a specific version to fulfill StockalikeSolarSystem requirements have been updated to the latest version, breaking compatibility with SASS and potentially damaging saves if the player ignores the warnings

What did you expect to happen?
some kind of warning, or red flag signalling that the update was not recommended.

Came up during discussion of #2319.

HebaruSan · 2018-02-26T22:35:09Z

E.g.:

	"depends": [
		{ "name": "ModuleManager" },
		{ "name": "Kopernicus", "version": "2:release-1.3.1-3" },
		{ "name": "SigmaDimensions", "version": "0.9.6" },
...
	],
	"recommends": [
		{ "name": "SigmaBinary-core", "version": "1:v1.6.9" },
		{ "name": "SigmaSciDefRenamer" },
		{ "name": "Kronometer", "version": "v1.3.1-1" }
	],

$ ckan list

KSP found at /home/user/.steam/steam/SteamApps/common/Kerbal Space Program

KSP Version: 1.3.1

Installed Modules:

^ Kopernicus 2:release-1.3.1-3
^ Kronometer v1.3.1-1
- KSPSteamCtrlr autodetected dll
- ModularFlightIntegrator 1.2.4.0
- ModuleManager 3.0.4
- SASS-DRP 2.0
- SASS-ER 1.2.4
- SASS-NH 2.0.1
- SASS-OPM 1.2.4
- SASS-RevJ 1
- SASS-RevSS 0.6
- SASS-Saru 0.1
- SASS-SE 1.0
- SASS-StockalikeNeptune 1
- SASS-UL 0.5.4
^ SigmaBinary-core 1:v1.6.9
^ SigmaDimensions 0.9.6
- SigmaSciDefRenamer v0.3.2
- Steamworks autodetected dll
- StockalikeSolarSystem v0.5.5

Legend: -: Up to date. X: Incompatible. ^: Upgradable. ?: Unknown. *: Broken. 

$ ckan upgrade --all

Upgrading modules...

Downloading "https://github.com/Kopernicus/Kopernicus/releases/download/release-1.3.1-7/Kopernicus-1.3.1-7.zip"
Downloading "https://github.com/Sigma88/Sigma-Dimensions/releases/download/v0.9.7/Sigma-Dimensions.v0.9.7.zip"
Downloading "https://github.com/Sigma88/Sigma-Binary/releases/download/v1.6.11/Sigma-Binary.v1.6.11.zip"
0 B/sec - downloading - 0 B left - 100%                    
Done!

$ ckan list

KSP found at /home/user/.steam/steam/SteamApps/common/Kerbal Space Program

KSP Version: 1.3.1

Installed Modules:

- Kopernicus 2:release-1.3.1-7
- Kronometer v1.3.1-2
- KSPSteamCtrlr autodetected dll
- ModularFlightIntegrator 1.2.4.0
- ModuleManager 3.0.4
- SASS-DRP 2.0
- SASS-ER 1.2.4
- SASS-NH 2.0.1
- SASS-OPM 1.2.4
- SASS-RevJ 1
- SASS-RevSS 0.6
- SASS-Saru 0.1
- SASS-SE 1.0
- SASS-StockalikeNeptune 1
- SASS-UL 0.5.4
- SigmaBinary-core 1:v1.6.11
- SigmaDimensions 0.9.7
- SigmaSciDefRenamer v0.3.2
- Steamworks autodetected dll
- StockalikeSolarSystem v0.5.5

Legend: -: Up to date. X: Incompatible. ^: Upgradable. ?: Unknown. *: Broken.

This means that version-specific dependencies can only be used to hold back versions at initial installation, so they're nearly useless for the much more common use case of determining whether a newly available update is compatible with your installed mods.

Sigma88 · 2018-02-26T23:23:13Z

thanks @HebaruSan

HebaruSan · 2018-02-27T00:05:34Z

No problem, @Sigma88. FYI, I just tested with this added to my local copy of SASS's metadata as an experiment:

"conflicts": [
    { "name": "Kopernicus", "version": "2:release-1.3.1-4" },
    { "name": "Kopernicus", "version": "2:release-1.3.1-5" },
    { "name": "Kopernicus", "version": "2:release-1.3.1-6" },
    { "name": "Kopernicus", "version": "2:release-1.3.1-7" }
],

... and ckan upgrade --all still upgraded Kopernicus 2:release-1.3.1-7, without prompting me or even telling me what it updated after the fact (I had to run ckan list afterwards to check on it). Looks like a very generalized problem with the upgrader.

(Note, I'm not at all suggesting that we would spam the metadata with conflicts like that, it's not at all a sustainable solution to require every planet pack author update their netkan with every Kopernicus update.)

HebaruSan · 2018-02-27T00:16:06Z

Actually, I may have messed up that test. I put the new metadata in available_modules instead of installed_modules, after installation. But if I do it before installation, it refuses to install at all, even though there's a non-conflicting version of Kopernicus available in the registry and specified as a depends:

$ ckan install StockalikeSolarSystem --ksp steam
The following inconsistencies were found:
StockalikeSolarSystem conflicts with Kopernicus.
Install canceled. Your files have been returned to their initial state.

Argh, conflict checking ignores versions!

CKAN/Core/Relationships/SanityChecker.cs

Line 52 in 86bf144

// TODO: This doesn't examine versions. We should!

Another note, we now have a Relationships tag to track these arcane dependency issues. I used CKAN for howmany years and never realized this stuff wasn't handled.

HebaruSan · 2018-02-27T00:25:31Z

OK, better test...

Installed SASS with the current normal metadata
Added the above "conflicts" block to SASS in installed_modules

$ ckan upgrade --all
2281 [1] ERROR CKAN.RegistryManager (null) - Loaded registry with inconsistencies:

The following inconsistencies were found:
StockalikeSolarSystem conflicts with Kopernicus.

Upgrading modules...


Unhandled Exception:
The following inconsistencies were found:
StockalikeSolarSystem conflicts with Kopernicus.  at CKAN.SanityChecker.EnforceConsistency (System.Collections.Generic.IEnumerable`1[T] modules, System.Collections.Generic.IEnumerable`1[T] dlls) [0x00017] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.Registry.CheckSanity () [0x00035] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.RegistryManager.Save (System.Boolean enforce_consistency) [0x00018] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.ModuleInstaller.AddRemove (System.Collections.Generic.IEnumerable`1[T] add, System.Collections.Generic.IEnumerable`1[T] remove, System.Boolean enforceConsistency) [0x0005f] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.ModuleInstaller.Upgrade (System.Collections.Generic.IEnumerable`1[T] modules, CKAN.IDownloader netAsyncDownloader, System.Boolean enforceConsistency) [0x00130] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.CmdLine.Upgrade.RunCommand (CKAN.KSP ksp, System.Object raw_options) [0x002be] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.CmdLine.MainClass.RunSimpleAction (CKAN.CmdLine.Options cmdline, CKAN.CmdLine.CommonOptions options, System.String[] args, CKAN.IUser user, CKAN.KSPManager manager) [0x0040a] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.CmdLine.MainClass.Execute (CKAN.KSPManager manager, CKAN.CmdLine.CommonOptions opts, System.String[] args) [0x0015b] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.CmdLine.MainClass.Main (System.String[] args) [0x000a8] in <cf1172f4644841da8b9b83c88c51f181>:0 
[ERROR] FATAL UNHANDLED EXCEPTION: The following inconsistencies were found:
StockalikeSolarSystem conflicts with Kopernicus.  at CKAN.SanityChecker.EnforceConsistency (System.Collections.Generic.IEnumerable`1[T] modules, System.Collections.Generic.IEnumerable`1[T] dlls) [0x00017] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.Registry.CheckSanity () [0x00035] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.RegistryManager.Save (System.Boolean enforce_consistency) [0x00018] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.ModuleInstaller.AddRemove (System.Collections.Generic.IEnumerable`1[T] add, System.Collections.Generic.IEnumerable`1[T] remove, System.Boolean enforceConsistency) [0x0005f] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.ModuleInstaller.Upgrade (System.Collections.Generic.IEnumerable`1[T] modules, CKAN.IDownloader netAsyncDownloader, System.Boolean enforceConsistency) [0x00130] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.CmdLine.Upgrade.RunCommand (CKAN.KSP ksp, System.Object raw_options) [0x002be] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.CmdLine.MainClass.RunSimpleAction (CKAN.CmdLine.Options cmdline, CKAN.CmdLine.CommonOptions options, System.String[] args, CKAN.IUser user, CKAN.KSPManager manager) [0x0040a] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.CmdLine.MainClass.Execute (CKAN.KSPManager manager, CKAN.CmdLine.CommonOptions opts, System.String[] args) [0x0015b] in <cf1172f4644841da8b9b83c88c51f181>:0 
  at CKAN.CmdLine.MainClass.Main (System.String[] args) [0x000a8] in <cf1172f4644841da8b9b83c88c51f181>:0

So once again we see that conflict checking ignores versions, which makes CKAN think that this install is inconsistent even before the upgrade, even though it shouldn't be. On the bright side, at least it didn't install the incompatible version this time:

$ ckan list
2260 [1] ERROR CKAN.RegistryManager (null) - Loaded registry with inconsistencies:

The following inconsistencies were found:
StockalikeSolarSystem conflicts with Kopernicus.

KSP found at /home/user/.steam/steam/SteamApps/common/Kerbal Space Program

KSP Version: 1.3.1

Installed Modules:

^ Kopernicus 2:release-1.3.1-3
^ Kronometer v1.3.1-1
- KSPSteamCtrlr autodetected dll
- ModularFlightIntegrator 1.2.4.0
- ModuleManager 3.0.4
- SASS-DRP 2.0
- SASS-ER 1.2.4
- SASS-NH 2.0.1
- SASS-OPM 1.2.4
- SASS-RevJ 1
- SASS-RevSS 0.6
- SASS-Saru 0.1
- SASS-SE 1.0
- SASS-StockalikeNeptune 1
- SASS-UL 0.5.4
^ SigmaBinary-core 1:v1.6.9
^ SigmaDimensions 0.9.6
- SigmaSciDefRenamer v0.3.2
- Steamworks autodetected dll
- StockalikeSolarSystem v0.5.5

Legend: -: Up to date. X: Incompatible. ^: Upgradable. ?: Unknown. *: Broken.

I'm going to split this out into yet another issue...

HebaruSan · 2018-03-02T20:38:55Z

ModuleInstaller.Upgrade runs a consistency check at the end, and SanityChecker.FindUnmetDependencies is responsible for finding unsatisfied dependencies. It ignores versions:

CKAN/Core/Relationships/SanityChecker.cs

Lines 167 to 175 in b75a450

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

Though fixing that still wouldn't result in an optimal user experience. I think CKAN would still try to upgrade everything, but then raise error messages afterwards and revert. Ideally it would avoid incompatible upgrades at the beginning, with a way for the user to figure out why.

HebaruSan added Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN labels Feb 26, 2018

HebaruSan changed the title ~~Version updates ignore "Requires" specifications~~ Upgrade ignores "version" property of "depends" Feb 26, 2018

HebaruSan added the Relationships Issues affecting depends, recommends, etc. label Feb 26, 2018

HebaruSan mentioned this issue Feb 27, 2018

Conflict checking ignores "version" property #2325

Closed

HebaruSan mentioned this issue Mar 3, 2018

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

Merged

HebaruSan self-assigned this Mar 3, 2018

politas closed this as completed in #2339 Mar 4, 2018

politas removed the Bug Something is not working as intended label Mar 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade ignores "version" property of "depends" #2324

Upgrade ignores "version" property of "depends" #2324

Sigma88 commented Feb 26, 2018 •

edited by HebaruSan

Loading

HebaruSan commented Feb 26, 2018 •

edited

Loading

Sigma88 commented Feb 26, 2018

HebaruSan commented Feb 27, 2018

HebaruSan commented Feb 27, 2018

HebaruSan commented Feb 27, 2018 •

edited

Loading

HebaruSan commented Mar 2, 2018 •

edited

Loading

Upgrade ignores "version" property of "depends" #2324

Upgrade ignores "version" property of "depends" #2324

Comments

Sigma88 commented Feb 26, 2018 • edited by HebaruSan Loading

Problem

HebaruSan commented Feb 26, 2018 • edited Loading

Sigma88 commented Feb 26, 2018

HebaruSan commented Feb 27, 2018

HebaruSan commented Feb 27, 2018

HebaruSan commented Feb 27, 2018 • edited Loading

HebaruSan commented Mar 2, 2018 • edited Loading

Sigma88 commented Feb 26, 2018 •

edited by HebaruSan

Loading

HebaruSan commented Feb 26, 2018 •

edited

Loading

HebaruSan commented Feb 27, 2018 •

edited

Loading

HebaruSan commented Mar 2, 2018 •

edited

Loading