-
Notifications
You must be signed in to change notification settings - Fork 14.7k
KAFKA-19174 Gradle version upgrade 8 -->> 9 #19513
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
Conversation
A label of 'needs-attention' was automatically added to this PR in order to raise the |
This PR is being marked as stale since it has not had any activity in 90 days. If you If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact). If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed. |
A label of 'needs-attention' was automatically added to this PR in order to raise the |
Update: project compilation works as expected (when powered by Gradle 9) but binary release creation fails. |
A label of 'needs-attention' was automatically added to this PR in order to raise the |
@dejan2609 any update? |
A label of 'needs-attention' was automatically added to this PR in order to raise the |
@chia7712 |
@dejan2609 thanks! |
Pending: I will also explain second problem (specific to Gradle 9, that is). |
A label of 'needs-attention' was automatically added to this PR in order to raise the |
@chia7712 A few unrelated tests are broken (for both Java 17 and 24) but other than that the build looks fine. |
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
java-version: ${{ inputs.java-version }} | ||
- name: Setup Gradle | ||
uses: gradle/actions/setup-gradle@94baf225fe0a508e581a564467443d0e2379123b # v4.3.0 | ||
uses: gradle/actions/setup-gradle@748248ddd2a24f49513d8f472f81c3a07d4d50e1 # v4.4.4 |
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.
Version 5.0.0 has been released. As a follow-up, I will file a minor patch to revisit all actions
#!/bin/sh | ||
|
||
# | ||
# Copyright © 2015-2021 the original authors. |
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.
I have filed https://issues.apache.org/jira/browse/KAFKA-19756 to write down the correct steps for upgrading gradle
Thank you for merging this @chia7712 ! It was quite a challenge to work on this, I will come up with a separate comment to credit all the people who were involved 🙂 I created two more related JIRA tickets:
|
"-Dorg.gradle.appname=$APP_BASE_NAME" \ | ||
-classpath "$CLASSPATH" \ | ||
org.gradle.wrapper.GradleWrapperMain \ | ||
-jar "$APP_HOME/gradle/wrapper/gradle-wrapper.jar" \ |
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.
This change, introduced in gradle 8.14, seems to be a behavioral change. It causes a no main manifest attribute
error if developers use the old wrapper. I will file a PR to revert to the previous behavior and open a Jira ticket for 5.0 to remove the workaround
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.
I file the PR #20658 already.
…ome issues with Gradle 9+ are solved) (#20652) Extends from: #19513 Note: in Gradle 9+ we have to add a switch like this: ``` ./gradlew dependencyUpdates --no-parallel ``` Related link: https://github.com/ben-manes/gradle-versions-plugin/releases/tag/v0.53.0 Reviewers: Chia-Ping Tsai <[email protected]>
@dejan2609 Thanks for working in this. It looks like a few custom tasks ( cc @chia7712 |
Ok, let me check that. |
@omkreddy Can't find issues here:
|
@omkreddy The same goes for a current trunk state (no errors whatsoever):
|
All in all: I see no errors here @omkreddy |
@dejan2609 I think the command/task (unitTest) is not running the tests. Normally we should see test execution output like
On trunk:
|
@omkreddy Are you referring to the command(s) that I used, i.e. See here for more details: https://docs.gradle.org/9.1.0/userguide/command_line_interface.html#sec:command_line_logging
Now, even your output posted above shows no errors:
📢 Edit/update (after few days or so): I was wrong, @omkreddy was right: I was searching for errors and while it's true that there are none of them tests are still skipped |
@omkreddy Thanks for your report. We will take a look and test other commands to ensure everything works correctly |
Can we add a CI task that validates these commands? I'd assume that the goal should be to avoid regressions for these moving forward A separate workflow would be fine, I think |
I will file a PR to fix the task
there are many custom tasks in kafka, but yes it is worth having the CI - I have opened https://issues.apache.org/jira/browse/KAFKA-19780 to discuss that later. |
Another task, |
The simple fix is to add |
I noticed another error that was introduced by #12280 and it unrelated to Gradle9
It can be fixed by changing the |
…ded with a `--no-parallel` switch (#20683) Prologue: #19513 (comment) Also related to: #20652 @chia7712 I double-checked custom gradle commands in `README.md` and AFAIK we should be fine now. Reviewers: Chia-Ping Tsai <[email protected]>
…-->> 9 (#20684) from: #19513 (comment) 1. Fix the task `unitTest` and `integrationTest`; 2. Change the `name` to a method call `name()` for `KeepAliveMode`. Reviewers: Ken Huang <[email protected]>, dejan2609 <[email protected]>, Chia-Ping Tsai <[email protected]>
Related JIRA ticket: https://issues.apache.org/jira/browse/KAFKA-19174
List of changes:
9.0.09.1.09.0.28.3.9changes
Related links:
Notes:
Reviewers: Chia-Ping Tsai [email protected]