Spotless: Spotless is breaking configuration avoidance for other tasks

[nedtwigg]

By default, spotless adds the spotlessCheck task as a dependency of the check task. The check task might not exist yet when the spotless task is applied. To allow Spotless to be applied before or after the JavaBasePlugin, we get the check task like this:

https://github.com/diffplug/spotless/blob/def65a1541abd8841d4754278ab45d87537a48f0/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java#L114-L116

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:

spotless {
  enforceCheck = false
}
afterEvaluate {
  tasks.check.dependsOn(tasks.spotlessCheck)
}

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:

• The fix has been tested and confirmed to actually fix the "eagerly configure other tasks" problem
• Whether the check task is created before or after spotless is applied, Spotless will integrate with it
• (maybe) If no check task exists, Spotless should create one
  
  
  
  • Spotless does not do this right now
  
  
  • It probably should though - if you have a folder (e.g. manual) which just has markdown and the only plugin is Spotless, then gradlew check ought 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 check is JavaBasePlugin.CHECK_TASK_NAME.

[JLLeitschuh]

@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"))
}

[esword]

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.

[nedtwigg]

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 maybe we spend 10s in configuration with broken Spotless, and the fix brings that down to 5s.

[esword]

you 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.

[JLLeitschuh]

@esword That's quite an impressive performance improvement.

[nedtwigg]

Fixed in 3.25.0.

[carbotaniuman]

@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. :P