scalameta / metals-vscode

Visual Studio Code extension for Metals
https://marketplace.visualstudio.com/items?itemName=scalameta.metals#overview
Apache License 2.0
300 stars 76 forks source link

metals-vscode incorrectly assumes java home directory. #1528

Open megri opened 2 months ago

megri commented 2 months ago

Describe the bug

Using asdf to manage jvm installs, vs code incorrectly assumes that a java reference found on $PATH has a home directory under <detected-java-path>/../.. This happens here: https://github.com/scalameta/metals-vscode/blob/5130d1d91cc47c391e94b2178baeedfdbf2bd4af/packages/metals-languageclient/src/getJavaHome.ts#L66

It then blindly tries to run <detected-java-path>/../../bin/java which fails.

To Reproduce Steps to reproduce the behavior:

  1. Install asdf: https://asdf-vm.com/guide/getting-started.html
  2. asdf plugin add java
  3. asdf install java zulu-21.36.19
  4. asdf global java zulu-21.36.19
  5. Open a Scala project and switch to the Output/Metals tab
  6. Output will look something like
Metals version: 1.3.5
Using coursier located at /opt/homebrew/bin/coursier
Searching for Java on PATH. Found java executable under /Users/you/.asdf/shims/java that resolves to /Users/you/.asdf/shims/java
failed while running /Users/you/.asdf/bin/java -version
Error: spawn /Users/you/.asdf/bin/java ENOENT
Java version doesn't match the required one of 17
No installed java with version 17 found. Will fetch one using coursier:
    openjdk version "17" 2021-09-14
    OpenJDK Runtime Environment Temurin-17+35 (build 17+35)
    OpenJDK 64-Bit Server VM Temurin-17+35 (build 17+35, mixed mode)

Expected behavior

If the actual JAVA_HOME is needed it could be found using something like this very hacky method I just pieced together:

echo 'System.getProperty("java.home")' | jshell -q 2>&1 | sed -n '1p' | sed -ne 's/^.*==> //p'

The jshell command should be located on $PATH as previously. Note that this imposes a delay of ~1.5 seconds on my setup.

Screenshots

Installation:

Additional context

Search terms

java, jvm, javaHome, JAVA_HOME

tgodzik commented 2 months ago

Thanks for reporting! How does the directory structure look like for /Users/you/.asdf ? shims/java is only the binary? I wonder how that actually work :thinking:

megri commented 2 months ago

Yeah, shims/ contains a bunch of scripts that forwards to somewhere else. shims/java looks like

#!/usr/bin/env bash
# asdf-plugin: java graalvm-22.3.1+java17
# asdf-plugin: java zulu-11.64.19
# asdf-plugin: java zulu-11.66.19
# asdf-plugin: java zulu-11.74.15
# asdf-plugin: java zulu-17.34.19
# asdf-plugin: java zulu-17.50.19
# asdf-plugin: java zulu-21.30.15
# asdf-plugin: java zulu-21.36.19
exec /opt/homebrew/opt/asdf/libexec/bin/asdf exec "java" "$@" # asdf_allow: ' asdf '

Running the jshell snippet from above gives "/Users/you/.asdf/installs/java/zulu-21.36.19/zulu-21.jdk/Contents/Home".

megri commented 2 months ago

If JAVA_HOME is actually not needed then running java -version with realJavaPath should be fine. Not sure when/if it is actually appropriate to back out of the binary's path and re-enter through bin/.

tgodzik commented 2 months ago

We should be fine just fixing the logic to not assume we're in bin/java. We could possibly do the fix for Java sources inside metals server as a fallback. So we wouldn't need to invoke anything before actually starting metals.