build with jdk 19 and Gradle 7.6 #4466

jimexist · 2022-10-05T13:40:54Z

Motivation:

we can enable jdk 19 builds and test out project loom features

Modifications:

include jdk 19 in the GitHub action build matrix

Result:

Closes Build and run with JDK 19 and possible enable loom (preview required) as well #4465
loom features can be enabled during tests

jimexist · 2022-10-05T13:46:19Z

ideally if this passes then we can get rid of jdk 16 in the build

codecov · 2022-10-05T14:57:36Z

Codecov Report

Base: 74.08% // Head: 74.04% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (9635988) compared to base (c34c6b5).
Patch has no changes to coverable lines.

❗ Current head 9635988 differs from pull request most recent head 6a7fd27. Consider uploading reports for the commit 6a7fd27 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #4466      +/-   ##
============================================
- Coverage     74.08%   74.04%   -0.04%     
+ Complexity    18185    18165      -20     
============================================
  Files          1537     1536       -1     
  Lines         67469    67379      -90     
  Branches       8537     8520      -17     
============================================
- Hits          49987    49894      -93     
+ Misses        13419    13416       -3     
- Partials       4063     4069       +6

Impacted Files	Coverage Δ
...ria/internal/client/dns/DelegatingDnsResolver.java	`81.25% <0.00%> (-18.75%)`	⬇️
...armeria/internal/common/stream/SubscriberUtil.java	`72.22% <0.00%> (-11.12%)`	⬇️
...rmeria/internal/client/dns/CachingDnsResolver.java	`78.37% <0.00%> (-8.11%)`	⬇️
...corp/armeria/common/stream/AggregationSupport.java	`76.00% <0.00%> (-6.00%)`	⬇️
...a/internal/client/dns/SearchDomainDnsResolver.java	`87.14% <0.00%> (-5.72%)`	⬇️
.../main/java/com/linecorp/armeria/server/Server.java	`76.71% <0.00%> (-5.14%)`	⬇️
...com/linecorp/armeria/client/BlockingWebClient.java	`29.62% <0.00%> (-3.71%)`	⬇️
...ia/common/stream/ConcatPublisherStreamMessage.java	`78.29% <0.00%> (-2.33%)`	⬇️
...rmeria/common/stream/ConcatArrayStreamMessage.java	`83.14% <0.00%> (-2.25%)`	⬇️
...rp/armeria/common/stream/DefaultStreamMessage.java	`88.42% <0.00%> (-2.11%)`	⬇️
... and 46 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

ikhoon · 2022-11-29T08:06:58Z

Gradle 7.6 has been released. https://docs.gradle.org/7.6/release-notes.html
We may continue to upgrade to Java 19.

- Proguard 7.2.2 -> 7.3.0 - Gradle 7.5.1 -> 7.6

- Set the default buildJdkVersion to the current Java version - Set "java.security.manager" to "allow" - Fix Javadoc errors

ikhoon · 2022-12-16T06:11:35Z

I forget to set 1.21.0 milestone for this PR. PTAL. 🙇‍♂️

minwoox

Thanks a lot for taking care of this @ikhoon! 😄
Thanks, @jimexist for starting this issue. 😄

minwoox · 2022-12-16T08:04:12Z

.github/workflows/actions_build.yml

-            build-${{ matrix.java }}-${{ runner.os }}-${{ secrets.CACHE_VERSION }}-
-            build-${{ matrix.java }}-${{ runner.os }}-
+      - name: Setup Gradle
+        uses: gradle/gradle-build-action@v2


So it works now? 😄

I checked the GitHub Actions log and it does not seem to work.
Both the original one and the new one also didn't work on GitHub-managed hosts.

Original

gradle-build-actions

I found that the cache occasionally works with the old configuration.
Anyway, it does not affect our self-hosted machines that cache by default in the local storage. Let me keep gradle-build-action configuration and check the caching result for multiple builds.

It seems to work now. There might be some complicate conditions to use cache.

minwoox · 2022-12-16T08:06:23Z

build.gradle

@@ -70,6 +70,13 @@ allprojects {
        jvmArgs '-XX:+HeapDumpOnOutOfMemoryError'
        jvmArgs "-XX:HeapDumpPath=${rootProject.buildDir}"

+        // The default value of the java.security.manager system property is changed to disallow in Java 18.


Let's also add why we also need this in 17.

minwoox · 2022-12-16T08:07:36Z

gradle/scripts/lib/java-shade.gradle

-                project.dependencies.add(shadedTestRuntime.name, dep)
+                // Do not use `project.dependencies.add(name, dep)` that discards the classifier of
+                // a dependency. See https://github.com/gradle/gradle/issues/23096
+                project.configurations.getByName(shadedTestRuntime.name).dependencies.add(dep)


Please also send a PR to line/gradle-scripts. 😉

jrhee17

Left some minor nits but looks good! Thanks @ikhoon ! 🙇 👍 🚀

jrhee17 · 2022-12-19T00:38:37Z

build.gradle

+        // - https://openjdk.org/jeps/411
+        // - https://bugs.openjdk.org/browse/JDK-8270380


core/src/main/java/com/linecorp/armeria/client/RequestPreparationSetters.java

jrhee17 · 2022-12-19T01:01:51Z

gradle/scripts/lib/java.gradle

@@ -1,11 +1,13 @@
-def buildJdkVersion = 17
+def buildJdkVersion = Integer.parseInt(JavaVersion.current().getMajorVersion())


Note: I believe this is a slight change in behavior since the jvm version which gradle is run with will be used by default instead of 17.

I agree with this change though, since I dislike having the most recent jdk version fragmented across multiple files 🙇 Just wanted to note this change 👍

gradle/scripts/lib/kotlin.gradle

- Kotlin does not support Java 19 as the target bytecode. As the default version follows the current Java version, a specific target is necessary for release. - Add a workaround that handles a regression in dependency in Gradle 7.6 See gradle/gradle#23096 for details. The PR reviewed in line/armeria#4466

minwoox · 2022-12-21T05:32:57Z

line/gradle-scripts#139 is merged. 😉

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

ikhoon · 2022-12-21T06:32:49Z

It is likely that the flaky Scala build is fixed.
We need to check it more but it looks promising. 😆

jrhee17 · 2022-12-21T06:35:30Z

Yay 😎 Procrastinating paid off 🥳

ikhoon · 2022-12-21T06:58:09Z

Any comments on this @minwoox?

minwoox

Thanks!!!! 🎉 🎉 🎉

ikhoon · 2022-12-21T08:21:54Z

Thanks again @jimexist for the initial work of this PR. 🙇‍♂️🙇‍♂️

build with jdk 19

9635988

jimexist requested review from ikhoon, jrhee17, minwoox and trustin as code owners October 5, 2022 13:40

ikhoon added the dependencies label Nov 29, 2022

ikhoon added 8 commits December 6, 2022 20:55

Merge branch 'master' into pr-4466@Jimexist+build-with-jdk-19

cab4096

Update dependencies

e0dbd18

- Proguard 7.2.2 -> 7.3.0 - Gradle 7.5.1 -> 7.6

Update build JDK versions in GitHub actions

fba07b0

Not use env in job.id

5f7eec9

Fix build errors

c92cf95

- Set the default buildJdkVersion to the current Java version - Set "java.security.manager" to "allow" - Fix Javadoc errors

check java version

42216a0

Gradle regression

ab51d5f

Use a different workaround

f6a1584

ikhoon added this to the 1.21.0 milestone Dec 16, 2022

Use Java targetCompatibility to set Kotiln target

7329bb9

minwoox approved these changes Dec 16, 2022

View reviewed changes

jrhee17 approved these changes Dec 19, 2022

View reviewed changes

minwoox self-requested a review December 19, 2022 02:23

Address comments by @jrhee17

148e982

ikhoon mentioned this pull request Dec 19, 2022

Make Gradle 7.6 compatable line/gradle-scripts#139

Merged

minwoox added 2 commits December 21, 2022 14:40

Merge branch 'master' into pr-4466.20221221143418

532148a

minwoox approved these changes Dec 21, 2022

View reviewed changes

ikhoon changed the title ~~build with jdk 19~~ build with jdk 19 and Gradle 7.6 Dec 21, 2022

ikhoon merged commit 2ff218b into line:master Dec 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build with jdk 19 and Gradle 7.6 #4466

build with jdk 19 and Gradle 7.6 #4466

jimexist commented Oct 5, 2022 •

edited

jimexist commented Oct 5, 2022

codecov bot commented Oct 5, 2022 •

edited

ikhoon commented Nov 29, 2022

ikhoon commented Dec 16, 2022

minwoox left a comment

minwoox Dec 16, 2022

ikhoon Dec 19, 2022

ikhoon Dec 19, 2022

ikhoon Dec 27, 2022

minwoox Dec 16, 2022

minwoox Dec 16, 2022

jrhee17 left a comment

jrhee17 Dec 19, 2022

jrhee17 Dec 19, 2022

minwoox commented Dec 21, 2022

ikhoon commented Dec 21, 2022

jrhee17 commented Dec 21, 2022

ikhoon commented Dec 21, 2022

minwoox left a comment

ikhoon commented Dec 21, 2022

		// - https://openjdk.org/jeps/411
		// - https://bugs.openjdk.org/browse/JDK-8270380

		@@ -1,11 +1,13 @@
		def buildJdkVersion = 17
		def buildJdkVersion = Integer.parseInt(JavaVersion.current().getMajorVersion())

build with jdk 19 and Gradle 7.6 #4466

build with jdk 19 and Gradle 7.6 #4466

Conversation

jimexist commented Oct 5, 2022 • edited

jimexist commented Oct 5, 2022

codecov bot commented Oct 5, 2022 • edited

Codecov Report

ikhoon commented Nov 29, 2022

ikhoon commented Dec 16, 2022

minwoox 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

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

minwoox commented Dec 21, 2022

ikhoon commented Dec 21, 2022

jrhee17 commented Dec 21, 2022

ikhoon commented Dec 21, 2022

minwoox left a comment

Choose a reason for hiding this comment

ikhoon commented Dec 21, 2022

jimexist commented Oct 5, 2022 •

edited

codecov bot commented Oct 5, 2022 •

edited