Update to mod adds dependency, CKAN upgrade doesn't install it?

[jakkarth]

I apologise if this issue is already open, I didn't see it when I searched. I'm using v1.6.23-0. It looks like Outer Planets Mod has somehow changed its dependency on Kopernicus (possibly due to Kopernicus 0.9 pre-release?). OPM is listed as updatable in the gui, so I tell it to upgrade and the progress bar just spins forever and it never finishes. I attempted to upgrade on the command line using ckan upgrade --all, and it downloaded the mod but I get a stack trace when it tries to install:

Unhandled Exception:
The following inconsistencies were found:
OuterPlanetsMod requires Kopernicus but nothing provides it  at CKAN.SanityChecker.EnforceConsistency (IEnumerable`1 modules, IEnumerable`1 dlls) [0x00000] in <filename unknown>:0 
  at CKAN.Registry.CheckSanity () [0x00000] in <filename unknown>:0 
  at CKAN.RegistryManager.Save (Boolean enforce_consistency) [0x00000] in <filename unknown>:0 
  at CKAN.ModuleInstaller.AddRemove (IEnumerable`1 add, IEnumerable`1 remove) [0x00000] in <filename unknown>:0 
  at CKAN.ModuleInstaller.Upgrade (IEnumerable`1 modules, CKAN.NetAsyncDownloader netAsyncDownloader) [0x00000] in <filename unknown>:0 
  at CKAN.CmdLine.Upgrade.RunCommand (CKAN.KSP ksp, System.Object raw_options) [0x00000] in <filename unknown>:0 
  at CKAN.CmdLine.MainClass.Main (System.String[] args) [0x00000] in <filename unknown>:0 
[ERROR] FATAL UNHANDLED EXCEPTION: The following inconsistencies were found:
OuterPlanetsMod requires Kopernicus but nothing provides it  at CKAN.SanityChecker.EnforceConsistency (IEnumerable`1 modules, IEnumerable`1 dlls) [0x00000] in <filename unknown>:0 
  at CKAN.Registry.CheckSanity () [0x00000] in <filename unknown>:0 
  at CKAN.RegistryManager.Save (Boolean enforce_consistency) [0x00000] in <filename unknown>:0 
  at CKAN.ModuleInstaller.AddRemove (IEnumerable`1 add, IEnumerable`1 remove) [0x00000] in <filename unknown>:0 
  at CKAN.ModuleInstaller.Upgrade (IEnumerable`1 modules, CKAN.NetAsyncDownloader netAsyncDownloader) [0x00000] in <filename unknown>:0 
  at CKAN.CmdLine.Upgrade.RunCommand (CKAN.KSP ksp, System.Object raw_options) [0x00000] in <filename unknown>:0 
  at CKAN.CmdLine.MainClass.Main (System.String[] args) [0x00000] in <filename unknown>:0

A few humble opinions:

1. It should not be possible for the gui to spin forever with no notification as to what's going on. An exception is thrown, so the problem is definitely detectable, so the user should be notified that a problem occurred and the process be cancelled. This should apply regardless of what caused the exception, as this is the second separate issue I've encountered that resulted in this behavior.
2. Kopernicus and KopernicusTech are both installable, but neither of them are currently installed. I infer from this that the dependency on Kopernicus et al was a recent addition to the OPM metadata. CKAN should detect that a new dependency exists when trying to upgrade and add the new dependency to the list of mods to be installed, rather than attempting to upgrade OPM and find out half way through that there is an unmet dependency.
3. Using CKAN is still way better than trying to manage the dependencies by hand. Thank you all for all your hard work.

[dbent]

It's probably the same issue as #795. OuterPlanetsMod previously depended on KopernicusTech and then with the v1.7 beta series, took a dependency on Kopernicus instead (not the same thing, and barely related to KopernicusTech). However, Kopernicus did not have a stable release for KSP 1.0 at that point so each mod using it was providing their own version, none of which were guaranteed to work with each other. So for the betas OuterPlanetsMod installed its own version of Kopernicus and conflicted with anything else using Kopernicus. With the release of v1.7 final, OuterPlanetsMod no longer distributes it's own copy of Kopernicus and can use a standard install. CKAN is probably choking on the fact that the new version of OuterPlanetsMod wants to install Kopernicus but the existing OuterPlanetsMod has conflicting files.

Long story short:

1. Remove OuterPlanetsMod
2. Install OuterPlantesMod
3. Rejoice.

[dbent]

For future reference you can reproduce by:

> ckan install OuterPlanetsMod=1.7_Beta_2.repackaged0
> ckan upgrade OuterPlanetsMod=1:1.7

[jakkarth]

Following those steps has resolved the OPM upgrade issue, thanks!

My points in my original post still stand. I understand that this situation is a little unusual, and that because of the conflicting files it would be difficult to resolve the problem in an automated fashion. However, this slightly different problem is still detectable, and the resolution steps are known.

• If there's a new dependency added to a mod, and CKAN can't satisfy it, CKAN should not attempt to install the mod without it.
• If there's a conflicting file problem that would cause CKAN to refuse to add the dependency to the install list, the user should be notified that there's a conflict and why the dependency can't be installed. There was no indication of this in either the GUI or command line. In fact, there was no indication that CKAN even considered installing the dependency.
• If the solution is to uninstall and reinstall, CKAN should either do that automatically or notify the user as to the nature of the problem and the suggested fix.
• CKAN gui should catch exceptions thrown during install and handle them as gracefully as possible, at the least giving a generic error message and cancelling the install dialog.

This issue has different command line error output from #795, though the causal circumstances do sound the same.

[dbent]

Yes this should be handled gracefully which is why I'm leaving the issue open.

Really the problem seems to be that CKAN is checking for consistency within a transaction, which it shouldn't care about. The only thing that CKAN should care about is that before a transaction the state of the world is consistent, and then that after a transaction the state of the world is consistent. Anything should be allowed to happen within a transaction so long as the end result is consistent.

[jakkarth]

Awesome, thanks again!

[dbent]

There are two issues here from my investigation:

1. upgrade --all: This consumes ModuleInstaller.Upgrade(IEnumerable<CkanModule>, NetAsyncDownloader), which does not perform relationship resolution. When using upgrade --all the upgrade then fails because OuterPlanetsMod depends on Kopernicus but nothing which provides it is being installed, because relationship resolution wasn't run.

   To my eye ModuleInstaller.Upgrade(IEnumerable<CkanModule>, NetAsyncDownloader) should be private and only consumed by ModuleInstaller.Upgrade(IEnumerable<string>, NetAsyncDownloader). Alternatively, the relationship resolution could be moved out of ModuleInstaller.Upgrade(IEnumerable<string>, NetAsyncDownloader) and into ModuleInstaller.Upgrade(IEnumerable<CkanModule>, NetAsyncDownloader).

   I'm interested to know if there's any reason why relationship resolution isn't run in some cases.

2. upgrade OuterPlanetsMod: This does perform relationship resolution, but fails on a psuedo-conflict. The relationship resolution correctly brings in Kopernicus as a dependency, but when its relationships are checked it triggers a conflict with OuterPlanetsMod=1.7_Beta_2.repackaged0 (the currently installed version), which while correct, is ineffectual because it's the version that will be uninstalled in the same transaction.

As mentioned above, I think the correct thing to do here is:

1. Always resolve depends relationships before any kind of upgrade at the very beginning of the transaction. This should fix the upgrade --all bust.
2. Defer conflicts resolution until the very end of a transaction. This should fix the upgrade OuterPlanetsMod bust.

[pjf]

Gosh dang this thread rocks. Seriously.

@jakkarth is dead on about this needing to be handled by the client sensibly.

@dbent's suggestion on how to resolve this seems solid, but I'm on day five of a teaching sprint (I'm the teacher!) so treat my opinions as ones made in a rather hazy state. :)

Marking this ticket as a bug, because it most definitely is. :)

[jakkarth]

Defer conflicts resolution until the very end of a transaction. This should fix the upgrade OuterPlanetsMod bust.

Just out of curiosity, why would we do it at the end of the transaction? We have the metadata about the final versions of all the mods involved at the start of the transaction; can we not verify there will be no conflicts before we spend the 10 minutes downloading and installing, rather than after? I know nothing of the way CKAN works internally but this seems like a strange choice :)

[dbent]

@jakkarth I'm talking in an abstract sense. We obviously shouldn't spend the time to download/install mods that will eventually conflict, but I consider than an implementation detail / optimization since the functional behavior would be equivalent in either case.

[jakkarth]

Ah, I see, makes sense.

[plague006]

This bug exists through 1.10.3