-
Notifications
You must be signed in to change notification settings - Fork 460
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
Spotless uses afterEvaluate #506
Comments
I'm using Gradle 6.0.1 myself, but I haven't seen this warning so far. At some point, you must say something like afterEvaluate {
if (hasPlugin(Java)) {
apply from: 'java-library-conventions.gradle.kts'
}
} The |
I tried to create a simple single project with spotless with no child modules but I did not see the deprecation warning. My project is multi module project below is an example of one of the modules is configured.
Rather than using the root project to configure the child projects I created my own plugin in the buildSrc similar to what junit does The root project is tiny it only looks like this.
I hope this helps. |
I think that confirms that my comment above is a correct diagnosis. This line in the junit buildSrc would cause this. This might be why junit has kept spotless outside of this plugin: You can definitely configure spotless inside your plugin if you want. You just can't do it inside an
Also, we just launched blowdryer, which has been really helpful to us internally for structuring our build scripts. If you have multiple git repositories, I think it's a lot better than the |
I don't have AfterEvaluate in my plugin below is the complete
|
But how do you apply this plugin? Search |
Found anything interesting? |
I'm closing due to inactivity, but not due to lack of curiosity! If you ever find anything interesting about what was happening here, I hope you'll come back and tell us what you find. |
From the stacktrace we can see that In other words, on multi project builds and when applied to a subproject, the Spotless plugin tries to go up the project hierarchy to the root project, adding an afterEvaluate block to an already evaluated project. This doesn't work and the registered |
The same stacktrace as the first comment? If I take that stacktrace, and remove every line that matches
It is true that if you apply Spotless to a subproject, it will automatically apply itself to the root project at the line that you noted - this is purely for dependency resolution reasons (long story). The problem model that you are proposing is this, correct?
If there is an example project which can reliably recreate the race condition above, then I can fix it pretty quickly. I might be wrong, but I don't think it is possible to make the timeline above happen. And that is because even though Gradle can do parallel execution, my understanding is that evaluation/configuration is still single-threaded. So I believe, perhaps mistakenly, that the root project never finishes evaluation until all subprojects have finished evaluation (which is different from execution). It's easy to prove me wrong - a multi-project build with just a single subproject that applies Spotless, with a big fat sleep can easily make the timeline above happen. Such an integration test would be an important part of ensuring that a fix is actually working. If someone builds the test (any repo that I can clone to reproduce is fine), then I can implement the fix quickly. I'm not willing to implement or even merge a "fix" without such a testcase, because I suspect (perhaps incorrectly) that you will get the same error after the "fix", because the problem actually lies elsewhere, and building the test will help you see that it is elsewhere, or else it will help me to build/verify a fix :) |
Project to reproduce the issue https://github.com/asaikali/spotless-issue |
See, I was right 😝 Thanks for sticking with this. @JLLeitschuh, this behavior seems like a gradle bug to me, but it's easy to fix in Spotless so I will definitely do so. I interpret the design intent of this deprecation warning as "you cannot call afterEvaluate from within an afterEvaluate block", and although that means that you cannot compose two plugins that each use afterEvaluate, trampolining is tricky and I can understand why gradle might make this choice. However, I don't think there is anything tricky about adding an "afterEvaluate" block to the root project from a subproject. There are quite a few plugins that use the rootProject as a central caching/coordinating place. It's easy for us to fix, but there might be other cases which are harder. The biggest argument that this is a gradle bug is that this works:
But this does not:
It's much easier for me to just fix this in Spotless than to shepherd this through Gradle's bug tracker, but figured I'd point it out to you just in case :) |
Fixed in |
Thanks for fixing this issue, your plugin is awesome, and you do all the hard work of keeping it going. Thank you. |
Thanks! I did this particular two-liner, but the community as a whole does a lot more than I do, especially the committer team (@fvgh, @jbduncan, @lutovich, @JLLeitschuh) |
Thanks for fixing this bug quickly and doing a new release ready for us to use! FWIW, reproducing the issue outside of the spotless plugin is a two liner: // settings.gradle
include("sub") // sub/build.gradle
rootProject.afterEvaluate {} Running
Gradle evaluates projects walking down the projects hierarchy, starting with the root project. When the |
if you are using multiple build files. If you're doing a multiproject build with just one file, then you're good: // settings.gradle
include("sub")
// build.gradle (root)
project(':sub') {
rootProject.afterEvaluate {}
} This makes it easy to write an integration test that you think is testing multiproject builds, but actually it isn't. We had/have a multiproject integration test, which is part of why I was so skeptical that this was our bug rather than user error. |
Yes. This is expected and I agree it can be confusing at first. The |
One of the (many) things I admire about Gradle is how it does such a good job "feeling" like a declarative description, rather than a script. It's almost like the only point of the scripting is just to allow you to describe repetitive data in a more concise way than writing it out by hand - beginners can ignore the scripting completely and still be productive. As a user, once you have used Naively, it also seems like it would be easy to fix. I can imagine the implementation advantages of making a project immutable once it has been "evaluated", but "afterEvaluate", being a purely additive operation and not a mutation per-se, seems like it could be easy to tack-on. If the design goal is "calling afterEvaluate from within afterEvaluate is a fuzzy concept, and we should prevent it", that seems noble, but it's different than the limit Gradle is currently enforcing. Anyway, not a big deal, probably not worth the effort for such a small case, just my final 2cents. Thanks for Gradle, always super impressed by the product and team :) |
Spotless plugin is generating a deprecation warning with Gradle 6.0.1
I am configuring spotless from a within the buildSrc folder for all projects that apply the java plugin
java-library-conventions.gradle.kts
containsThe
buildSrc/build.gradle.kts
containsfull stack trace for the warning from gradle 6.0.1 and spotless 3.27.0
The text was updated successfully, but these errors were encountered: