cashapp / licensee

Gradle plugin which validates the licenses of your dependency graph match what you expect
https://cashapp.github.io/licensee/docs/1.x/
Apache License 2.0
626 stars 29 forks source link

Remove afterEvaluate #194

Closed hfhbd closed 1 year ago

hfhbd commented 1 year ago

This PR removes the afterEvaluate, mostly used with android and Kotlin multiplatform support by reconfiguring existing tasks using providers.

Advantage:

Disadvantages:

What do you think?

JakeWharton commented 1 year ago

Well I'm not really thrilled with either choice. That should be Gradle's slogan.

Does this mean that if you disable a variant in an Android project you still see its tasks created? Is there a new AGP API that we can use to fix that case? I know there's the "before" and "on" callbacks now.

hfhbd commented 1 year ago

Yeah, the whole thing, AGP + KGP, is a mess! The biggest problem is Gradle: If you want to remove afterEvaluate, you can only use withPlugin, because hasPlugin depends on the execution order of the plugins, so it wouldn't run if android/kgp is applied later. But withPlugin is basically a fire-and-forget, (ideally) you can't track its execution or the absence of one plugin. So the only workaround is having multiple nested withPlugins, resulting in the reconfiguration of existing tasks... (You could hack it with AtomicBooleans though)

Did you mean variants, like debug/release? Afaik this is a BuildType, and I didn't find a method to disable these build types: https://developer.android.com/reference/tools/gradle-api/8.0/com/android/build/api/dsl/BuildType

Or do you mean a product flavor?

We don't use any special AGP callback/API. Instead, we use configureEach. This Gradle API should only run for the actual buildtypes/product flavors. So disabling a product flavor using https://developer.android.com/build/build-variants#filter-variants should work, will add a test later. But yes, this will still create a disabled task licenseeFreeGoogle and the real enabled task licenseeAndroidFreeGoogle.

I don't think, the new callbacks would help, due to the AGP + KGP mess with two plugins using withPlugin. Additionally, AGP 8 requires Gradle 8, but this isn't fully compatible with KGP 1.8.20...

hfhbd commented 1 year ago

Alternatively, we could keep afterEvaluate for Android, but remove it for plain projects, like java, kotlin.jvm, and kotlin.js, I think.

hfhbd commented 1 year ago

What exactly is your problem with the (annoying) additional created tasks? The visibility in AS/Idea? If so, we could "hide" them by changing the group from verification to other. Not ideal too, but at least it is less confusing and not visible in verification anymore.

hfhbd commented 1 year ago

I switched the implementation: Plain/normal plugins (java, kotlin.jvm, kotlin.js, kotlin.mpp) are configured directly (and mpp without Android support). The Android plugin still uses afterEvaluate. This is needed to don't register unused tasks which need to be skipped but are still visible.

I think this is the best implementation (ever).

hfhbd commented 1 year ago

The new onVariants api is nice, but requires no afterEvaluate. With this requirement, the only option to support AGP + MPP I think, is prefixing the android tasks and output folders with android. This option does not register unnecessary tasks, but would be a breaking change.