Skip to content
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.

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

[jb] Use the default Java SDK version when there's no explicit configuration for it yet #12163

Merged
merged 1 commit into from
Aug 23, 2022

Conversation

felladrin
Copy link
Contributor

@felladrin felladrin commented Aug 16, 2022

Description

Currently, when we open a Java Workspace, it selects Java 17 as SDK by default:
image

This PR changes it to use the JDK preferred by the user (the ones defined in JAVA_HOME or PATH environment variables), in case it's not defined in a configuration file yet (e.g. .idea/misc.xml):
image

How does it work

jdkTable.preconfigure() will set the ProjectRootManager.getInstance(project).projectSdk to a specific version if there’s an .idea/misc.xml file defining the JDK for the project.
And if there’s no configuration file, it will add a JDK to the list of available SDKs but won't set up the project SDK. That's where we set them up with the default JDK version found by JavaHomeFinder.

Related Issue(s)

Fixes #12030

How to test

  1. Open the preview environment generated for this branch
  2. Choose the Stable version of IntelliJ IDEA as your preferred editor
  3. Test a repository that contains the JDK specified in .idea/misc.xml:
    a. Start a workspace using this repository: https://github.com/gitpod-io/spring-petclinic
    b. Verify that the workspace starts successfully
    c. Verify that the IDE opens successfully
    d. In JetBrains Client, open the menu "File" >> "Project Structure" and check if it's using the same JDK version as the one defined in .idea/misc.xml.
    image
    e. Stop the workspace and restart it. Now confirm there are no duplicated SDKs listed in File >> Project Structure >> Platform Settings >> SDKs.
  4. Test a repository that doesn't contain the JDK specified in .idea/misc.xml or doesn't contain that file at all:
    a. Start a workspace using this repository: https://github.com/gitpod-io/template-java-spring-boot
    b. Verify that the workspace starts successfully
    c. Verify that the IDE opens successfully
    d. In JetBrains Client, open the menu "File" >> "Project Structure" and check if it's using the same JDK version as the one you see when you run echo $JAVA_HOME on the terminal. (Notice there are also other auto-detected JDKs listed, allowing users to change it if they want.)
    image
    e. Stop the workspace and restart it. Now confirm there are no duplicated SDKs listed in File >> Project Structure >> Platform Settings >> SDKs.
    image

Release Notes

JetBrains IDEs will use the default Java SDK version when there's no explicit configuration for it yet.

Documentation

None.

Werft options:

  • /werft with-preview

@roboquat roboquat added size/M and removed size/S labels Aug 17, 2022
@roboquat roboquat added size/S and removed size/M labels Aug 17, 2022
@roboquat roboquat added size/M and removed size/S labels Aug 17, 2022
@felladrin felladrin force-pushed the felladrin/12030 branch 2 times, most recently from 526ca96 to 6baab9a Compare August 17, 2022 15:49
@felladrin
Copy link
Contributor Author

I've tried a lot of things, but I'm still facing this error (which I confirmed is also happening with the code from main branch, indicating it's due to some logic change on the latest IDEs).

2022-08-17 16:25:23,456 [  21755]   INFO - #c.i.u.i.RootChangesLogger - New rootsChanged event for "spring-petclinic" project with partial rescanning with trace_hash = 818578700:
java.lang.Throwable
	at com.intellij.util.indexing.RootChangesLogger.info(RootChangesLogger.java:27)
	at com.intellij.util.indexing.EntityIndexingServiceImpl.logRootChanges(EntityIndexingServiceImpl.java:108)
	at com.intellij.util.indexing.EntityIndexingServiceImpl.indexChanges(EntityIndexingServiceImpl.java:87)
	at com.intellij.openapi.roots.impl.ProjectRootManagerComponent.synchronizeRoots(ProjectRootManagerComponent.java:299)
	at com.intellij.openapi.roots.impl.ProjectRootManagerComponent.fireRootsChangedEvent(ProjectRootManagerComponent.java:212)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl.fireRootsChanged(ProjectRootManagerImpl.java:477)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl$1.fireRootsChanged(ProjectRootManagerImpl.java:137)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl$1.fireRootsChanged(ProjectRootManagerImpl.java:134)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl$BatchSession.rootsChanged(ProjectRootManagerImpl.java:108)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl.makeRootsChange(ProjectRootManagerImpl.java:449)
	at com.intellij.workspaceModel.ide.impl.legacyBridge.project.ProjectRootManagerBridge.fireRootsChanged(ProjectRootManagerBridge.kt:160)
	at com.intellij.workspaceModel.ide.impl.legacyBridge.project.ProjectRootManagerBridge.access$fireRootsChanged(ProjectRootManagerBridge.kt:34)
	at com.intellij.workspaceModel.ide.impl.legacyBridge.project.ProjectRootManagerBridge$JdkChangeListener.jdkAdded(ProjectRootManagerBridge.kt:254)
	at com.intellij.util.messages.impl.MessageBusImplKt.invokeMethod(MessageBusImpl.kt:649)
	at com.intellij.util.messages.impl.MessageBusImplKt.invokeListener(MessageBusImpl.kt:629)
	at com.intellij.util.messages.impl.MessageBusImplKt.deliverMessage(MessageBusImpl.kt:399)
	at com.intellij.util.messages.impl.MessageBusImplKt.pumpWaiting(MessageBusImpl.kt:378)
	at com.intellij.util.messages.impl.MessageBusImplKt.access$pumpWaiting(MessageBusImpl.kt:1)
	at com.intellij.util.messages.impl.MessagePublisher.invoke(MessageBusImpl.kt:437)
	at jdk.proxy1/jdk.proxy1.$Proxy109.jdkAdded(Unknown Source)
	at com.intellij.openapi.projectRoots.impl.ProjectJdkTableImpl.addJdk(ProjectJdkTableImpl.java:247)
	at com.intellij.openapi.projectRoots.impl.JavaAwareProjectJdkTableImpl.lambda$preconfigure$0(JavaAwareProjectJdkTableImpl.java:45)
	at com.intellij.openapi.application.impl.ApplicationImpl.runWriteAction(ApplicationImpl.java:1023)
	at com.intellij.openapi.projectRoots.impl.JavaAwareProjectJdkTableImpl.preconfigure(JavaAwareProjectJdkTableImpl.java:45)
	at io.gitpod.jetbrains.remote.GitpodProjectManager.configureSdks$lambda-3$lambda-2$lambda-0(GitpodProjectManager.kt:61)

@akosyakov
Copy link
Member

I've tried a lot of things, but I'm still facing this error (which I confirmed is also happening with the code from main branch, indicating it's due to some logic change on the latest IDEs).

Could you ask JB folks please?

@felladrin
Copy link
Contributor Author

felladrin commented Aug 17, 2022

It looks like it's working now!
image
image
image

idea.log:

2022-08-17 18:27:03,209 [  19757]   WARN - #k.c.CoroutineScope - gitpod: 'spring-petclinic' project: SDK detected: zulu-11 (/home/gitpod/.sdkman/candidates/java/11.0.16.fx-zulu)
2022-08-17 18:27:03,234 [  19782]   WARN - #i.g.j.r.GitpodProjectManager - gitpod: 'spring-petclinic' module: SDK was auto preconfigured: zulu-11 (/home/gitpod/.sdkman/candidates/java/11.0.16.fx-zulu)

And that's interesting: the error is still there (meaning it was not affecting this PR changes):

2022-08-17 18:27:03,209 [  19757]   WARN - #k.c.CoroutineScope - gitpod: 'spring-petclinic' project: SDK detected: zulu-11 (/home/gitpod/.sdkman/candidates/java/11.0.16.fx-zulu)
2022-08-17 18:27:03,234 [  19782]   WARN - #i.g.j.r.GitpodProjectManager - gitpod: 'spring-petclinic' module: SDK was auto preconfigured: zulu-11 (/home/gitpod/.sdkman/candidates/java/11.0.16.fx-zulu)
2022-08-17 18:27:03,264 [  19812]   INFO - #c.j.r.f.FollowMeManager - Register new user 0:'gitpod' with ClientId=ClientId(value=Host)
2022-08-17 18:27:03,754 [  20302]   INFO - #c.i.l.t.l.TypeScriptLibraryProviderImpl - Start refreshing typescript libraries
2022-08-17 18:27:07,211 [  23759]   INFO - #c.i.u.i.RootChangesLogger - New rootsChanged event for "spring-petclinic" project with partial rescanning with trace_hash = 324407506:
java.lang.Throwable
	at com.intellij.util.indexing.RootChangesLogger.info(RootChangesLogger.java:27)
	at com.intellij.util.indexing.EntityIndexingServiceImpl.logRootChanges(EntityIndexingServiceImpl.java:108)
	at com.intellij.util.indexing.EntityIndexingServiceImpl.indexChanges(EntityIndexingServiceImpl.java:87)
	at com.intellij.openapi.roots.impl.ProjectRootManagerComponent.synchronizeRoots(ProjectRootManagerComponent.java:299)
	at com.intellij.openapi.roots.impl.ProjectRootManagerComponent.fireRootsChangedEvent(ProjectRootManagerComponent.java:212)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl.fireRootsChanged(ProjectRootManagerImpl.java:477)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl$1.fireRootsChanged(ProjectRootManagerImpl.java:137)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl$1.fireRootsChanged(ProjectRootManagerImpl.java:134)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl$BatchSession.lambda$levelDown$0(ProjectRootManagerImpl.java:84)
	at com.intellij.openapi.application.WriteAction.lambda$run$1(WriteAction.java:86)
	at com.intellij.openapi.application.impl.ApplicationImpl.runWriteActionWithClass(ApplicationImpl.java:1011)
	at com.intellij.openapi.application.impl.ApplicationImpl.runWriteAction(ApplicationImpl.java:1037)
	at com.intellij.openapi.application.WriteAction.run(WriteAction.java:85)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl$BatchSession.levelDown(ProjectRootManagerImpl.java:84)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl.mergeRootsChangesDuring(ProjectRootManagerImpl.java:401)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl.projectJdkChanged(ProjectRootManagerImpl.java:313)
	at com.intellij.openapi.roots.impl.ProjectRootManagerImpl.setProjectSdk(ProjectRootManagerImpl.java:308)
	at io.gitpod.jetbrains.remote.GitpodProjectManager.configureSdk$lambda-5$lambda-4(GitpodProjectManager.kt:87)

@felladrin felladrin force-pushed the felladrin/12030 branch 3 times, most recently from c6ccbe7 to 3d1a90a Compare August 18, 2022 14:43
@roboquat roboquat removed the size/M label Aug 18, 2022
@roboquat roboquat added size/S and removed size/M labels Aug 19, 2022
@akosyakov
Copy link
Member

akosyakov commented Aug 22, 2022

/werft run

👍 started the job as gitpod-build-felladrin-12030.22
(with .werft/ from main)

@felladrin
Copy link
Contributor Author

felladrin commented Aug 22, 2022

I’ve updated the code, and now it avoids creating duplicated JDKs.
I'll test it on the Preview Environment before marking it as Ready for Review again.

For reference, running on the dev server is working like this:

For https://github.com/gitpod-io/spring-petclinic:

gitpod: 'spring-petclinic' project: SDK detected: 17 (/home/gitpod/.sdkman/candidates/java/17.0.4.fx-zulu)

For https://github.com/spring-projects/spring-petclinic:

gitpod: 'spring-petclinic' project: SDK detected: zulu-11 (/home/gitpod/.sdkman/candidates/java/11.0.16.fx-zulu)
gitpod: 'spring-petclinic' module: SDK was auto preconfigured: zulu-11 (/home/gitpod/.sdkman/candidates/java/11.0.16.fx-zulu)
gitpod: 'spring-petclinic' project: SDK was auto preconfigured: zulu-11 (/home/gitpod/.sdkman/candidates/java/11.0.16.fx-zulu)

@gitpod-io gitpod-io deleted a comment from werft-gitpod-dev-com bot Aug 22, 2022
@gitpod-io gitpod-io deleted a comment from werft-gitpod-dev-com bot Aug 22, 2022
@felladrin
Copy link
Contributor Author

felladrin commented Aug 22, 2022

I found out the cause of https://github.com/spring-projects/spring-petclinic/ not connecting and not using the expected JDK:

I've replaced it with this other repo: https://github.com/gitpod-io/template-java-spring-boot

It's now ready for review again.

@felladrin felladrin marked this pull request as ready for review August 22, 2022 11:58
@akosyakov
Copy link
Member

akosyakov commented Aug 22, 2022

This docker image has broken SSH functionality from Gitpod. I can’t connect to https://github.com/spring-projects/spring-petclinic/ even on https://gitpod.io/. And I noticed it was only #12163 (comment) (launch-dev-server.sh) because it dosn’t make use of .gitpod.yml config when we open a repo in dev-serv

@felladrin could you create an issue for it and add into IDE project please? 🙏

@akosyakov
Copy link
Member

akosyakov commented Aug 22, 2022

@andreafalzetti Could you help to verify it? 🙏

@akosyakov
Copy link
Member

Code-wise it looks good.

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Aug 23, 2022

Reviewing this now 👀

@andreafalzetti
Copy link
Contributor

andreafalzetti commented Aug 23, 2022

The prev env is broken, I am going to rebuild it

Screenshot 2022-08-23 at 08 07 06

/werft run with-clean-slate-deployment

👍 started the job as gitpod-build-felladrin-12030.27
(with .werft/ from main)

Copy link
Contributor

@andreafalzetti andreafalzetti left a comment

Choose a reason for hiding this comment

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

Works as advertised!

@roboquat roboquat merged commit 6b6bac3 into main Aug 23, 2022
@roboquat roboquat deleted the felladrin/12030 branch August 23, 2022 10:31
@roboquat roboquat added deployed: IDE IDE change is running in production deployed Change is completely running in production labels Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production deployed Change is completely running in production editor: jetbrains release-note size/M team: IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IntelliJ should pick up default SDK not any
4 participants