-
Notifications
You must be signed in to change notification settings - Fork 465
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
XML formatter config is sometimes not applied when running gradle --parallel #492
Comments
I tried to trace out the execution below, under "Trace". Nothing jumped out at me as the problem, but I added a "My best guess" section, which is the subset of the full Trace which I think is most pertinent. My best guessspotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java Lines 567 to 586 in b7f8c55
spotless/lib-extra/src/main/java/com/diffplug/spotless/extra/EclipseBasedStepBuilder.java Lines 118 to 121 in b7f8c55
TraceGradle configurationThe spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java Lines 567 to 586 in b7f8c55
spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java Lines 289 to 296 in b7f8c55
In the spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java Line 244 in b7f8c55
spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java Lines 593 to 604 in b7f8c55
Gradle executionspotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java Lines 230 to 237 in b7f8c55
Step executionspotless/lib-extra/src/main/java/com/diffplug/spotless/extra/wtp/EclipseWtpFormatterStep.java Lines 51 to 53 in b7f8c55
spotless/lib-extra/src/main/java/com/diffplug/spotless/extra/EclipseBasedStepBuilder.java Lines 118 to 121 in b7f8c55
|
Tried to reproduce the issue, both within the IDE and on our CI env, but failed. The spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java Line 573 in b7f8c55
Then, in the spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java Line 580 in b7f8c55
The state to the formatter is calculated lazily, though the builder is mutable (has the call to spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java Line 579 in b7f8c55
But presumably this is not a problem, a friend who knows a lot about Gradle says that the configuration phase is single-threaded. Still, the error matches the description of one "default-configured" object overwriting a properly configured one. The steps in the FormatExtension are copied to the SpotlessTask a bit further down: spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java Line 602 in b7f8c55
Tell you the truth, I have no idea why our execution failed to pick up the configFile setting in one of our CI runs. And right now, I cannot reproduce it. |
Sounds good. I was surprised at this bug, because (as you say) gradle configuration is single-threaded even for parallel builds, and I've used parallel myself for a long time without problems. Absolutely possible there's a bug somewhere (global search for static fields might bring up an ill-advised cache somewhere), so we'll leave this open in case you or someone else gets any more whiffs of a parallel race condition bug. |
Sorry, thought that it was related to an issue I found. But it is not. |
OK, while preparing quite some cleanup, I finally found a remaining bug in the XML formatter, which had not been addressed by #489 . Together with the new @epkanol Sorry about the trouble caused. |
I had another look at the Eclipse source. Currently the Spotless WTP provides the possibility to configure multiple formatter settings for the same formatter. For example you can configure in Gradle an XML formatter for XML files in directory
I actually think it is cleaner to disallow different configurations for all WTP formatters. I am not sure whether this option is used in real life. Any thoughts? |
Thanks for having a look at this! For our purposes, we'd be happy with having a single formatting configuration for the whole project. I probably should mention that currently we only use XML formatting. |
I'm fine with anything, IMO the easiest path is:
|
Released in |
I'm trying the spotless maven plugin 2.9.0 and it's failing with parallel builds with error messages like this one (due to parallel build, some other message is in the mix, sorry about it):
Seems related to this ticket? But I can open a new one too.
|
@aaime, could you please open a new issue. Please specify the Spotless Eclipse XML formatter version if specified in your configuration (you can use older formatters with newer Spotless versions). I am a little bit surprised by the exception line:
formatter = new DefaultXMLPartitionFormatter(); |
Done: #828 |
At this moment, don't know if this is a gradle or spotless-plugin problem.
Sometimes, when running
gradle --parallel
, theconfigFile()
setting is not applied, or is applied too late (after formatting has started in another thread)Command line used from Jenkins is:
gradle --parallel build check --info
This is the spotless version:
This is the spotless configuration:
The xml prefs are the standard eclipse prefs:
Error message (trimmed):
It should be noted that the file was already formatted (as is seen in the above output, using two spaces) when this error was seen.
Something in either gradle or spotless causes the formatter to start formatting before it has received the correct service config (i.e. the contents of the property file).
I'd be happy to help to figure out what, if you could guide me into where to start looking (e.g. what are the entry points between the spotless plugin/ gradle configuration and the wtp formatter).
The text was updated successfully, but these errors were encountered: