Upgrade Gradle to 8.1.1 #4854

ikhoon · 2023-05-02T07:10:42Z

Motivation:

Gradle 8 has been released.

Modifications:

In Gradle 8, dependencies can't be declared in a resolvable configuration (canBeResolved=true). gradle-scripts used resolvable configurations such as compileClasspath, runtimeClasspath to inject platform dependencies.

armeria/gradle/scripts/lib/common-dependencies.gradle

Lines 180 to 183 in 2096983

    
           // Add to resolvable configurations 
        
           if (configuration.canBeResolved && !configuration.canBeConsumed) { 
        
               add(configuration.name, platform(dependencyManagementProject)) 
        
           }

* Where: Script '.../gradle/scripts/lib/common-dependencies.gradle'

* What went wrong: A problem occurred configuring project ':foobar'.
> Dependencies can not be declared against the `compileClasspath` configuration.

platform dependencies should be added to non resolvable and non
configurations. There is no workaround to set dependencies to
resolvable configurations. So I changed common-dependencies.gradle
to set platform dependencies to the non resolvable and non
configurations.

As a workaround shadedTestImplementation is newly added to declare
dependencies while configuring shadedTest task. The original shadedTestRuntime
extends shadedTestImplementation and is only used to resolve its dependencies.

Related discussion: https://discuss.gradle.org/t/problem-with-compileclasspath-after-gradle-8-update/44940

Gradle 8 also disallows implicit dependencies between two tasks. The solution was quite simple. I added an explicit dependency for the implicit dependency with .dependsOn(...) whenever encountering the error.

Result:

You can now use Gradle 8 with gradle-scripts.

Motivation: [Gradle 8](https://docs.gradle.org/8.0/release-notes.html) has been released. Modifications: - In Gradle 8, dependencies can't be declared in a resolvable configuration (canBeResolved=true). `gradle-scripts` used the resolvable configurations such as `compileClasspath`, `runtimeClasspath` to inject platform dependencies. https://github.com/line/armeria/blob/20969832640bcc0fa66edc68c531891ab35cea25/gradle/scripts/lib/common-dependencies.gradle#L180-L183 ``` * Where: Script '.../gradle/scripts/lib/common-dependencies.gradle' * What went wrong: A problem occurred configuring project ':foobar'. > Dependencies can not be declared against the `compileClasspath` configuration. ``` `platform` dependencies should be added to non resolvable and non configurations. There is no workaround to set dependencies to resolvable configurations. So I changed `common-dependencies.gradle` to set platform dependencies to the non resolvable and non configurations. `shadedTestImplementation` is newly added to declare dependencies while configuring `shadedTest` task. The original `shadedTestRuntime` extends `shadedTestImplementation` and is only used to resolve its dependencies. - Gradle 8 also disallows implicit dependencies between two tasks. The solution was quite simple. I added an explicit dependency for the implcit dependency with `.dependsOn(...)` whenever encounting the error. Result: You can now use Gradle 8 with `gradle-scripts`.

ikhoon · 2023-05-02T07:11:09Z

gradle/scripts/lib/common-dependencies.gradle

@@ -15,8 +15,6 @@ buildscript {
    }
 }

-def managedDependencyOverrides = [] as Set


It was an unused code.

ikhoon · 2023-05-02T07:13:35Z

gradle/scripts/lib/kotlin.gradle

-            tasks.runKtlintCheckOverMainSourceSet.dependsOn(project.ext.getGenerateSourcesTask())
+
+            // `tasks.withType(KtLintCheckTask)` does not work here.
+            ['Main', 'Test', 'Jmh'].each { name ->


jmh module injects runKtlintCheckOverJmhSourceSet and compileJmhKotlin that tasks need explicit dependencies for generated sources from proto or thrift files.

trustin · 2023-05-02T12:57:36Z

Thanks for looking into this potentially very tricky issue, @ikhoon! Let me wait for the build to pass first at the moment.

ikhoon · 2023-05-02T13:15:39Z

core/build.gradle

-            group: 'Verification',
-            description: 'Runs the TestNG unit tests.',
-            dependsOn: tasks.shadedTestClasses) {
-        useTestNG()


Gradle prohibits overriding the test engine with Test tasks.
On the other hand, the JVM Test Suite Plugin supports multiple test engines.
So the test tasks have been migrated to the new test plugin.

…rride it on FixedStreamMessageVerification

minwoox

Thanks a lot for the upgrade. 🙇

gradle/scripts/lib/java-shade.gradle

trustin

Thank you so much, @ikhoon! This is a labor of love 💘

trustin · 2023-05-08T10:35:39Z

...utoconfigure/src/test/java/com/linecorp/armeria/spring/ArmeriaSettingsConfigurationTest.java

@@ -38,7 +38,7 @@
 @SpringBootTest(classes = TestConfiguration.class)
 @ActiveProfiles({ "local", "settings" })
 @DirtiesContext
-class ArmeriaSettingsConfigurationTest {
+public class ArmeriaSettingsConfigurationTest {


Question: Why was this necessary?

ArmeriaSettingsConfigurationTest uses JUnit 4 which requires a public class.
I don't get why it wasn't detected before.

jrhee17

Left a minor question but looks good to me! Thanks @ikhoon 🙇 👍 🚀

jrhee17 · 2023-05-08T10:08:37Z

gradle/scripts/lib/java.gradle

-        }
-
-        // Use a different JRE for testing if necessary.
-        if (rootProject.ext.buildJdkVersion != rootProject.ext.testJavaVersion) {


Question) Would it be better to pull changes from line/gradle-scripts#148 before merging this PR?

jrhee17 · 2023-05-09T02:27:15Z

gradle/scripts/lib/common-dependencies.gradle

-                    add(configuration.name, platform(dependencyManagementProject))
-                }
-
-                // Find version overrides in dependency declaration configurations
                if (!configuration.canBeResolved && !configuration.canBeConsumed) {


Question) I'm wondering if we want to target resolvable dependencies instead (since these are the configurations which will actually be applied)

Suggested change

if (!configuration.canBeResolved && !configuration.canBeConsumed) {

if (configuration.canBeResolved && !configuration.canBeConsumed) {

There are three types of configurations.

Bucket - implementation, api ...

Resolvable - compileClasspath, runtimeClasspath, ...

Consumable - apiElement, ...

We directly added the dependencies to resolvable configurations. In particular, platform dependencies are added to each configuration here. Unfortunately, in Gradle 8, bucket styles are only allowed to add dependencies, and exceptions are raised if the dependencies are added to resolvable ones.

So I changed to add the platform dependencies to the buckets. It should be fine because they will be eventually inherited into resolvable configurations. All resolvable configurations defined in the Gradle Java plugin have at least one bucket configuration.
https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_configurations_graph

The only exception was shadedTestRuntime created by gradle-scripts that does not have a bucket parent. I added shadedTestImplementation as a bucket so that we inject the platform dependencies into it. As shadedTestRuntime extends shadedTestImplementation, all dependencies added to shadedTestImplementation will be visible to shadedTestRuntime.

I understood the concept while reviewing the PR, but I'm not sure why I left this review 😅 Sorry about that.
In short, I understood that we can't add dependencies to a resolvable configuration 🙇

Co-authored-by: minux <songmw725@gmail.com>

subrepo: subdir: "gradle/scripts" merged: "c20bc5b41" upstream: origin: "https://github.com/line/gradle-scripts" branch: "main" commit: "c20bc5b41" git-subrepo: version: "0.4.5" origin: "https://github.com/ingydotnet/git-subrepo" commit: "dbb99be"

ikhoon · 2023-05-11T08:26:09Z

All bugs encountered while upgrading Gradle 8 have been fixed. I will send a PR to line/gradle-scripts.

See line/armeria#4854 for the detailed motivation and changes.

ikhoon · 2023-05-15T07:47:00Z

Thanks for reviewing. 🙇‍♂️

ikhoon added the dependencies label May 2, 2023

ikhoon added this to the 1.24.0 milestone May 2, 2023

ikhoon requested review from jrhee17, minwoox and trustin as code owners May 2, 2023 07:10

ikhoon commented May 2, 2023

View reviewed changes

Migarate to JVM test suite plugin to configure both JUnit and TestNG

31e83f0

ikhoon commented May 2, 2023

View reviewed changes

ikhoon added 2 commits May 2, 2023 22:21

Replace enabled with required

8dfc487

Fix a broken test which was not triggered by CI

e185bfc

ikhoon force-pushed the gradle-8 branch from 794924e to e185bfc Compare May 2, 2023 15:04

ikhoon added 2 commits May 3, 2023 10:25

Revert the fix in required_subscribeOnAbortedStreamMustFail() and ove…

228cf4a

…rride it on FixedStreamMessageVerification

Add another dependsOn

f15feb4

minwoox approved these changes May 8, 2023

View reviewed changes

gradle/scripts/lib/java-shade.gradle Outdated Show resolved Hide resolved

trustin approved these changes May 8, 2023

View reviewed changes

jrhee17 approved these changes May 9, 2023

View reviewed changes

ikhoon and others added 5 commits May 9, 2023 22:35

Update gradle/scripts/lib/java-shade.gradle

b1e6ffe

Co-authored-by: minux <songmw725@gmail.com>

Fix bugs in testJavaVersion

7a5f791

Fix bugs in flakyTests

8cc6f69

Enable auto provisioning of Toolchains

16aa066

ikhoon force-pushed the gradle-8 branch from 5c6fe67 to 16aa066 Compare May 11, 2023 04:38

ikhoon added 2 commits May 11, 2023 14:17

Specify testJavaVersion for flakyTests

64183fc

Add javaRepositories

785a5de

ikhoon added a commit to ikhoon/gradle-scripts that referenced this pull request May 11, 2023

Support for Gradle 8

ca96101

See line/armeria#4854 for the detailed motivation and changes.

ikhoon mentioned this pull request May 11, 2023

Support for Gradle 8 line/gradle-scripts#149

Merged

ikhoon added a commit to line/gradle-scripts that referenced this pull request May 12, 2023

Support for Gradle 8 (#149)

dad1db8

See line/armeria#4854 for the detailed motivation and changes.

ikhoon merged commit e15c7b7 into line:main May 15, 2023

ikhoon deleted the gradle-8 branch May 15, 2023 07:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Gradle to 8.1.1 #4854

Upgrade Gradle to 8.1.1 #4854

ikhoon commented May 2, 2023 •

edited

Loading

ikhoon May 2, 2023

ikhoon May 2, 2023

trustin commented May 2, 2023

ikhoon May 2, 2023

minwoox left a comment

trustin left a comment

trustin May 8, 2023

ikhoon May 9, 2023

jrhee17 left a comment

jrhee17 May 8, 2023

ikhoon May 9, 2023

jrhee17 May 9, 2023

ikhoon May 9, 2023

jrhee17 May 10, 2023

ikhoon commented May 11, 2023

ikhoon commented May 15, 2023

	// Add to resolvable configurations
	if (configuration.canBeResolved && !configuration.canBeConsumed) {
	add(configuration.name, platform(dependencyManagementProject))
	}

	if (!configuration.canBeResolved && !configuration.canBeConsumed) {
	if (configuration.canBeResolved && !configuration.canBeConsumed) {

Upgrade Gradle to 8.1.1 #4854

Upgrade Gradle to 8.1.1 #4854

Conversation

ikhoon commented May 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trustin commented May 2, 2023

Choose a reason for hiding this comment

minwoox left a comment

Choose a reason for hiding this comment

trustin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrhee17 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikhoon commented May 11, 2023

ikhoon commented May 15, 2023

ikhoon commented May 2, 2023 •

edited

Loading