build: Bump Gradle Wrapper to v8.4

[JoseLion]

Resolves #1420. This PR intends to add support for JDK21. Some details on the changes:

• Based on feat: Support java 19 #1319, which added support for Java 19.
• The com.diffplug.spotless plugin had to be updated to work with newer Java versions.
• The Gradle wrapper was updated to v8.4-rc-1. I'm not sure this is necessary for the project to build, but that is the minimum Gradle version that supports Java 21.
• I successfully tested the changes using the debug instructions and a project with the Java Toolchain set to 21. However, I had a lot of trouble building the project in my local environment, so I had some slight doubts.

PS: I can further expand on the issues I faced while building the project in my local environment, and open separate tickets to solve them if they make sense 🙂

[JoseLion]

@microsoft-github-policy-service agree

[jdneo]

Hi JoseLion, is this for #1420?

[JoseLion]

Hi @jdneo! Yes, I was wrapping up some last details and writing a description. I just marked the PR ready for review 🙂

[jdneo]

I had a lot of trouble building the project in my local environment

I guess one of the problem you are facing is that the build-server-for-gradle repo is not public but it is required to build this extension. (Our plan is to release a stable version of this extension at the end of this month containing the build server approach, meanwhile make the build-server-for-gradle repo public).

[jdneo]

Oh, btw, I tend to avoid making some big changes when close to the release date.

Would you mind that merging this PR after the release at the end of this month? (We will have National Day holiday in China at beginning of Oct., I can review this PR after the holiday, which is Oct 7th)

[JoseLion]

I guess one of the problem you are facing is that the build-server-for-gradle repo is not public

Makes sense. But that only causes problems if I try to build the jars with ./gradlew buildJars. I actually had trouble with some other stuff:

• The grpc-tools@^1.12.4 dependency seems to be broken on Windows, so I had to downgrade to v1.11.3
• The Debug Language Server: Launch Language Server launch command only seems to work with Java 8. I had to switch my Java versions several times to be able to test the changes.
• The gradle-server:test fails with a lot of java.lang.reflect.InaccessibleObjectException. It seems to be related to the Java version because I was able to solve it running with Java 11 (using the Java Toolchain).
• The extension/package-lock.json file seems to have been created with a different version of Node.js than the one in the engines constraint (^14.15.4), so every time I installed NPM dependencies, the file was changed.

I can open separate issues for each of these if they make sense. I think it'd be also great to update a few things, like the Node.js version, or the Gradle scripts, where possible 🙂

Would you mind that merging this PR after the release at the end of this month?

Sure, that sounds good to me. Thanks a lot! Any chance you can approve the workflow to see if all checks are passing? That way, I could attempt to fix anything failing in the meantime.

[jdneo]

Thank you JoseLion!

I only took over this project recently. But I can confirm that I encountered those issues you mentioned above. It will be so great if you can help file issues for them! I apologize that I haven't done it before when I met them, though I should do that. :)

The grpc-tools@^1.12.4 dependency seems to be broken on Windows, so I had to downgrade to v1.11.3

Yes, your finding is correct.

The Debug Language Server: Launch Language Server launch command only seems to work with Java 8. I had to switch my Java versions several times to be able to test the changes.

Sorry I haven't debugged the Language Server yet. I can ask my teammate about this next week.

The gradle-server:test fails with a lot of java.lang.reflect.InaccessibleObjectException. It seems to be related to the Java version because I was able to solve it running with Java 11 (using the Java Toolchain).

Yes, I observed this as well when I was trying to update the CI pipeline. Only Java 11 can be used to run the tests.

The extension/package-lock.json file seems to have been created with a different version of Node.js than the one in the engines constraint (^14.15.4), so every time I installed NPM dependencies, the file was changed.

We should definitely upgrate the node version, since the latest VS Code is using 18.x now. 14 is too old.

[JoseLion]

No worries at all! I'll be happy to file those issues. I can also try to help with some of them after this month's release since I now have more context on the project 🙂

[jdneo]

Thank you! It's so great to have users like you helping us make the product better!

[jdneo]

This PR is still in my todo list, sorry for the late responses. I'll review it after the holiday.

[jdneo]

Looks like build will fail when using new com.diffplug.spotless plugin with Java 8.

I'm thinking that, maybe a better way to run the CI is to first build the extension and then test it towards different Java versions?

[JoseLion]

@jdneo, I was thinking something like that. We could run the tests in different Java versions using Gradle's Java Toolchain. That way, we won't need to install different Java versions on CI. Gradle will do that for us, and caching should make this process even faster after the first run. Also, we'll be able to run tests for all Java versions locally, so that's a win too!

I can help with a PR for this. Let me know if you think this is a good idea 🙂

[JoseLion]

Checking the GitHub Actions workflow, another option is to move the Build Extension step to the build-and-analyse job. Then we can upload the artifact and download it on the test-extension job, just like it's done with the Java lib 🙂

[slymon99]

On redhat-developer/vscode-java#3314 we found that eclipse buildship was breaking some JDK21/gradle8.4 builds eclipse-buildship/buildship#1271. I don't think this will block this PR but may be useful for other people trying to get JDK21 working

[jdneo]

@JoseLion Sounds like the second approach is easier. I would prefer the second one.

I can help with a PR for this. Let me know if you think this is a good idea 🙂

Like always, any contributions are welcome. Thank you! 👍

[jdneo]

@slymon99 Thank you for the information. According to the compatibility matrix, 8.4 does not fully support JDK 21 now. We will keep an eye on it.

https://docs.gradle.org/current/userguide/compatibility.html

[JoseLion]

@jdneo I opened #1438 to solve the CI issue. If it works as expected, I can update this PR branch once that one is merged into develop 🙂

[JoseLion]

@jdneo with #1438 now merged, I updated this PR so we can try the new CI configuration. Hopefully, everything should go through this time 😁

[jdneo]

Looks like com.diffplug.spotless:spotless-plugin-gradle:6.21.0 has some hard dependencies on Java version?

[jdneo]

Hi @JoseLion

Today I created a new Gradle project with 8.4, and set java.import.gradle.java.home to JDK 21, looks like things are just working without the change.

So, I guess maybe the project created by gradle init is not enough to test JDK 21 support. Would you mind to share how you verify this change on your side? Thanks.

[JoseLion]

Hi @jdneo!

I have been looking into that, and it seems I had it wrong! I was using the debug methods described in the CONTRIBUTING.md file, but I didn't notice that the client was failing to start. It looks like the problem is I'm missing the build-server-for-gradle project locally. The docs show how to add it and build the jars, but it looks like the repo is private, and I cannot clone it. Is it possible to make that repo public? Or maybe there's another way of testing the extension, like using a "snapshot" version built by CI on the branch? What do you think? 🤔

[jdneo]

It looks like the problem is I'm missing the build-server-for-gradle project locally.

Sorry for that. One way of workaround I guess could be manually copied those required jars from the installed extension location.

For example, if you are using VS Code stable, you can found it at $HOME/.vscode/extensions/vscjava.vscode-gradle-3.13.xxxx/server. Copy all the files under that folder into ./extension/server

[JoseLion]

Excellent! Thank you @jdneo, that worked. This is how I'm testing the changes:

Without this PR changes:

1. Create a project with gradle init
2. Change the distributionUrl property in gradle-wrapper.properties file to use Grade v8.0
3. Set java.import.gradle.java.home to JDK 21
4. Save the gradle.build file
5. The Gradle for Java logs should show an error containing "Unsupported class file major version 65."

With this PR changes:

1. Follow steps 1-3 of the previous section (or use the same project).
2. Save the gradle.build file
3. The Gradle for Java logs should show "CONFIGURE SUCCESSFUL."

---

The reason why it works on Gradle v8.4 with and without these changes is because how the Utils.hasCompatibilityError(..) method works: If the current Gradle version is equal to or higher than the version matching the current JDK version, it won't cause any compatibility error. That's how I understood this part of the code, but maybe that's incorrect. Let me know what you think and if the steps above work for you 🙂

[JoseLion]

PS: I rebased the PR branch onto develop to update it and keep a linear history

[jdneo]

I got following exception when using Gradle 8.0 and JDK 21:

[info] Build file changed: c:\Users\xxx\Desktop\gradle_init\app\build.gradle
[info] Java Home: C:\Program Files\Eclipse Adoptium\jdk-21.0.1+12
[info] JVM Args: --add-opens=java.base/java.util=ALL-UNNAMED,--add-opens=java.base/java.lang=ALL-UNNAMED,--add-opens=java.base/java.lang.invoke=ALL-UNNAMED,--add-opens=java.prefs/java.util.prefs=ALL-UNNAMED,--add-opens=java.base/java.nio.charset=ALL-UNNAMED,--add-opens=java.base/java.net=ALL-UNNAMED,--add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED,-XX:MaxMetaspaceSize=256m,-XX:+HeapDumpOnOutOfMemoryError,-Xms256m,-Xmx512m,-Dfile.encoding=UTF-8,-Duser.country=US,-Duser.language=en,-Duser.variant
[info] Gradle User Home: C:\Users\xxx\.gradle
[info] Gradle Version: 8.0
[info] > Task :buildSrc:extractPluginRequests UP-TO-DATE
> Task :buildSrc:generatePluginAdapters UP-TO-DATE
> Task :buildSrc:compileJava UP-TO-DATE
> Task :buildSrc:compileGroovy NO-SOURCE
> Task :buildSrc:compileGroovyPlugins FAILED
[error] FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':buildSrc:compileGroovyPlugins'.
> BUG! exception in phase 'semantic analysis' in source unit 'precompiled_Gradle_initJavaApplicationConventions' Unsupported class file major version 65

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

CONFIGURE FAILED in 212ms
4 actionable tasks: 1 executed, 3 up-to-date
[error] Error getting build for c:\Users\xxx\Desktop\gradle_init: The supplied build action failed with an exception.
[info] Found 0 tasks
[error] [gradle-server] The supplied build action failed with an exception.

I think it makes sense because according to the compatibility matrix, Gradle 8.0 won't support java 21.

When you test locally, please just use Debug Extension. I found that Debug Server & Extension will use JDK 8 to launch the server no matter what is set to the setting java.import.gradle.java.home

[JoseLion]

Oh, I see what you mean! I've been testing it wrong all along 😅

The good news is that I found the problem. It seems that the GetBuildHandler.run() method relies on running ./gradlew model to 1) get the project model, and 2) determine if the project has compatibility issues. But 2) is achieved using a try-catch block, which means that the compatibility handle logic will only execute if an exception is thrown when the action runs. That is a problem because the ./gradlew model action will not fail on Java 21 from Gradle v8.2 onwards.

I think we have two possible solutions:

1. Find another task we could run after ./gradlew model, which should fail if the Java version is incompatible.
2. Fail fast! Don't wait for a Gradle task failure to check for compatibility. Instead, check before attempting to run the ./gradlew model action and stop the execution if incompatibility issues are found.

IMO, option 2 is more straightforward and avoids coupling the code logic to Gradle's behavior (which changes from version to version). I quickly tested option 2, and things are working as expected. I even got the VSCode notification with the incompatibility message (screenshot below). What do you think? 😁

[jdneo]

We need to make sure: it is on purpose or not that the compatibility handle logic will only execute in the try-catch block.

For example, maybe, there are some cases that the compatibility matrix does not strictly match the real situation?

I tend to not touch the execution logic unless we have a strong reason to change it :)

But, I think it does not block this PR to be merged. IMO, we can slightly change the title of the PR and then merge it. For example, Bump gradle wrapper to 8.4. Support JDK 21 sound a little bit off-topic because we actually did not do any special things regarding to JDK21 support, it depends on users' gradle version totally.

[JoseLion]

Yes, there could be other reasons for that. Checking for compatibility issues only in the catch-block is a more flexible approach. The downside is that users will never get the "VSCode incompatibility notification" when using Gradle v8.2 or higher. They will have to run a Gradle task within VSCode to ensure their Gradle version is compatible with the Java version used by VSCode.

But I agree! That part is off-topic regarding Support JDK 21. I think it's ok if we change the PR title to the one you suggested. I'll also update the branch so it can be merged 🙂

[jdneo]

@JoseLion Thank you for the contribution.

[robaho]

Thanks for all the hard work.

It’s a shame a lot of these subsystems don’t adhere to the Java forward compatibility promise.

[jdneo]

@JoseLion I've got some update about the Java 21 stuff. Because now the compatibility matirx has two column: Support for compiling/testing/… & Support for running Gradle. I asked the developer from Gradle about the difference. The answer is:

• "Support for running Gradle": the first Gradle version that can run on a particular java version.
  That means, at the moment, no Gradle version is committed to be able to run on Java 21. The first version to support that will be 8.5

• "Support for compiling/testing/…​": effectively lists which is the first Gradle version that can configure toolchains with a particular Java version. Toolchains were introduced in Gradle 6.7, hence the N/A listed for previous versions.

According to the definition, I think we should replace 8.4 with 8.5 where checking the Gradle version and Java version. WDYT?

[JoseLion]

@jdneo, that's very useful to know! I've been using the Java Toolchain lately, so that may be why I didn't get into any trouble with Java 21 so far. I think it's a good idea to replace 8.4 with 8.5 in that part of the code. I'll be glad to help with a PR for that! I have a couple of questions, though:

1. Gradle 8.5-rc-2 was released today (14 Nov). I don't know when the release candidate will be promoted, but a second release candidate may mean it shouldn't take much longer. Should we go ahead and use 8.5-rc-2, or should we wait for the stable 8.5 to be released?
2. Should we update the project to use 8.5 as we did on this PR? It could be a good idea to ensure things run ok on Java 21. Plus, 8.5 comes with some nice improvements 😉

Let me know what you think.

[jdneo]

Yes, we can wait until the 8.5 is released and then replace 8.4 as well as bump the wrapper version.

[JoseLion]

@jdneo Gradle 8.5 is finally released 🎉 I opened #1455 to follow up with this change 🙂