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

Add fuzzy versioning to the CKAN. #1499

Merged
merged 15 commits into from
Dec 13, 2015
Merged

Add fuzzy versioning to the CKAN. #1499

merged 15 commits into from
Dec 13, 2015

Conversation

pjf
Copy link
Member

@pjf pjf commented Oct 20, 2015

The buzz is that 1.0.5 will be out soon-ish, and will be mostly compatible
with 1.0.4. However the last time we had a highly compatible KSP release,
it wasn't fun for CKAN users. This PR hopes to fix that.

These changes all relate to #1173, and include:

IGameComparator

Rather than having only a single way to test if a mod is compatible, we now have a whole family that conform to the IGameComparator interface. The classic mode the CKAN used is the 'Strict' version, but I've now adjusted the default to be the 'Generally Recognised As Safe' (GRAS) version, which will allow installing mods from 1.0.4 on 1.0.5 unless they're flagged otherwise.

Also included for completeness is 'You're On Your Own' (YoYo) comparisons, which let you do anything.

At the moment there's no way for the user to switch comparators, but basics of doing that are here.

Autofac

Autofac is an inversion of control / dependency injection framework. It means that when we create instances Autofac can supply them with dependencies. We only use this for IGameComparator at the moment, but it has potential to make our tests better, and for us to set a variety of options at start-up in a more sane fashion.

Right now we'reusing the Service locator (anti-)pattern, which isn't my favourite, but it means we didn't need to re-achitecture everything.

Schema updates

We introduce a new keyword ksp_version_strict into the spec, which defaults to false. If set to true, it says that says that comparisons must always be strictly performed. The idea is we can flip this on for any 1.0.4 mods we know don't work in 1.0.5.

Feedback?

This isn't a complete implementation, and I'd love feedback. Right now the user can't switch comparison modes, and there's a risk that we may try to choose an older mod as being more compatible than a more recent mod. However I think that's a separate change that's independent to this work, so I hope to throw in a separate PR for it.

@pjf
Copy link
Member Author

pjf commented Oct 20, 2015

I've also made sure I'm pulling from the KSP-CKAN repo, so feel free to add extra commits as needed with revisions if needed. :)

@dbent : If you have a chance, can you look over this for sanity? I know you're the Wizard of Autofac, so I'd love to know if I'm off in a completely wrong direction.

pjf added 2 commits October 20, 2015 20:35
My git whines without this, and .xml files are cool with it.
* tidy_spaces:
  AvailableModule: Documentation and spacing fixes only.
  Update Changelog
  Add conflicts relationship to sort order
@pjf
Copy link
Member Author

pjf commented Oct 20, 2015

Potential gotcha

At the moment, we default strict mode to false, meaning all 1.0.4 mods will be 1.0.5 installable unless marked otherwise. However imagine the following:

  • FictionalKerbalHeatMod has four releases for KSP 1.0.4, but is not 1.0.5 compatible.
  • We mark it as ksp_version_strict in the netkan file, but only the latest release gets that flag.
  • When 1.0.5 comes out, the CKAN tries to install an old version of FictionalKerbalHeatMod.
  • Derp.

Unsatisfying solution

When we need to enforce strict mode, we simply tag all documents that target KSP 1.0.4 that don't work with 1.0.5. It requires human effort, and we'd have to bump to spec_version at the same time, but it's not a disaster. I can do this on the command-line with perl; so no matter what, we have a solution, even if it's not a satisfying one.

Satisfying solution

In an ideal world, as AvailableModule.Latest() looks back through our list of versions to install, it should halt if a version is rejected because our KSP version is "too new". This means we should never pick an older version because it's more forward-compatible than a newer version. (I think @Dazpoet or @plague006 made this point once, and I'm really starting to agree with it.)

Unfortunately, I've spent a good hour trying to find a decent way to say "hey, stop looking" in AvailableModule.Latest(), but don't have one yet. Admittedly I am on a two day sprint, and after 20-something hours of coding I think whatever neurotransmitters I use for logic are severely depleted.

Other solutions

I'm sure they're out there. Suggestions welcome.

TL;DR: We may have a little extra metadata finangling for mods we want to place into strict mode, but way less than the amount of effort needed when 1.0.5 comes out and is "mostly compatible" with 1.0.4.

@dbent
Copy link
Member

dbent commented Oct 21, 2015

Cool, I'll probably look at this tomorrow (~12 hours from now).

return _container;
}

set
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any need to make this set-able?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for testing! And for user preferences!

@dbent
Copy link
Member

dbent commented Oct 21, 2015

👍 with comments.

@pjf
Copy link
Member Author

pjf commented Oct 22, 2015

@dbent : You do the best code reviews! Shall add some commits that address your comments! Thank you so much!

@Dazpoet
Copy link
Member

Dazpoet commented Oct 30, 2015

These things sound pretty freaking amazing 👍

Also anything making CKAN stop from using older versions due to being more forward compatible would be pretty amazing but last I checked (which admittedly was quite some time ago) @plague006 was on the case and hunting down any mod daring to be non-conforming to his rigorus demands of uniformity :)

@Olympic1
Copy link
Member

POKE

@mheguy
Copy link
Contributor

mheguy commented Nov 10, 2015

Some mods that will need the strict version flag:

  • ContractConfigurator
  • ChuteSafetyIndicator
  • ContractConfigurator-AnomalySurveyor
  • Trajectories
  • WaypointManager
  • ModuleAnimateEmissive
  • SCANsat
  • ScienceAlert
  • DMagicOrbitalScience
  • RemoteTech
  • AGExt
  • RasterPropMonitor
  • IntakeBuildAid
  • TweakScale
  • Kopernicus

The preceding were all .netkans, the following are .ckans only:

  • TweakableEverything-EngineStagingToggle (will not be updated in the future)
  • TweakableEverything-FairingStagingToggle (will not be updated in the future)
  • MechJeb2

@solarsootysmudge
Copy link

There is no point in going for another pull request just yet to do the same as #1513. However can we look at this for the opposite reason given in the request?

Can we be given an option to force application of ksp_version_strict on an single mod where the assumed fuzzy version metadata turns out to be potentially wrong. Almost all mods are 1.0.5 compatible and are now appearing as such. However there are some on the allowed list that should not be there ...

[1.0] CrossFeedEnabler v3.3 May 11 (approved for 1.0.5. Is it not in stock now? - Asking becuase it was done by the man that can best answer the question)
[1.0.x - OBSOLETE] ClampsBeGone - Stop clamps following your vessels into flight! (yes it is obsolete in 1.0.5 so can't be 1.0.5 approved)

More from @plague006

This comes from the original concept post seeking solutions as an alternative with an added....

TL;DR: We may have a little extra metadata finangling for mods we want to place into strict mode, but way less than the amount of effort needed when 1.0.5 comes out and is "mostly compatible" with 1.0.4.

@Y3mo
Copy link

Y3mo commented Nov 12, 2015

How about doing nothing of the sort and just enabling users to install all mods instead of only the ones in the "compatible" list?
Then the responsibility for installing not specifically marked compatible versions is where it actually belongs, with the users installing them.
If users want a "controlled environment", they can just use the default "compatible" list.

edit: Might display an additional warning or something, when a version is installed which is not marked as compatible.

@mheguy mheguy added this to the Fuzzy versioning milestone Nov 12, 2015
@lamont-granquist
Copy link

How about you just let users force install a mod and ignore the kerbal version on that install, and do no depsolving of deps (either fail due to deps not being there and tell users which deps need to be force installed first, or just install for the user and warn that deps haven't been installed).

This PR seems to be getting lost trying to solve a very hard problem that has a ton of edge conditions and needs a lot of clever magic to try to make it work. You can simplify the problem a lot by just rejecting the depsolving problem--which is complicated a lot by old packages that necessarily lacking metadata that couldn't have been added to them because packages don't know when they're written what their "real" maximum kernel version will be.

But really all I want right now is a button that lets me install the v1.0.5-compatible Adjustable Landing Gear mod. If you force me to do my own depsolving, that is fine. Just blow up if there's a dep that isn't satisfied and make me manually go force install every dep and do the validation myself that the whole tree is really v1.0.5 compatible.

And I offer this advice as a software developer who has fought with both the rubygems depsolver and the depsolver for chef cookbooks for the past 4 years. Depsolving is a really hard problem, and you're asking your depsolver to try to mitigate the problem of past metadata not being able to predict the future.

@solarsootysmudge
Copy link

@lamont-granquist. I am neither a contributor or a programmer. So please take the following with a pinch of salt. What I do is help with the financial funding a little bit and have watched the project though it's up and downs. That is all. So please forgive me if I give a silly answer here.

Your are asking for forced installs. After a somewhat detailed discussion #1173. We ended up here. Up to now compatability checking has be included with the mod spec_version. With the mod being thrown out if it did not strictly match. That was ok when every update of KSP broke things. Now with stable builds the opposite is true.

Mods can now be assumed to be compatible. With the new default setting letting you instal mods as KSP updates. The 'Generally Recognised As Safe' (GRAS) option. Right now installing the v1.0.5-compatible Adjustable Landing Gear mod should be as simple as ticking it. The default settings list it as compatible along with the dependency in the Animation Modules. Max KSP version in the current metadata might be 1.0.4 but the default filter option is now GRAS. At least that how I see it CKAN 1.14.3

However mod owners know best and can stop this default. CKAN serves two groups and the first is obvious: mod users. The second is indirect but no less important: the mod authors. If they say a new version of KSP always breaks things so it goes. This only affects a few mods which can use the 'Strict' option of version control on request. The user community can also help here by giving feedback on what has become incompatible. See #2475 as an example.

Of course the end user can still throw caution to the wind and force installs. The completeness of adding in the 'You're On Your Own' (YoYo) option. Which what I think your asking for. Can be done but there no reason to throw out the first two solutions.

Logically there is several outcomes depending on how the filtering is chosen by the end user or mod author. This is an attempt to come up with a solution that tries to please everyone. However difficult that may be. It is worth trying.

@mheguy
Copy link
Contributor

mheguy commented Dec 6, 2015

Just throwing it out there that the 1.0.4 -> 1.0.5 window has basically closed. Anything compatible with 1.0.5 (should) be marked as such by now. But this can still be useful for the 1.0.X -> 1.1 transition. (or 1.1.0 -> 1.1.X).

@solarsootysmudge
Copy link

A quick question how much does it matter when we get odd version numbers?

Having problems getting it to work in a 1.0.4 build. I am looking at some "exotic" version numbering system by CKAN standards with EVE. What I am seeing in the CKAN file search is.

Running KSP 1.0.4

EVE version number is 7-4
With low resolution config version number also 7-4 installed it works.
High resolution file is listed as EVE-1.05-1 but it does not work. ( i think the bot has found the wrong config)

When running with KSP 1.0.5

EVE version is now 1:EVE-1.05-4 and there is only one config file also marked 1:EVE-1.05-4

I am think the character string version numbers are messing up the CKAN bot. A possible candidate for very fuzzy version control?

@mheguy
Copy link
Contributor

mheguy commented Dec 10, 2015

@nobodyhasthis

EVE is odd in KSP 1.0.4.

This PR isn't about mod version numbers but rather about ksp_version compatibility fuzziness. With that said I won't ignore the issue you brought up even though unrelated. The EVE situation you're describing arose when a new version of EVE was released for 1.0.5 but things got "messy". This should be fixed with KSP-CKAN/CKAN-meta#859.

@pjf
Copy link
Member Author

pjf commented Dec 13, 2015

Oh my, catching up on my own pull requests here.

@plague006:

Just throwing it out there that the 1.0.4 -> 1.0.5 window has basically closed. Anything compatible with 1.0.5 (should) be marked as such by now. But this can still be useful for the 1.0.X -> 1.1 transition. (or 1.1.0 -> 1.1.X).

I wasn't too stressed about this missing 1.0.5, because it turned out that 1.0.4 and 1.0.5 had a lot of incompatible changes between them, and before the release I had a polite request from those close to Squad that 1.0.5 be considered a significant new release.

Having said that, I still think these features are definitely something we want in the client. It would have been great for 1.0.3 -> 1.0.4, and my software sense tells me there's a high likelihood of it being useful with 1.1.0 -> 1.1.1.

@nobodyhasthis : Thank you for your awesome explanations! You rock!

@lamont-granquist : As it happens, this patch also provides the foundation to turn off version checks entirely, if that's what you want. Just engage the YoyoGameComparator and you're golden. In the GUI that will probably be a "restart in the DANGER ZONE" style option; on the command-line we'll probably have a switch --versioning=yoyo or similar.

In any case I'm going to finish my changes to @dbent's review, since I believe this is something we want in the core even if we haven't got a front-end for it.

P.S. You all rock!

pjf added 6 commits December 13, 2015 15:05
* master: (22 commits)
  Changed the AssignentSpacer of the FileIniDataParser instance to the empty string. This removes spaces around the '=' in the kde mimeapps and .desktop files and should enable KDE to parse the files without issues. Should fix #1498
  Fix typo bool->boolean
  Documet find_matches_files option
  Rebuild Ships Directory on Load
  Add extra property to have find match on files for backward compatability
  Actually default use_filename_version to false
  Remove TODO
  Automatically populate ci resource
  Fix spelling
  Update Changelog
  Support asset matching
  Make filename as version opt-in
  Allow find to match on files as well as directories
  Remove old Jenkins API
  More robust Jenkins support
  CONTRIBUTING: Grammar fix
  back to pattern
  Pattern -> enum
  Bring the schema up to date
  Changelog joy for Steam location fixes (#1444, #1481)
  ...
@pjf
Copy link
Member Author

pjf commented Dec 13, 2015

I've made by updates based upon @dbent's changes. I've made 1.0.4 GRAS with 1.0.3 mods, so users won't see anything yet, but this now gives us a framework to let users run the client with different comparison routines, along with a way to handle hotfixes nicely in the future. :)

About to merge as all my changes except 757d9cb as either renames or documentation only changes. (If anyone wants to verify 757d9cb looks sensible, that would be grand!)

@pjf pjf merged commit 2ef85a3 into master Dec 13, 2015
pjf added a commit that referenced this pull request Dec 13, 2015
* 1173_IGameComparator:
  CHANGELOG: Update for fuzzy versioning.
  Tests corrections: GRAS is between 1.0.4 and 1.0.3 now.
  Spec.md: Updated example of GRAS versioning (1.0.3 in 1.0.4 -> OK)
  GrasGameComparator: 1.0.4 is a fancy 1.0.3. 1.0.5 is its own version.
  Changed GameComparator class names based upon code review.
  Updates from @dbent's code review.
  Autofac: Newline normalisation.
  CkanModule: Fill IGameComparator on chained deserialisation.
  Relocate Autofac so our build system is happy. :)
  Establish ServiceLocator, give CkanModule a real ctor, use IGameComparator
  Added IGameComparator! Fuzzy versioning yeah!
  Expanded KSPVersion.Targets to accept strings.
  Add `ksp_version_strict` to Spec/Schema
  Added Autofac 3.5.2
@pjf pjf removed the pull request label Dec 13, 2015
@Olympic1 Olympic1 deleted the 1173_IGameComparator branch December 13, 2015 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants