vlsi / vlsi-release-plugins

A set of plugins to simplify Gradle release tasks
Apache License 2.0
41 stars 13 forks source link

license-gather: add a task for verifying license compatibility #49

Closed vlsi closed 2 years ago

vlsi commented 2 years ago

Fixes #46

vlsi commented 2 years ago

Alternative option:

enum class CompatibilityType {
    ALLOW, UNKNOWN, REJECT;
}

data class LicenseCompatibility(
    val type: CompatibilityType,
    val reason: String
)
vlsi commented 2 years ago

@DreierF , WDYT?

See https://github.com/vlsi/vlsi-release-plugins/pull/49/files#diff-fe8458ed996db9a41bf2a2d964d994e3fe4debc06be8d33ce83c60bb706de5faR97-R118

DreierF commented 2 years ago

@vlsi Do you have any timeline for when you plan to release this?

vlsi commented 2 years ago

Oh, thanks for the ping. I somehow forgot about all this. Just wondering: do you think it is all good? (I don't remember if I intended to make more changes)

DreierF commented 2 years ago

@vlsi The functionality LGTM as it is. I also have a working version running against a bigger project.

DreierF commented 2 years ago

@vlsi Is there anything missing from your point of view?

vlsi commented 2 years ago

@DreierF , I've released 1.78, so please let me know if it works for you.

DreierF commented 2 years ago

First of all thanks for your work on this!

All in all it works as expected and I'm glad that I can start using it :) However, I found some minor issues regarding Groovy compatibility:

    allow(AsfLicenseCategory.A) {
        because("The ASF category A is allowed")
    }

does not seem to work. It results in

Could not find method allow() for arguments [class com.github.vlsi.gradle.release.AsfLicenseCategory$A, precompiled_ProjectLicense$_run_closure2$_closure11@443721af] on task ':verifyLicense' of type com.github.vlsi.gradle.license.VerifyLicenseCompatibilityTask.

Another minor issue is the usage of the and and or operators. Not sure how this was intended to work. What works is:

import static com.github.vlsi.gradle.license.api.LicenseExpressionExtensions.and
import static com.github.vlsi.gradle.license.api.LicenseExpressionExtensions.or

...
    overrideLicense("...") {
        expectedLicense = and(and(Apache_2_0, LGPL_2_1), MPL_1_1)
        effectiveLicense = or(or(Apache_2_0, LGPL_2_1_only), MPL_1_1)
    }

When the methods would be actual instance methods instead of extension methods this could be written as

    overrideLicense("...") {
        expectedLicense = Apache_2_0 & LGPL_2_1 & MPL_1_1
        effectiveLicense = Apache_2_0 | LGPL_2_1_only | MPL_1_1
    }

but I'm not sure if I might be missing some implementation details that would make this very hard to achieve. https://groovy-lang.org/operators.html#Operator-Overloading

vlsi commented 2 years ago

does not seem to work

Does it happen with 1.78? I think the test uses allow(....Category.A), it uses groovy DSL, and the test seems to pass

When the methods would be actual instance methods

The methods should be instance in 1.78. Do they work for you? 

DreierF commented 2 years ago

🙈 I used 1.78, but completely forgot that I still had an intermediate version in mavenLocal that was picked up instead of the final 1.78 from Maven Central. That resolved all mentioned issues. Sorry for the false alarm.

vlsi commented 2 years ago

No worries 😉 . I added the mentioned changes just a couple of days ago.

My initial intention was to keep License interface minimal, however, now I see the conversion to LicenseExpression is almost always needed, so I just gave up with the said extensions and moved them to the instance methods.