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

Internal Robotics Next #7210

Merged
merged 5 commits into from
Jun 2, 2019
Merged

Internal Robotics Next #7210

merged 5 commits into from
Jun 2, 2019

Conversation

meirumeiru
Copy link
Contributor

first try... the release is not yet online... can you still test it if it's ok? we plan to upload the release in about 4 hours

and, other question... some parts are under the "Creative Commons Attribution-NonCommercial-NoDerivatives 4.0 International License" and some under "MIT License" ... should/can we name all 3 of them? or put this in a description?

@DasSkelett
Copy link
Member

DasSkelett commented May 27, 2019

some parts are under the "Creative Commons Attribution-NonCommercial-NoDerivatives 4.0 International License" and some under "MIT License" ... should/can we name all 3 of them? or put this in a description?

Yep, you can have an array of licenses, no problem. This should work:

    "license"        : ["GPL-3.0", "CC-BY-NC-ND-4.0", "MIT"],

There's a comma missing in line 21, and the forum url should be

https://forum.kerbalspaceprogram.com/index.php?/topic/184787-*

I think.

@meirumeiru
Copy link
Contributor Author

correct... thanks for the fix :-)

@meirumeiru
Copy link
Contributor Author

it is online now and I hope your check runs now

@DasSkelett
Copy link
Member

Actually I just remember our discussion in the forum thread. (For anyone reading along: this comment and some above and below)

We should change the conflict part to

"conflicts"      : [
        { "name": "KerbalJointReinforcement", "max_version": "v3.3.3" },
        { "name": "KerbalJointReinforcementContinued" }
],

This way the original KJR is a conflict, and KJR continued, indepentent of future version numbers.

Also, pinging @HebaruSan, wouldn't a

"provides"      : "InfernalRobotics",

make sense here? And maybe a replaced_by relationship added to InfernalRobotics, since this is currently the only maintained fork, if I'm not misinformed?

@meirumeiru
Copy link
Contributor Author

yeah... ok... in case the continued KJR will get compatible... then you're right
and... for the provides of "InfernalRobotics"... you can do this, but I've to mention, that IR and IR Next don't conflict and can be installed side-by-side... I think nobody supports the old IR... but, IR Next has completely different module names in order to provide this possibility... therefor I'm not completely sure if it should provide "InfernalRobotics"... but, it's hard to say

@DasSkelett
Copy link
Member

DasSkelett commented May 28, 2019

My primary "fear" is that KJR Continued might go above v3.99 one day, that means v4.X.
That wouldn't be caught by the current conflicts.

Well, if they are really technically compatible, I think that we shouldn't take away the possibility to install them side-by-side. Who knows whether a crazy adventurer comes by one day, and goes all in.
Moreover I don't know a mod that would benefit from an "automatic relationship update" with a provides.

@meirumeiru
Copy link
Contributor Author

meirumeiru commented May 28, 2019

the problem is, that KJR next is providing KJR... I understand why it has been added, but... if it wasn't there, it wouldn't be a problem now
sure, it means that a lot of people out there would have to update their configs or mods or patches... I don't know what is the best solution for this problem

@DasSkelett
Copy link
Member

No, that's no problem, because KJR Next starts at version v4.0.0. It is not included in the conflicts relationship.
So my solution works fine, if I'm not missing something, and it is not even hacky.

To go sure, I can test it tomorrow.

@meirumeiru
Copy link
Contributor Author

it is no problem at the moment... but when KJR continued goes >= 4, then it is... that's what I mean
if KJRn != KJR, then this wouldn't be a problem
but... we may have other problems then
anyway... all your solutions are fine for me

@DasSkelett
Copy link
Member

but when KJR continued goes >= 4

As it is right now, it would be a problem. But if we list it separately, without a version restriction, it doesn't matter if it goes above v4.

@DasSkelett
Copy link
Member

Okay somehow it does still trigger a conflict with KJR Next:

3916 [1] INFO CKAN.RelationshipResolver (null) - KerbalJointReinforcementNext v4.0.10 would cause conflicts, excluding it from consideration
3921 [1] INFO CKAN.RelationshipResolver (null) - KerbalJointReinforcementNext v4.0.10 would cause conflicts, excluding it from consideration

or if KJR next is already installed:

The following inconsistencies were found:
* InfernalRoboticsNext v3.0.0 conflicts with KerbalJointReinforcementNext v4.0.10

But I think this is a CKAN client side error. @HebaruSan can you confirm?

@meirumeiru
Copy link
Contributor Author

I have an other solution to this problem
can we not remove the conflicts part completely and replace it with a "conflicts with KJR_nonIRcompatible" and you add a "provides KJR_nonIRcompatible" to the original KJR and KJR continued?? ... or name it "KerbalJointReinforcementOriginal" ? like this it would also work with older clients (at least in the first time...)

@HebaruSan
Copy link
Member

@DasSkelett, if I'm understanding your question, relationship version constraints are only applied for exact identifier matches, not indirect matches via the provides property:

https://github.com/KSP-CKAN/CKAN/blob/b996c59304dd575286e235cf978667e3150998c1/Core/Types/CkanModule.cs#L116-L130

Does that answer your question?

@DasSkelett
Copy link
Member

DasSkelett commented May 30, 2019

@DasSkelett, if I'm understanding your question, relationship version constraints are only applied for exact identifier matches, not indirect matches via the provides property:

KSP-CKAN/CKAN:Core/Types/CkanModule.cs@b996c59#L116-L130

Does that answer your question?

Yes, that must be the problem.
Could the solution be

"conflicts"      : [
        { "name": "KerbalJointReinforcement", "max_version": "v3.3.3" },
        { "name": "KerbalJointReinforcementContinued" },
		{ "name": "KerbalJointReinforcementNext", "max_version": "v0.0.0" }
],

?
Or do you know of anther way to solve this relationship network @HebaruSan?

@meirumeiru
Copy link
Contributor Author

yes... add the "provides KerbalJointReinforcementOriginal to the other 2 and let it conflict with Infernal Robotics

@dbent
Copy link
Member

dbent commented May 30, 2019

Yep, you can have an array of licenses, no problem. This should work:

Unfortunately, according to the spec, an array of licenses means that the mod is dual- or multi-licensed, i.e. the user can choose which license they want to use. In the case where different parts of the same mod have different (singular) licenses, then the recommendation is that the most restrictive license should be chosen. KSP-CKAN/CKAN#22 is a long open issue for providing a mechanism to specify licenses on a per-file basis.

In this case, CC-BY-NC-ND-4.0 would be the most restrictive license of the three (the "no commercial" and "no derivatives" clauses pretty much guarantee that).

@meirumeiru
Copy link
Contributor Author

ok, then we should remove the additional licenses and only let the gpl 3 there

@HebaruSan
Copy link
Member

@meirumeiru, it's supposed to be the most restrictive license. If the module said GPL-3.0, users would think they have the right to distribute derivative works, which they do not if the file in question is under CC-BY-NC-ND-4.0.

I'll update it to CC-BY-NC-ND-4.0. Please let us know if this is not acceptable.

@HebaruSan
Copy link
Member

@DasSkelett, regarding the relationships, could you please summarize what we're trying to achieve here, maybe in a tabular format? The specification of this scenario seems to be scattered over many comments here and on the forum. It would help with brainstorming if we had a quick reference of what's supposed to be compatible with what.

@meirumeiru
Copy link
Contributor Author

@HebaruSan about the licensing -> the module is GPL-3, but some art inside is not... I talked to the artist about that and he says he's ok with having it listed as GPL-3 in CKAN and only having the license mentioned in a text file inside the zip
so, I don't think we have a problem here

and for what we try to achieve

-> conflict IR with KJR (original) and KJR Continued, but not with KJR Next (this is even recommended)
since KJR Next is providing KJR, this is a problem

my solution would be -> add "provides KerbalJointReinforcementClassic" to the other two and then a conflict with this to IR Next... is way easier than reprogramming CKAN

@meirumeiru
Copy link
Contributor Author

oh and... I've an other problem now... I have to compile it differently for <= 1.7 and >= 1.7.1 ... do we have to add this twice to CKAN or is there a way to make CKAN distinguish which zip it needs to download for which KSP version?

@HebaruSan
Copy link
Member

@dbent, how strong is the licensing recommendation, and does it have an exception for what a contributor is "ok with"? I don't know what the conditions would be for deviating from it, since a recommendation isn't a strict rule.

For the relationships... the RO folks also report that KJRNext providing KJR creates problems for them, see #7203. Maybe we remove that provides? Then the traditionalist KJRContinued would be installed for folks just needing a KJR continuation.

Also I would like to point out KJR NEXT should NOT be able to provide for KJR. RO/RSS does not support next at all. It has known issues with the RO/RSS suite of mods, in fact both RO and KJR Continued conflicts with next yet ckan seems to be allowing people to install it with these mods.

For separate ZIPs per game version, we support that via separate module versions. E.g., KSPInterstellarExtended does this:

Mod version KSP version
1.21.7.0 1.3.1
1.21.7.1 1.4.5
1.21.7.2 1.5.1
1.21.7.3 1.6.1
1.21.7.4 1.7.0

@meirumeiru
Copy link
Contributor Author

meirumeiru commented Jun 1, 2019

alltough the problems of RO with KJR Next are solved, I think I'm ok with not having this provides inside, since it's a lot different... not just a little update or something... almost completely new (that's why I named ferram only second :-) but, that's just a detail) ... and it would also solve the problem, that everybody thinks I'm responsible for the KJR continued development
if you remove it, you can then also conflict KJR Next with KJR
so this solution works for me
and for IR ... well, whatever works, I will have a pre and post 1.7.1 version tell me how to upload and version it and that's fine...

@HebaruSan
Copy link
Member

OK, the provides is removed and the conflict in this module is now versionless and will catch both old KJR and KJRContinued.

@HebaruSan
Copy link
Member

... can't you just add it once as IRNext1 and then as IRNext2 both providing IRNext... but one with max version 1.7 the other with min version 1.7.1 and then they can have the same version? but different asset match?

The problem with that:

  1. I install KSP 1.7.0
  2. I install IRNext1 with CKAN
  3. I upgrade my KSP to 1.7.1
  4. I should be prompted to upgrade to the version for 1.7.1, but that won't happen because that's IRNext2 and I have IRNext1 installed

@meirumeiru
Copy link
Contributor Author

The problem with that:

I install KSP 1.7.0
I install IRNext1 with CKAN
I upgrade my KSP to 1.7.1
I should be prompted to upgrade to the version for 1.7.1, but that won't happen because that's IRNext2 and I have IRNext1 installed

yeah... that's what I thought too... 30 seconds after sending it... that's why I deleted it again, but you were too quick

@dbent
Copy link
Member

dbent commented Jun 2, 2019

@dbent, how strong is the licensing recommendation, and does it have an exception for what a contributor is "ok with"? I don't know what the conditions would be for deviating from it, since a recommendation isn't a strict rule.

It should be fine, I figure the license metadata is mostly advisory since it does not include the actual terms. One thing to keep in mind, however, is that as far as I know the license metadata is also used programmatically to decide which mods we can upload to archive.org. So if a mod is covered by multiple licenses, one of which does not permit redistribution, but the license in its metadata does, then we'd be redistributing when we shouldn't be.

@HebaruSan
Copy link
Member

OK, GPL-3.0 it is! 👍

@meirumeiru, I think that means this will be ready to merge, unless you want to discuss the details of versioning for 1.7.0/1.7.1 some more first.

@meirumeiru
Copy link
Contributor Author

@HebaruSan, great
I only need a little explanation how to publish/build it... do I need 1 version file with 2 entries? or 2 version files? and will you have 1 or 2 entries for IR? ...

@HebaruSan
Copy link
Member

Treat them as separate releases, like the table I shared earlier.

@meirumeiru
Copy link
Contributor Author

Treat them as separate releases, like the table I shared earlier.

ah, ok... so you're using the version file from inside the zip and not the one you find on github to read out the data... when I put 2 new releases online, you will find both, list both and act according to the version included in this zip (1.7 / 1.7.1)... ok... understood... I'm ready to go

@HebaruSan
Copy link
Member

Yes, with the caveat that the CKAN bot currently only checks the latest release per mod, and it runs once every 4 hours. So waiting 4 hours between posting the two releases is advised. (SpaceDock hosted mods are an exception to this because SpaceDock can notify CKAN when new releases are uploaded.)

Sounds good, merging. Thanks for the submission!

@HebaruSan HebaruSan merged commit 4510810 into KSP-CKAN:master Jun 2, 2019
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.

None yet

4 participants