Use worker api and toolchains for checkstyle#20069
Conversation
This reverts commit bbfb48378fabab9ae05551a3382b781323e46c1b.
WIP formatting and review feedback WIP checkstyle worker api Fix tests for CheckstylePlugin Signed-off-by: Rene Groeschke <rene@elastic.co>
Signed-off-by: Rene Groeschke <rene@elastic.co>
Signed-off-by: Rene Groeschke <rene@elastic.co>
Signed-off-by: Rene Groeschke <rene@elastic.co>
Signed-off-by: Rene Groeschke <rene@elastic.co>
5dddc42 to
d639f7c
Compare
ea7a428 to
349dafa
Compare
big-guy
left a comment
There was a problem hiding this comment.
Other than the XSLT output, this LGTM.
|
|
||
| protected abstract String getActionName(); | ||
|
|
||
| protected abstract Closure<Object> getAntClosure(); |
There was a problem hiding this comment.
Is this like what you tried? This passes for me:
9f230ae
| def stylesheet = stylesheetString.isPresent() | ||
| ? stylesheetString.get() | ||
| : Checkstyle.getClassLoader().getResourceAsStream('checkstyle-noframes-sorted.xsl').text | ||
| ant.xslt(in: xmlOutputLocation, out: htmlOutputLocation) { |
There was a problem hiding this comment.
Something seems to be different with this change... I'm seeing this output all the time with this change:
[ant:xslt] Warning: org.apache.xerces.jaxp.SAXParserImpl$JAXPSAXParser: Property 'http://javax.xml.XMLConstants/property/accessExternalDTD' is not recognized
[ant:xslt] Warning: org.apache.xerces.jaxp.SAXParserImpl$JAXPSAXParser: Property 'http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit' is not recognized.
https://ge.gradle.org/s/dlizegkowms2c/console-log#L615
This doesn't happen on current master.
There was a problem hiding this comment.
Hmm, good catch. I will check
There was a problem hiding this comment.
It seems that the culprit is having xerces:xercesImplon the classpath. It's brought in by the nekohtml dependency. I removed both dependencies. It doesn't seem we need them, since xerces seems to be integrated into the JDK.
Any thoughts on that?
There was a problem hiding this comment.
I see now why we used nekohtml in some places... Since default SaxParser is very strict and html is not that strict. Hmm...
I will try to use Jsoup instead of the nekohtml.
a6fa4f7 to
fa8bdbf
Compare
|
@bot-gradle test this |
|
OK, I've already triggered the following builds for you: |
|
@bot-gradle test and merge |
|
Your PR is queued. See the queue page for details. |
|
OK, I've already triggered a build for you. |
Moving it out of test only per gradle#20069 See also: gradle#20884 (comment) Signed-off-by: Michael J Bailey <Michael.J.Bailey@aexp.com>
Runs CodeNarc in a Worker. Based off of [previous work done for checkstyle](#20069) - [x] Write tests - [x] Figure out how to configure logging for worker - [x] Link toolchain with that set in java plugin - [ ] Update release notes Co-authored-by: Justin Van Dort <jvandort@gradle.com>
It brings 5-10% benefit vs master for
gradle/gradlerunning:gradle clean checkstyleMain checkstyleTest --no-build-cache --no-configuration-cache --infoperformance-tests.zip
Also, there is an even bigger benefit when there are multiple source sets reported by @breskeby in #19361 (comment).
Before:
After:
