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

Check game version compatibility when installing specific version #2228

Merged
merged 2 commits into from Dec 21, 2017

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Dec 18, 2017

Problem

ckan install identifier=version currently bypasses game version compatibility checks. This means that an incompatible module that you normally couldn't install at all, can be installed just by specifying any version:

$ mono ckan.exe install AvisorforValentina
Module AvisorforValentina required but it is not listed in the index, or not available for your version of KSP.
If you're lucky, you can do a `ckan update` and try again.
Try `ckan install --no-recommends` to skip installation of recommended modules.
$ mono ckan.exe install AvisorforValentina=1.01
About to install...

 * A visor for Valentina 1.01

Continue? [Y/n] 

This makes the identifier=version format unsafe by default, since compatibility checking is just turned off; this could be a problem if I intend to install an older compatible version but make a typo in the version number.

It also isn't documented anywhere that it works this way, so a user might not even realize that he's installing incompatible versions once he discovers this argument format.

Changes

This pull request checks that the version the user specified is compatible with the game version. It also adds the module version to the error message:

$ mono ckan.exe install AvisorforValentina=1.01
Module AvisorforValentina 1.01 required but it is not listed in the index, or not available for your version of KSP.
If you're lucky, you can do a `ckan update` and try again.
Try `ckan install --no-recommends` to skip installation of recommended modules.
Or `ckan install --allow-incompatible` to ignore module compatibility.

To avoid losing the ability to install incompatible mods if the user so chooses, a new explicit Cmdline option --allow-incompatible is added:

$ mono ckan.exe install --help
CKAN 1.24.0-PRE+5cf8b5017bc5
Copyright © 2014-2017
 
install - Install a KSP mod
Usage: ckan install [options] modules

  --allow-incompatible    (Default: False) Install modules that are not 
                          compatible with the current game version

This works for both the identifier=version format and the simpler identifier format, so you now don't have to know the version number to install the latest incompatible version of a mod:

$ mono ckan.exe install AvisorforValentina=1.01 --allow-incompatible
About to install...

 * A visor for Valentina 1.01

Continue? [Y/n] 
$ mono ckan.exe install AvisorforValentina --allow-incompatible
About to install...

 * A visor for Valentina 1.01

Continue? [Y/n] 

@HebaruSan HebaruSan added Bug Cmdline Issues affecting the command line Pull request and removed Bug labels Dec 18, 2017
@techman83
Copy link
Member

I believe this is used often to force install a non-compatible version. Though there was some work put towards loose compatibility checking options a while back.
Should we allow and warn instead?

@politas
Copy link
Member

politas commented Dec 20, 2017

Yes, I think I'd call this a feature, not a bug. Adding a warning about any incompatibilities would be good.

@HebaruSan
Copy link
Member Author

Thanks for the replies. I will take another look at this.

@HebaruSan
Copy link
Member Author

OK, I added a few more changes to create an explicit option for allowing incompatible versions. This way the command can still be safe by default, and the back door to get around compatibility checking is explicitly documented.

@politas politas merged commit 9460b39 into KSP-CKAN:master Dec 21, 2017
politas added a commit that referenced this pull request Dec 21, 2017
@HebaruSan HebaruSan deleted the fix/inst-c-version-check branch December 21, 2017 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmdline Issues affecting the command line Pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants