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

Preconfigure JDK before getting java executable #562

Closed
wants to merge 1 commit into from
Closed

Preconfigure JDK before getting java executable #562

wants to merge 1 commit into from

Conversation

mzarnowski
Copy link
Contributor

Currently, importing any sbt project would fail when there is no JDK defined within IntelliJ. This can happen for example if someone installs it for the first time and does not configure the JDK manually.

Why does this happen?
Previously, there was a DefaultJdkConfigurator application component which tried to preconfigure a JDK during IDE startup. In this commit, the component was removed and the ProjectJdkTable::preconfigure method was introduced. This method is actually being invoked automatically in ProjectSdksModel during the project import, but it happens much later than the SbtExternalSystemManager needs the JDK.

How does the PR work?
It preconfigures the JDK just before the JDK is needed. The operation is idempotent, so there is no worry that it will affect subsequent import operations.

Why is there no tests to cover this case?
Sadly, the preconfigure method is ignored in unit test mode. We are currently monitoring this behavior within our own scala test suite which is interacting with an actual IDE and not its unit test mode. In fact, that is how we found this issue.

Before 2020.1, the default JDK was being preconfigured automatically  on startup if there was no JDK defined. Now it is being done much later, so importing projects would fail every time until the user specifies the JDK manually
@jastice jastice self-requested a review July 29, 2020 13:21
@jastice jastice self-assigned this Jul 29, 2020
@jastice
Copy link
Member

jastice commented Jul 29, 2020

Thank you for this, I've been meaning to look into it!

@jastice
Copy link
Member

jastice commented Jul 31, 2020

Manually merged via 484bf55

Thank you!

@jastice jastice closed this Jul 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants