Repository navigation
Spotless is breaking configuration avoidance for other tasks #444
Description
Activity
@nedtwigg The following will still force eager evaluation (just at a different time).
afterEvaluate { tasks.check.dependsOn(tasks.spotlessCheck) }What I think you want is this:
tasks.named("check") { dependsOn(tasks.named("spotlessCheck")) }A little pedantic, but to be exactly correct according to the docs:
tasks.named("check").configure { dependsOn(tasks.named("spotlessCheck")) }And I can confirm this works. It saved us eagerly configuring about 33,000 tasks in our large project. Quite a hit.
Reacted by Jonathan Leitschuh- added 2 commits that reference this issue
on Sep 27, 2019 Thanks for note @esword!
It saved us eagerly configuring about 33,000 tasks in our large project.
I'll merge the fix above regardless, but I am curious how much clock time this saves you. As in
gradlew someTask was 10s and now it's 5s. Or maybewe spend 10s in configuration with broken Spotless, and the fix brings that down to 5s.Reacted by Jonathan Leitschuhyou are close. project init/config went from 12s to 7s. This is when running gradlew help. That's what I use as the baseline. We have over 450 subprojects and a pretty complex project init. 7s is still way to high (I'm working on that as well), but the jump to 12 was nuts.
Reacted by Jonathan Leitschuh@esword That's quite an impressive performance improvement.
Reacted by Jonathan Bluett-DuncanFixed in
3.25.0.- added a commit that references this issue
on Nov 7, 2019 @nedtwigg I just got back from some other work to focus on the project that inspired this, and I noticed that task configuration avoidance was working properly. Thanks for merging this, I can now rest assured knowing I only have 1 task configured when I run the help command.
For what it's worth, my unmerged code works correctly, because
configureEach() is lazy. :PReacted by Ned Twigg
By default, spotless adds the
spotlessChecktask as a dependency of thechecktask. Thechecktask might not exist yet when the spotless task is applied. To allow Spotless to be applied before or after theJavaBasePlugin, we get thechecktask like this:spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java
Lines 114 to 116 in def65a1
The problem is that this appears to trigger task configuration for every task in the project.
This is different than Spotless using configuration avoidance itself, this is about Spotless breaking configuration avoidance for other plugins. There has been a long discussion about moving Spotless itself to use configuration avoidance. The end result of that discussion is this unmerged code, which I believe (perhaps mistakenly) will trigger the exact same issue:
https://github.com/carbotaniuman/spotless/blob/93ae599bcbc71848bbd8d48de2ab9532d3fb420b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTaskSetupConfigAvoidance.java#L92-L96
Configuration avoidance is a performance enhancement. If you adopt the APIs, but don't test that the build has actually gotten faster / configuration has actually been avoided, then you may or may not be getting any advantage from the work.
If you want to fix this issue, it is easy to experiment right in your buildscript. For example:
This will produce the exact same behavior as
enforceCheck = true, but it might fix the configuration avoidance problem. To get a fix merged, it will need to meet the following criteria:(maybe) If no check task exists, Spotless should create oneSpotless does not do this right nowIt probably should though - if you have a folder (e.g.manual) which just has markdown and the only plugin is Spotless, thengradlew checkought to check the formatting of that folder.Strikethrough is edit by Ned Twigg 9/29: I was wrong about this, Spotless already applies BasePlugin, which creates check. I was confused because the String constant for
checkisJavaBasePlugin.CHECK_TASK_NAME.