Cannot add a configuration with name 'classpath' as a configuration with that name already exists (intermittent plugin-gradle concurrency bug) #372
Comments
What did you upgrade from? I suspect it's a very old version because...
is something you probably have to do. If you just got this error message for the first time, I'm guessing you upgraded from a pretty old one. |
|
3.6.0. But like I wrote, I think that error message is incorrect in this case since I get a InvalidUserDataException not a ResolveException and our artifactory logs shows that it resolved correctly. |
|
Have you tried adding |
|
Well, I have these entries in our artifactory logs.... So it's not a problem with resolving the dependency, but the error message is constructed with the assumption that if the code fails here, then that must be the error. |
|
I'd be happy to discuss at https://gitter.im/diffplug/spotless However, I'd like to first try the error message's suggestion first. This code has a lot of people using it exactly like you, except that many seem to have added |
|
Ahh, ok I kinda see how that could help... Our spotless dependency comes from buildSrc/build.gradle since we want it available for all .gradle scripts. That probably means that a "classpath" configuration linked to the rootProject's buildScript hasn't been created yet. Adding it to the buildscript should force create the "classpath" configuration in the rootProjects build.gradle in the configure phase, making it safe to use during task execution. So I added your suggestion and tested with a loop similar to this (with more debugging added): It failed on iteration 42 after 1234 s. Ran again and it failed on iteration 30 after 852 s. Then I removed the workaround and ran the same script again. So basically the same issues with or without I still unsure if it's ok to call |
|
Interesting! And when you say it failed, you mean that it failed with
It's an odd error, because I would expect that calling getters, even if they aren't threadsafe, would at worst return stale data. And spotless doesn't use use those except to create a It's super valuable that you've found this issue and found a way to reproduce it. However, it's rare and "fails safe" (no silent incorrect output), so it's a fairly benign issue. Happy to merge any PR's which fix the issue, but it's not going to make the top of my todo list. Creating these detached configurations is used throughout the codebase to enable lazy evaluation, so it's likely that there's no easy workaround without a major performance regression. |
nedtwigg
changed the title
nedtwigg
changed the title
|
Copied from gradle forums Disclaimer: I haven’t taken much time to understand the codebase. It seems there’s a race condition initializing the root project’s buildscript classpath. What’s the reason for wanting to create the detached configurations in the root project’s buildscript? Why not create them in the current project? If you did Instead of I’d assume any race conditions could be avoided since each thread would be mutating a separate project instance |
|
Well c188743 changed this to use the rootProject's buildscript block instead since it makes configuration of the buildscript repos easier... I guess reverting that would require a good explanation of how to configure buildscript repositories for all projects. That change simplified configuration. |
See here for an example of using composite builds to build an external plugin then use it Or you could simply copy the spotless gradle sources to your project's |
|
Well I get the setttings.gradle/includeBuild to work...it just doesn't replace the the spotless dependency for me. Any idea why? "./gradlew buildEnvironment" shows the non-included version. |
|
Hack the version so it's different to what's in the plugin repository. See here for how to include the plugin |
Aha! This seems like a great insight. I wonder if this would solve the problem while still allowing the buildscript repositories simplification: synchronized (GradleProvisioner.class) {
project.getRootProject().getBuildscript().getConfigurations().detachedConfiguration(deps)
} |
|
There's absolutely no need to use the root project to create the |
|
Any idea why composite builds does not work for spotless? Have you tried that for local development? |
The wrong solution IMHO |
No and no :) Possibly because spotless uses a pretty old gradle wrapper. We're still compatible with 2.x.
My memory might be wrong, but I think this is the story:
It would be possible to confirm or refute this with a little archaeology, but...
if this works out then it's probably not worth the archaeology, because we'll have a simple and proven fix to merge :) |
|
Thanks for the detailed information... I see why you're doing it in the root project's buildscript. It's worth raising a bug against gradle. It seems that two threads are trying to add the "classpath" configuration to the root project's buildscript. I guess both threads think they're the first to load the buildscript classpath |
|
If anyone wants to raise the bug and shepherd it through Gradle, that's fine with me. If I were a gradle dev, I would probably close it and say "don't do this!", or if I were really diligent I would lock down the API more carefully so that our proposed workaround is impossible. "Don't modify one project from another" is a good restriction that makes parallel project evaluation possible, and if they really enforce the limit then spotless' multiproject support is totally broken, and users are going to have to declare https://gizmodo.com/astronauts-have-done-so-so-much-with-duct-tape-and-ele-1711503831 |
|
Well, if you're mutating the configurations in the root project, one could argue that the tasks should live there too You could even create "dummy" tasks in the subprojects which |
Agreed! That's why I don't think raising a bug with gradle is likely to be useful. But we're doing a special mutation, we're creating a detached configuration, which doesn't change the other named configurations. That's why I think we can get away with abusing Gradle's threading spec, so long as we agree to at least coordinate with ourselves by only abusing the spec one-at-a-time. This will probably break if another plugin out there uses the same hack that we are using, but I think our situation is unique because we implemented lazy-task-configuration before they had APIs for it.
That's true. It's a much bigger change than duct-taping in a At best, such a change will necessarily prevent parallel configuration of spotless tasks, and it will require spotless to always be applied to the root project, which is not required currently. And unless we're careful, we might end up preventing parallel execution of spotless tasks as well. |
|
I'll not be contributing such a fix so I think your duct tape wins. I'd argue Is slightly better than |
|
For what it's worth, I think there's still a bug in Gradle. I get the feeling this race condition exists even if you don't mutate the root project. It seems the race condition is before that, it occurs when creating the root project's buildscript classloader. |
|
Here's the pull-request |
|
I might be a bit late to the party, however, is there a true reason to create a detached configuration and resolve it? Could Spotless create its own configuration if it requires extra tools on the classpath? For instance, we have the following tool-specific configurations: It looks like Spotless could create regular configuration rather than trying to clone one from I've got a CI failure around GradleProvisioner, however, the next build (with no changes at all) succeeded. |
What is unsafe about it? The failure you describe seems important, if you can get it to happen even just 1 time out 10, it would be worth opening an issue, as it would reveal a gradle bug worth fixing. Currently, a step doesn't do any work at all until it runs for the first time. This basically allowed us to do configuration avoidance before Gradle had APIs for that. Now that Gradle has that built in, we could dump ours, but it's baked deeply into the code. Also, our maven plugin benefits from the lazy configuration, and maven does not have the configuration avoidance APIs that gradle does. Summary: it is very difficult to change this aspect of Spotless, and even if it were easy there are real downsides to our non-gradle users. If you have a bug, it would be very helpful to find a way to reproduce it and post it as an issue. |
The above "You probably need to add a repository containing" was produced out of a project that has repositories defined. I don't know if it is Spotless bug or Gradle bug, however, the code that synchronizes on configurations looks fishy, so I ask here first. |
|
It is fishy, and there is a long story (see here). The constraints are:
As described above, our
|
This could probably be handled with https://docs.gradle.org/current/userguide/declaring_repositories.html#declaring_a_repository_filter (and I agree it was not available before Gradle 5.0. However, it looks like for now users have a way to confine artifacts to repositories, so is this "build script repository" really needed now? As a bonus, it would have a proper name in the build logs, and users would be able to use Gradle's dependency approach to specify versions (e.g.
They seem to go in the quite opposite direction: they forbid resolution from user threads, they forbid to resolve configurations across projects. I won't be too much surprised if they would watch carefully the code Spotless uses, think much, and invent a clever way to forbid that style of use :( For instance, you might have seen gradle/gradle#10844 which is pretty much the same issue. And you see how they close the issue. |
I agree this is not likely to be successful.
The snapshot was just an example. As in the issue that you linked, people expect their project and buildscript repos to be separate. Another case might be I believe the most likely paths to nirvana here are:
|
It seems to defeat Gradle Build Scan as well (however they seem to agree to fix that): https://discuss.gradle.org/t/your-build-scan-could-not-be-displayed-what-does-this-mean/33302/10 |
|
Starting with |
Hi, we just had a spurious error on our jenkins after upgrading our spotless gradle plugin to 3.18 due to "org.gradle.api.InvalidUserDataException: Cannot add a configuration with name 'classpath' as a configuration with that name already exists."
This is for a multiproject build with parallel execution and it seems that gradle tries to create the classpath configuration multiple times in DefaultScriptHandler.
Seeing as there are no locks in this code, I'm guessing that defineConfiguration() is not meant to be called in parallel but somehow it is being called that way.
Also the error message printed to the log about missing repos is a bit confusing due to GradleProvisioner catching Exception and printing a error message that's better suited for catching ResolveException. I first troubleshooting thought was to go to our artifactory and check our logs (it sent the files correctly).
Gradle 5.2.1, Spotless gradle plugin 3.18, Ubuntu 16.04 docker container.
The text was updated successfully, but these errors were encountered: