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

Java home not recognised #1464

Closed
Arthurm1 opened this issue Feb 27, 2024 · 10 comments · Fixed by #1469
Closed

Java home not recognised #1464

Arthurm1 opened this issue Feb 27, 2024 · 10 comments · Fixed by #1469

Comments

@Arthurm1
Copy link
Contributor

Describe the bug

Metals doesn't recognise my JAVA_HOME setting.

To Reproduce Steps to reproduce the behavior:

  1. Set your machine's environment settings JAVA_HOME to an already downloaded jdk 21.
  2. Open the latest nightly VSCode Metals extension.

Metals starts up and immediately Coursier downloads a JDK 17 which then overwrites the JAVA_HOME on my machine and adds this jdk 17 to my machines path.
This is pretty invasive and stops any apps that require jdk 21 from working on my machine.

I see there is a new Java Version setting in Metals which defaults to 17. I can change this to 21 but coursier will still download a JDK instead of using my own. It does download a version 21 though.

Metals now seems to ignore the VSCode "Java home" setting - I can't get that to affect any of this.

Not sure if this is a Windows only issue and it can't detect I have a JAVA_HOME setting. The release version of Metals works fine.

If the idea is for Metals to download a JDK version if one is not present then maybe the 2 VSCode settings (java home & java version) could be merged into something like...

  1. Use JAVA_HOME (default)
  2. Use specific JDK dir : enter jdk dir here
  3. Download jdk 11
  4. Download jdk 17
  5. Download jdk 21

Expected behavior

I'd expect Metals to recognise my JAVA_HOME and use that be default.

The changes to my machines JAVA_HOME and path are worrying.

Installation:

  • Operating system: Windows 11
  • VSCode version: 1.86.2
  • VSCode extension version: 1.29.2
  • Metals version: 1.2.2

Search terms

java_home

@tgodzik
Copy link
Contributor

tgodzik commented Feb 27, 2024

We should use the JAVA_HOME and only download if it's not available. Seems that something is wrong there. Maybe, it was due to the default being Java 17, which forced us to download JDK 17 since 21 was not matching what we would expect?

We can allow using any higher than the default.

@tgodzik
Copy link
Contributor

tgodzik commented Feb 27, 2024

@kasiaMarek could you take a look?

@tgodzik
Copy link
Contributor

tgodzik commented Feb 27, 2024

Also looks like it doesn't show progress (at least to me). We also should probably use --setup flag to avoid setting JAVA_HOME for the user.

@kasiaMarek
Copy link
Contributor

Metals now seems to ignore the VSCode "Java home" setting - I can't get that to affect any of this.

Yes, that setting is now meant to point to java home you want to use for your project. Java version refers to the one that is used to start metals with. I guess it should be described a bit better.

I can change this to 21 but coursier will still download a JDK instead of using my own.

That's a bug, it should use your JAVA_HOME if the version matches. (We can change this to allow any higher as well.)
Could you share your metals plugin output?

@kasiaMarek
Copy link
Contributor

We also should probably use --setup flag to avoid setting JAVA_HOME for the user.

Quite the contrary I think. We are using the flag and we shouldn't.

@Arthurm1
Copy link
Contributor Author

My system JAVA_HOME env var was c:\jdk-21 in this test.
My VSCode settings for Metals are only: "metals.javaHome": "c:\\jdk-21"

Here's the log output...

Metals version: 1.2.2
Using coursier located at C:\Users\arthurm\AppData\Local\Coursier\data\bin\cs.bat
No installed java with version 17 found. Will fetch one using coursier.
Coursier: Checking if the JAVA_HOME, PATH user environment variable(s) need(s) updating.
Coursier: Some global environment variables were updated. It is recommended to close this terminal once the setup command is done, and open a new one for the changes to be taken into account.
Using Java Home: C:\Users\arthurm\AppData\Local\Coursier\cache\arc\https\github.com\adoptium\temurin17-binaries\releases\download\jdk-17.0.8%252B7\OpenJDK17U-jdk_x64_windows_hotspot_17.0.8_7.zip\jdk-17.0.8+7

@tgodzik
Copy link
Contributor

tgodzik commented Feb 27, 2024

We also should probably use --setup flag to avoid setting JAVA_HOME for the user.

Quite the contrary I think. We are using the flag and we shouldn't.

I was actually trying to write the opposite, but I lost the "n't" 😅

@kasiaMarek
Copy link
Contributor

I made some changes that should already be on the newest snapshot. It won't probably resolve the issue of discovering your JAVA_HOME but it should provide more logging information and won't mess-up your JAVA_HOME and PATH variables anymore. So if you're willing to try it I'm hoping the additional logs will help us debug this further.

@Arthurm1
Copy link
Contributor Author

It doesn't change my env vars now - which is good. But still downloads jdk 17 and uses that.

My only VSCode settings are "metals.javaHome": "c:\\jdk-21"

Logs...

Metals version: 1.2.2
Using coursier located at C:\Users\arthurm\AppData\Local\Coursier\data\bin\cs.bat
C:\jdk-21\bin\java -version:
openjdk version "21" 2023-09-19 LTS
OpenJDK Runtime Environment Temurin-21+35 (build 21+35-LTS)
C:\jdk-21\bin\java -version:
OpenJDK 64-Bit Server VM Temurin-21+35 (build 21+35-LTS, mixed mode, sharing)
No installed java with version 17 found. Will fetch one using coursier.
Coursier: openjdk version "17.0.8" 2023-07-18
OpenJDK Runtime Environment Temurin-17.0.8+7 (build 17.0.8+7)
Coursier: OpenJDK 64-Bit Server VM Temurin-17.0.8+7 (build 17.0.8+7, mixed mode, sharing)
Using Java Home: C:\Users\arthurm\AppData\Local\Coursier\cache\arc\https\github.com\adoptium\temurin17-binaries\releases\download\jdk-17.0.8%252B7\OpenJDK17U-jdk_x64_windows_hotspot_17.0.8_7.zip\jdk-17.0.8+7

If I add the setting "metals.javaVersion": "21" then it still downloads a JDK...

etals version: 1.2.2
Using coursier located at C:\Users\arthurm\AppData\Local\Coursier\data\bin\cs.bat
C:\jdk-21\bin\java -version:
openjdk version "21" 2023-09-19 LTS
C:\jdk-21\bin\java -version:
OpenJDK Runtime Environment Temurin-21+35 (build 21+35-LTS)
OpenJDK 64-Bit Server VM Temurin-21+35 (build 21+35-LTS, mixed mode, sharing)
No installed java with version 21 found. Will fetch one using coursier.
Coursier: openjdk version "21.0.2" 2024-01-16 LTS
OpenJDK Runtime Environment Temurin-21.0.2+13 (build 21.0.2+13-LTS)
Coursier: OpenJDK 64-Bit Server VM Temurin-21.0.2+13 (build 21.0.2+13-LTS, mixed mode, sharing)
Using Java Home: C:\Users\arthurm\AppData\Local\Coursier\cache\arc\https\github.com\adoptium\temurin21-binaries\releases\download\jdk-21.0.2%252B13\OpenJDK21U-jdk_x64_windows_hotspot_21.0.2_13.zip\jdk-21.0.2+13

@kasiaMarek
Copy link
Contributor

kasiaMarek commented Feb 29, 2024

Yeah, thanks of doing this. So the issue is that we check java version by running java -version and looking for something matching /\d+\.\d+\.\d+/ and it just shows "21" in your case.

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 a pull request may close this issue.

3 participants