-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
KAFKA-14680: Upgrade gradle version from 7.6 to 8.0.1 #13205
Conversation
Related to #13199 Thanx for heads up @ijuma On my local box build ends up in green, not sure why it fails on Jenkins; will investigate (and wait for plugin changes to be merged).
|
@dejan2609 We'll wait for the final release, but it's fine to start working through the issues. Looks like the build failed. |
@ijuma yes, somehow build on Jenkins fails to start, I will rebase and then test on my local box (with Windows/Linux). |
Gradle 8 has been released. Have you been able to figure out why the build is failing? |
@ijuma some findigs: build fails due to issues with gradle wrapper bootstrapping. I will post more details today. |
Update: gradle wrapper bootstrapping is ok now, but spotless Scala checks are failing... |
It seems that Spotless Gradle plugin needs to be alligned with Gradle 8.0 (I filed a ticket here: diffplug/spotless#1572) Thing is that they dropped support for direct Java 8 builds: https://github.com/diffplug/spotless/blob/main/plugin-gradle/CHANGES.md#6140---2023-01-26 (their suggestion for other teams is to use Java cross compilation: https://docs.gradle.org/8.0/userguide/building_java_projects.html#sec:java_cross_compilation). Kafka obviously still needs to build artifacts against Java 8, so maybe it would be a good idea to use Spotless team suggestion. All-in-all, herewith a plan for a Gradle 7 -->> 8 upgrade:
@ijuma If it is ok with you I can start working towards this solution. |
An alternative would be to drop spotless until we drop support for Java 8 (Apache Kafka 4.0). What actually uses spotless today? |
At the moment spotless is used via Jenkins CI server ➡️ Jenkinsfile 'spotlessScalaCheck' task execution: https://github.com/apache/kafka/blob/3.4.0/Jenkinsfile#L23 My suggestion is to:
Note: this PR already removes task dependency. |
Yeah, that's a security feature. I suggest filing a JIRA about disabling spotlessScalaCheck with the rationale and have a separate PR for that change. |
Update: there are some new issues with Gradle 8.0.1; I will try to investigate that also. |
4e47230
to
5ffbb0a
Compare
@ijuma Good news, it seems that I managed to provide a workaround for Gradle 8.0.1 (and across all JDK/Scala versions that Apache Kafka uses and the moment). Big shoutout for @ljacomet from Gradle team, he went over and above during investigation 🎉 |
Thanks @dejan2609. I still prefer the path I described here: Do you know if that works? |
Hi @ijuma , just to pitch in that setting
PS: Encountered this error while trying to package |
@inglor Thanks. The following comment explains that the way |
@inglor thanx for stopping by, any insight or help is highly appreciated ! @ijuma I scraped fast solution that, suprisingly enough, works (at least compilation is succesful, that is). I tested locally across entire matrix: JDK 8/11/17 x Scala 2.12/13 (with ./gradlew -PscalaVersion=2.1X clean jar) and all builds are fine. Still need to spend some time with this to doublecheck and maybe even to compare compilation console output (trunk vs. this PR). |
@ijuma Thanks for the pointer - I have missed that. Gradle version 8.0.1:
|
@dejan2609 Good catch. I fixed it in the same branch. And tested with Scala 2.12 and 2.13. See ijuma@31aec82 |
Gradle 8 related links: * https://github.com/gradle/gradle/releases/tag/v8.0.0 * https://github.com/gradle/gradle/releases/tag/v8.0.1 * https://docs.gradle.org/8.0.1/userguide/upgrading_version_7.html#changes_8.0 notes: * Javac and Scalac options are reshuffled (workaround for Gradle 8.0.1 bug: gradle/gradle#23962 (comment)) * spotless plugin task reference is removed (newer plugin versions require Java 11, so we can't use them until Kafka 4.0); plugin configuration is kept * jacoco version is bumped: 0.8.7 -->> 0.8.8 https://docs.gradle.org/8.0.1/userguide/jacoco_plugin.html#sec:configuring_the_jacoco_plugin
Blocks #13205. Rationale: - build works fine in trunk with Gradle 7.6 and spotless gradle plugin 6.13.0 for all currently used JDK versions (that is: JDK 8 / JDK 11 / JDK 17) - however, recent spotless gradle plugin versions (6.14.+) support only JDK 11+ versions: https://github.com/diffplug/spotless/blob/main/plugin-gradle/CHANGES.md#6140---2023-01-26 - given a fact that Kafka still needs to support JDK 8 builds (until Kafka version 4.0) it is reasonable to simply remove spotless checks out of Jenkinsfile (and re-introduce them when the time comes). For even more details see GitHub discussion here: #13205 (comment) Reviewers: Ismael Juma <[email protected]>
@dejan2609 I merged #13263 and updated the PR description. Please take a look. |
@dejan2609 One issue is that IntelliJ still doesn't handle this correctly. Before this PR, it would add the following to
Now it adds:
Both cause the build to break with the latest Scala 2.13 version since that deprecated |
I pushed a commit that reverts part of my previous commit. We need to set
This is still a problem since this is deprecated in newer Scala versions, but this PR doesn't make it worse. @dejan2609 would you be willing to submit a ticket for the Scala plugin for IntelliJ so it doesn't set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks. I'll wait for the CI to complete before merging. The last CI build looked good, so it should be fine.
|
JDK 8 build passed, the other two had unrelated failures:
|
Should we disable |
@chia7712 Correct, For the last few days we (@ijuma and me):
However,
|
@dejan2609 thanks for nice explanation! That is why I suggested to disable spotlessScala from build.gradle :) |
@chia7712 I just pushed PR that (hopefully) solves this regression, feel free to check it. |
@ijuma ticket is created here: https://youtrack.jetbrains.com/issue/SCL-21039 |
@ijuma please review Scala plugin for Intellij ticket (link above); I need some help to provide reproducer. |
The |
Details:
-release
/-source
/-target
to workaround idiosyncrasies in Gradle 8.0.1 and newer Scala 2.13 versions.test
task no longer triggers thespotless
task since a newer version is required for Gradle 8 support, but the newer version requires Java 11.Note: relates to MINOR: Add spotlessScalaCheck dependency to streams-scala test task #5479
Gradle upgrade highlights:
Full release notes: https://docs.gradle.org/8.0/release-notes.html