Bump org.codehaus.groovy:groovy-all from 2.4.21 to 3.0.25#18469
Bump org.codehaus.groovy:groovy-all from 2.4.21 to 3.0.25#18469yashmayya wants to merge 1 commit into
Conversation
yashmayya
added
dependencies
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #18469 +/- ##
============================================
+ Coverage 63.66% 63.69% +0.02%
Complexity 1685 1685
============================================
Files 3266 3266
Lines 199821 199821
Branches 31022 31022
============================================
+ Hits 127222 127275 +53
+ Misses 62459 62407 -52
+ Partials 10140 10139 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
yashmayya
force-pushed
the
bump-groovy-3.0.25
branch
from
b34e609 to
a676ccf
Compare
xiangfu0
left a comment
xiangfu0
left a comment
There was a problem hiding this comment.
Found one high-signal compatibility issue; see inline comment.
| <dependency> | ||
| <groupId>org.codehaus.groovy</groupId> | ||
| <artifactId>groovy-all</artifactId> | ||
| <artifactId>groovy</artifactId> |
There was a problem hiding this comment.
This narrows Pinot's shipped Groovy runtime from the old groovy-all fat jar to a hand-picked subset, which is a backward-compat break for clusters that have Groovy enabled. Pinot can run with Groovy static analysis unset, and the current 2.4 jar on the runtime classpath includes classes like groovy.util.CliBuilder and groovy.jmx.builder.JmxBuilder; those disappear with this dependency list, so existing transform/query scripts can start failing with ClassNotFoundException after upgrade even though CI stays green. This needs either a compatibility-preserving runtime surface or an explicit breaking-change rollout plus parity coverage for the supported Groovy modules.`
There was a problem hiding this comment.
Good catch — you're right that the per-module list dropped real classes (CliBuilder is in groovy-cli-picocli in 3.x, JmxBuilder in groovy-jmx), and CI being green only proved the Pinot-Java surface, not customer transform scripts.
Updated to <type>pom</type> + <exclusions> for just groovy-testng / groovy-test-junit5 / groovy-test — those three are what dragged in the conflicting TestNG/JUnit jars that crashed Surefire's fork last time. Everything else from the 2.4 fat-jar surface is back on the classpath. Verified via mvn dependency:tree:
\- org.codehaus.groovy:groovy-all:pom:3.0.25:compile
+- groovy, groovy-ant, groovy-astbuilder, groovy-cli-picocli,
+- groovy-console, groovy-datetime, groovy-docgenerator,
+- groovy-groovydoc, groovy-groovysh, groovy-jmx, groovy-json,
+- groovy-jsr223, groovy-macro, groovy-nio, groovy-servlet,
+- groovy-sql, groovy-swing, groovy-templates, groovy-xml
(groovy-testng/groovy-test-junit5/groovy-test absent as expected.)
Locally re-ran the full pinot-broker test suite — no SurefireBooterForkException / NoClassDefFoundError pattern from the previous push.
In Groovy 3.x, groovy-all is published as a POM aggregator rather than a fat JAR, so the dependency needs <type>pom</type>. The aggregator's transitive groovy-testng / groovy-test-junit5 / groovy-test jars however collide with Surefire's TestNG fork classpath on the unit-test runners and crash unrelated test classes with NoClassDefFoundError, so those three sub-modules are excluded. Everything else (groovy core, templates, json, xml, sql, jmx, swing, ant, cli-picocli, datetime, nio, jsr223, console, etc.) is kept so user transform/filter scripts that relied on the old fat-jar runtime surface (e.g. CliBuilder, JmxBuilder, JsonSlurper) continue to work.
yashmayya
force-pushed
the
bump-groovy-3.0.25
branch
from
a676ccf to
3a72f91
Compare
4 participants
Supersedes #18461, which fails because Groovy 3.x publishes
groovy-allas a POM aggregator rather than a fat JAR — Dependabot's straight version bump leaves Maven looking forgroovy-all-3.0.25.jaron Central, which 404s.Changes
pom.xml: bumpgroovy-all2.4.21 → 3.0.25, rename property togroovy.version, add<type>pom</type>so Maven resolves the aggregator.pinot-spi/pom.xml: mirror<type>pom</type>on the consumer dep.<exclusions>forgroovy-testng,groovy-test-junit5, andgroovy-test. These three transitively drag in TestNG/JUnit jars that collide with Surefire's TestNG fork on the unit-test runner and crash unrelated test classes withNoClassDefFoundError: org/apache/pinot/client/admin/PinotAdminException(which an earlier revision of this PR reproduced — see commit history).Everything else the 3.x aggregator pulls in is kept so the runtime Groovy stdlib surface user transform/filter scripts may rely on from the old 2.4.21 fat jar —
groovy.util.CliBuilder(now ingroovy-cli-picocli),groovy.jmx.builder.JmxBuilder(now ingroovy-jmx),groovy.swing.*,groovy.servlet.*,groovy.ant.*,groovy.console.*, JsonSlurper, XmlSlurper,groovy.sql.Sql, date/time helpers, etc. — keeps working.Confirmed via
mvn dependency:treeonpinot-spi:Test plan
Groovy-touching tests across affected modules:
GroovyTemplateUtilsTest,IngestionJobLauncherTestGroovyFunctionEvaluatorTest,SchemaUtilsTest,PartitionerTest,TransformQueriesTestSurefireBooterForkException/NoClassDefFoundError— the CI failure pattern from the previous push is goneSecureASTCustomizerAPI surface (setImportsWhitelist,setReceiversWhiteList,addExpressionCheckers,MethodCallExpression,ImportCustomizer, …) is unchanged between Groovy 2.4 and 3.0; the malicious-script blocklist (System.exit,.execute(), metaClass tricks,PinotAdministrator.main) continues to be rejected.