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

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

Merged
merged 4 commits into from
Feb 13, 2022

Conversation

vlsi
Copy link
Owner

@vlsi vlsi commented Jan 10, 2022

Fixes #46

@vlsi
Copy link
Owner Author

vlsi commented Jan 10, 2022

Alternative option:

enum class CompatibilityType {
    ALLOW, UNKNOWN, REJECT;
}

data class LicenseCompatibility(
    val type: CompatibilityType,
    val reason: String
)

Copy link
Contributor

@DreierF DreierF left a comment

Choose a reason for hiding this comment

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

Thanks a lot for implementing that suggestion @vlsi!

tasks.register("verifyLicenses", VerifyLicenseCompatibilityTask.class) {
metadata.from(generateLicense)
allow(SpdxLicense.EPL_2_0) {
because("JUnit is OK")
Copy link
Contributor

Choose a reason for hiding this comment

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

Love it ❤️

@vlsi vlsi force-pushed the verify_license branch 8 times, most recently from c09f0a9 to c4a7601 Compare January 12, 2022 13:02
@DreierF
Copy link
Contributor

DreierF commented Jan 29, 2022

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

@vlsi
Copy link
Owner Author

vlsi commented Jan 29, 2022

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
Copy link
Contributor

DreierF commented Jan 29, 2022

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

@DreierF
Copy link
Contributor

DreierF commented Feb 7, 2022

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

@vlsi vlsi force-pushed the verify_license branch 6 times, most recently from 2d38a61 to 590320b Compare February 13, 2022 08:48
vlsi added 4 commits February 13, 2022 12:54
…ncremental in case overides are present

Ideally the task should be incremental in all the cases, however, override tracking is not implemented yet.
@vlsi vlsi merged commit a2de23e into master Feb 13, 2022
@vlsi vlsi deleted the verify_license branch February 13, 2022 11:24
@vlsi
Copy link
Owner Author

vlsi commented Feb 13, 2022

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

@DreierF
Copy link
Contributor

DreierF commented Feb 13, 2022

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
Copy link
Owner Author

vlsi commented Feb 13, 2022

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
Copy link
Contributor

DreierF commented Feb 13, 2022

🙈 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
Copy link
Owner Author

vlsi commented Feb 14, 2022

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.

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.

Add license check
2 participants