Issue #227: Incompatible with the configuration cache

[jbduncan]

What's changed?

This is a draft PR for issue #227, showing my work as I go along.

What's your motivation?

To fix issue #227.

Anything in particular you'd like reviewers to focus on?

I'm trying to figure out how to avoid Gradle-specific things like Project, Configuration, Task and Task.project to allow rewrite-gradle-plugin to be configuration-cache-compatible. Any insights would be appreciated.

Any additional context

I'll mention in gradle/gradle#13490 that this plugin is configuration-cache-incompatible. Also I'm considering hopping on the Gradle Slack or forums to get some help.

Checklist

• I've added unit tests to cover both positive and negative cases
• Incompatible with the configuration cache #227 (comment) is no longer a blocker.
• I've added the license header to any new files through ./gradlew licenseFormat
• I've used the IntelliJ IDEA auto-formatter on affected files
• I've updated the documentation (if applicable)

[jbduncan]

Blocked until I've received the go ahead to bump the min Gradle version to one that better supports the configuration cache, as per #227 (comment).

[timtebeek]

Thanks for getting this started! As mentioned in #227 (comment) we might want to wait until we hear if requiring Gradle 4.3 is an option, and what the upsides versus downsides of that are. I'm not sure the upsides have been discussed much in #227 up to now.

In general I can say we're in the business of (among other things) migrating legacy applications; as such we would like to support going as far back as possible, until such a time that we no longer expect or wish to support migrating from ancient versions. That does have downsides in particular for us as developers on OpenRewrite, but raising the minimum required version before folks can get started with automated migrations is also not ideal.

Let's wait for feedback in the issue; potentially expand a bit on the upsides of versions bump versions, and see from there how to continue.

[jbduncan]

@timtebeek Okey dokey! It's not the end of world if we can't go forward in this direction, as there's the option of turning on a flag in each and every task to indicate they're incompatible with the configuration cache, which solves the problem but without any speed gains.

[jbduncan]

That being said, this flag may only be available in later Gradle versions, so if we need to stick with a config-cache-unaware minimum version of Gradle for the foreseeable future, then so be it. 😄

[jbduncan]

I have many failing tests that I'm not sure how to fix, so when I next have time, I'll copy changes bit by bit into a new local branch.

But even if I fix everything, I've discovered that rewrite-gradle-plugin needs the Gradle root project and subprojects to parse everything, which may be incompatible with the configuration cache. So the simplest change may be to just use notCompatibleWithConfigurationCache to tell Gradle that rewrite-gradle-plugin is incompatible. Stay tuned.

Note for self: use these resources:

• https://docs.gradle.org/8.5/userguide/configuration_cache.html#config_cache:requirements:use_project_during_execution
• https://docs.gradle.org/8.5/dsl/org.gradle.api.provider.ProviderFactory.html#org.gradle.api.provider.ProviderFactory:gradleProperty(java.lang.String)
• https://docs.gradle.org/8.5/javadoc/org/gradle/api/provider/ValueSource.html (warning: needs Gradle 6.1+)

[knutwannheden]

Can't a Gradle plugin query the Gradle version at runtime and then conditionally use new features? Then we could use the config cache when available.

[jbduncan]

@knutwannheden True. I'm struggling to see how that would help, though, because it looks like rewrite-gradle-plugin needs to scan the entire Gradle project to work; root project, subprojects and all. My fear is that the config cache needs plugins to work on one subproject at a time, not the whole project, but I could be wrong. Worst case scenario, notCompatibleWithConfigurationCache will do the trick.

WDYT? :)

[jbduncan]

@knutwannheden Please let me know if I've misunderstood you!

[knutwannheden]

My Gradle skills are rather limited, unfortunately. Possibly @shanman190 night have a suggestion.

[shanman190]

So @jbduncan is correct in the assessment that for configuration cache support the Gradle plugin would have to undergo a pretty drastic overhaul due to the strict requirements set forth by the configuration cache feature. (You can see more here: https://docs.gradle.org/current/userguide/configuration_cache.html#config_cache:requirements)

To illustrate a few of these that would have impact, the DefaultProjectParser would have to have the Micrometer JVM metrics module removed because it accesses JVM state and would have to be restructured such that it accepts details about the project without actually working with the org.gradle.api.Project type and some of its siblings.

Overall, right now, I think that it's in the realm of being achievable, but it would require a bit of familiarity with the inner workings of that class in specific.

[jbduncan]

Overall, right now, I think that it's in the realm of being achievable, but it would require a bit of familiarity with the inner workings of that class in specific.

Indeed, so it will take me some time to fix it.

I'm hoping notCompatibleWithConfigurationCache will prove to be a good tactical solution in the meantime. I'll report back when I have more info.

[jbduncan]

So, the configuration cache was introduced in Gradle 6.6, but my testing shows that notCompatibleWithConfigurationCache was introduced in Gradle 7.4.

Therefore, I'll update this PR to conditionally use notCompatibleWithConfigurationCache if the current Gradle is 7.4+.

I'll make another PR in the future to see if I can make everything config-cache-compatible (following @shanman190's advice) as far back as Gradle 6.6 and remove the notCompatibleWithConfigurationCache call, hopefully making the plugin faster in the process.

In summary, this PR will make rewrite-gradle-plugin config-cache-compatible for Gradle 7.4+.

[jbduncan]

Team, I'm ready for a review. Hit me. 🤛

[jbduncan]

Sorry for the delay, this PR fell off my radar! I believe I've dealt with @timtebeek's feedback, so I'm ready for another round of reviews.