update Gradle to 8.7 #203
update Gradle to 8.7 #203
Conversation
| @@ -1,5 +1,7 @@ | |||
| distributionBase=GRADLE_USER_HOME | |||
| distributionPath=wrapper/dists | |||
| distributionUrl=https\://services.gradle.org/distributions/gradle-7.4.2-bin.zip | |||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.7-bin.zip | |||
There was a problem hiding this comment.
Could you please clarify why this change was necessary?
Does advancing the Gradle version constrain which Gradle versions users of the plugin can use?
There was a problem hiding this comment.
Some of the KT-66949 refactoring would benefit from newer Gradle features, or fixed Gradle bugs. It's also nicer to write build scripts which have newer features, like property assignment.
Updating shouldn't affect the Gradle compatibility (despite its reputation for constantly changing, Gradle actually has a pretty solid deprecation cycle - assuming the build scripts are kept up-to-date!)
However, you're right that we should test it. We can specify the Gradle version being tested, and then we can be more confident about the changes. I'll make a new commit specify a fixed version for testing...
There was a problem hiding this comment.
I've had a quick check and I think that, while Benchmark Plugin says it supports Gradle 7.0, some of the tests fail when tested using Gradle 7.0...
There was a problem hiding this comment.
I've pushed a commit to bump the minimum supported & tested version to Gradle 7.6.3 7.6.4.
Context: BenchmarksPlugin says it supports, at minimum, Gradle 7.0. However, when the tests run using 7.0, some fail! Conclusion: BenchmarksPlugin hasn't supported 7.0 for a long time.
The README says the minimum supported Gradle version is 7.4.
So, rather than fixing the code to support a really, really, old Gradle version, let's just bump the supported & tested version to be the latest 7.x Gradle version (which is also the approach KGP takes).
There was a problem hiding this comment.
I've made a new PR that standardises and tests the supported Gradle version, without making changes to the actual Gradle version used to build & test the project.
There was a problem hiding this comment.
Let's go ahead and merge that PR first, and rebase this PR onto those changes. If the newer Gradle version (8.7) does not require raising the minimum supported version (7.4), I would suggest we keep the minimum supported version unchanged. WDYT?
There was a problem hiding this comment.
tested version to be the latest 7.x Gradle version (which is also the approach KGP takes).
Not exactly. In KGP, we support Gradle 6.8.3+ and aim to keep as much as possible if it's reasonably possible. I would suggest not bumping the minimal version even higher. The best strategy is to keep the minimal versions in sync between all the ecosystem plugins to minimize the effort user should put into dealing with all our plugins
There was a problem hiding this comment.
tested version to be the latest 7.x Gradle version (which is also the approach KGP takes).
Not exactly. In KGP, we support Gradle 6.8.3+ and aim to keep as much as possible if it's reasonably possible. I would suggest not bumping the minimal version even higher. The best strategy is to keep the minimal versions in sync between all the ecosystem plugins to minimize the effort user should put into dealing with all our plugins
Ah, my mistake. What I meant was KGP supports the latest version of each minor release. Am I right in thinking that KGP tests and supports 7.4.2, but not 7.4?
It'd be good to align the supported Gradle version across JB projects. I'm concerned that supporting and testing 6/7/8 would be a large amount of work though!
Gradle stopped bug/security fixes for Gradle 6 over a year ago, so it'd help to drop it. And, so long as Benchmark supports Gradle 7, then it should be compatible with Gradle 8 projects without significant issues.
- replace deprecated Project.buildDir with layout.buildDirectory - replace deprecated createTempFile with Files.createTempFile - change NativeBenchmarkExec from `open` to `abstract` to avoid `does not implement abstract base class member public abstract fun acceptServiceReferences` error
Context: BenchmarksPlugin says it supports, at minimum, Gradle 7.0. However, when the tests run using 7.0, some fail! Conclusion: BenchmarksPlugin hasn't supported 7.0 for a long time. So, rather than fixing the code to support a really, really, old Gradle version, let's just bump the supported & tested version to be the latest 7.x Gradle version (which is also the approach KGP takes).
adam-enko
force-pushed
the
update/gradle-8-7
branch
from
c5ce4e5 to
af4da4e
Compare
| ) { | ||
| /** Defaults to the minimum Gradle version specified in [kotlinx.benchmark.gradle.BenchmarksPlugin] */ | ||
| private val gradleVersion: GradleTestVersion = gradleVersion ?: GradleTestVersion.v7 |
There was a problem hiding this comment.
Could you please clarify why v7("7.6.4") is used instead of the MinSupportedGradleVersion("7.4")?
There was a problem hiding this comment.
it was a typo, thanks for spotting it
integration/src/main/kotlin/kotlinx/benchmark/integration/GradleTestVersion.kt
Outdated
Show resolved
Hide resolved
opentoabstractto avoiddoes not implement abstract base class member public abstract fun acceptServiceReferenceserrorPart of KT-66949
On hold - waiting for #204