Skip to content

MINOR: Upgrade to Gradle 8.14.1 #19780

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 27, 2025
Merged

MINOR: Upgrade to Gradle 8.14.1 #19780

merged 1 commit into from
May 27, 2025

Conversation

fvaleri
Copy link
Contributor

@fvaleri fvaleri commented May 21, 2025 •
edited by github-actions bot
Loading

This patch upgrades to Gradle 8.14.1. Note that we are switching to
using the recommended "-bin" package.

@github-actions github-actions bot added triage PRs from the community build Gradle build or GitHub Actions labels May 21, 2025
@fvaleri
Copy link
Contributor Author

fvaleri commented May 21, 2025 •
edited
Loading

cc @showuon

I was also wondering:

  • Do we need to keep gradlewAll given the deprecation note?
  • Why are we using gradle-8.14-all.zip instead of gradle-8.14-bin.zip?

@github-actions github-actions bot added the small Small PRs label May 21, 2025
@fvaleri fvaleri force-pushed the gradle-ver branch 2 times, most recently from 324a790 to 5cfd2df Compare May 21, 2025 16:54
showuon
showuon reviewed May 23, 2025
View reviewed changes
Copy link
Member

@showuon showuon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

Do we need to keep gradlewAll given the deprecation note?

Sorry, I don't know about the gradlewAll deprecation note. Could you tell me more?

Why are we using gradle-8.14-all.zip instead of gradle-8.14-bin.zip?

I agree with using bin zip. Checking the gradle doc (expend the Installing manually section), it said:

We recommend downloading the bin file; it is a smaller file that is quick to download (and the latest documentation is available online).

Also, could we verify your change in this PR by upgrading the gradle to a newer minor version 8.14.1? Thank you.

@github-actions github-actions bot removed the triage PRs from the community label May 23, 2025
@fvaleri
Copy link
Contributor Author

fvaleri commented May 23, 2025 •
edited
Loading

Sorry, I don't know about the gradlewAll deprecation note. Could you tell me more?

There is a deprecation note inside the file in the root directory, but I see this was recently changed, so we have to keep it for backwards compatibility. See #17313.

I agree with using bin zip.

At some point we switched to "all", but I was unable to find any reason. cc @ijuma

could we verify your change in this PR by upgrading the gradle to a newer minor version 8.14.1? Thank you.

That was released few hours ago. Let's give it a try.

@fvaleri fvaleri force-pushed the gradle-ver branch 2 times, most recently from 8df6f76 to 0221695 Compare May 23, 2025 06:36
@fvaleri fvaleri changed the title MINOR: Simplify Gradle upgrade MINOR: Upgrade to Gradle 8.14.1 May 23, 2025
@fvaleri
Copy link
Contributor Author

fvaleri commented May 23, 2025 •
edited
Loading

@showuon the "bootstrapWrapper" task was not working with my previous changes. This is used to add support for downloading the Gradle wrapper JAR if it doesn't exist. For this reason I updated the PR to just upgrade Gradle and switch to "-bin" binary. Let me know if it's fine. Thanks.

@fvaleri fvaleri force-pushed the gradle-ver branch 2 times, most recently from 3be74b0 to ec78525 Compare May 23, 2025 11:22
showuon
showuon approved these changes May 23, 2025
View reviewed changes
Copy link
Member

@showuon showuon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Let's wait for CI result.

@fvaleri
Copy link
Contributor Author

fvaleri commented May 23, 2025

@showuon the build is broken due to https://github.com/apache/kafka/pull/18929/files#r2104172356. An fix was opened here: https://github.com/apache/kafka/pull/19792/files.

@fvaleri fvaleri force-pushed the gradle-ver branch 2 times, most recently from 7ea3545 to 72d7370 Compare May 23, 2025 12:27
@ijuma
Copy link
Member

ijuma commented May 23, 2025

At some point we switched to "all", but I was unable to find any reason. cc @ijuma

We just use the command that regenerates the file - that used all before. Not sure if that changed.

@fvaleri
MINOR: Upgrade to Gradle 8.14.1
eb1127e 
This patch upgrades to Gradle 8.14.1.
Note that we are switching to using the recommended "-bin" package.

Signed-off-by: Federico Valeri <[email protected]>
@fvaleri
Copy link
Contributor Author

fvaleri commented May 23, 2025 •
edited
Loading

@ijuma thanks.

It looks like distributionType is not used anymore in wrapper.gradle, so I removed it and switched to using "-bin".

@fvaleri
Copy link
Contributor Author

fvaleri commented May 27, 2025

There are a couple of failing tests that seems to be flaky.

@showuon
Copy link
Member

showuon commented May 27, 2025

We just use the command that regenerates the file - that used all before. Not sure if that changed.

Let's move to bin as suggested. And we can always move back to all if needed.

@showuon showuon merged commit ad849f1 into apache:trunk May 27, 2025
18 of 20 checks passed
@fvaleri fvaleri deleted the gradle-ver branch May 27, 2025 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@showuon showuon showuon approved these changes

Assignees
No one assigned
Labels
build Gradle build or GitHub Actions ci-approved small Small PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fvaleri @ijuma @showuon @m1a2st