isomarcte / metals-pkgbuild

Scala Language Server (Metals) Package For Pacman Based Systems (e.g. Arch Linux)
0 stars 0 forks source link

metals-vim is using default system java #7

Closed 2m closed 5 years ago

2m commented 5 years ago

I get the following error in coc.nvim when opening a Scala file with nvim:

[coc.nvim] Unsupported Java version 11.0.1-testing, no functionality will work. To fix this problem, restart the server using Java 8.
[coc.nvim] Internal error.
[coc.nvim] Error output from languageserver.metals: Server initialization failed.
  Message: Internal error.
  Code: -32603

I am running Java 11 by default, but have Java 8 also installed under /usr/lib/jvm/java-8-openjdk/jre/bin/.

To reproduce, you'll have to have nvim installed and then also install vim-plug.

After then execute nvim -u metals.vim App.scala where metals.vim would contain:

call plug#begin('~/.local/share/nvim/plugged')
Plug 'neoclide/coc.nvim', {'do': { -> coc#util#install()}}
call plug#end()

Then type :messages to see the full error.

isomarcte commented 5 years ago

Any idea how to fix this?

[coc.nvim] error: Uncaught exception: Error: Header must provide a Content-Length property.
    at StreamMessageReader.onData (/home/foobar/.local/share/nvim/plugge
d/coc.nvim/build/index.js:38575:27)
    at Socket.readable.on (/home/foobar/git/.local/share/nvim/plugged/coc.nv
im/build/index.js:38560:18)
    at Socket.emit (events.js:193:13)
    at addChunk (_stream_readable.js:295:12)
    at readableAddChunk (_stream_readable.js:276:11)
    at Socket.Readable.push (_stream_readable.js:231:10)
    at Pipe.onStreamRead (internal/stream_base_commons.js:154:17)
2m commented 5 years ago

Haven't seen this before. Boot looks similar to: https://github.com/neoclide/coc-solargraph/issues/6

2m commented 5 years ago

I think that is a fallout from the communication failure with metals-vim. I have upgraded to the updated metals AUR package, and now I do not get the unsupported Java error anymore when using Java 11, but in the .metals/metals.log I see the following:

SEVERE: Internal error: java.lang.ClassCastException: class jdk.internal.loader.ClassLoaders$AppClassLoader cannot be cast to class java.net.URLClassLoader (jdk.internal.loader.ClassLoaders$AppClassLoader and java.net.URLClassLoader are in module java.base of loader 'bootstrap')
java.util.concurrent.CompletionException: java.lang.ClassCastException: class jdk.internal.loader.ClassLoaders$AppClassLoader cannot be cast to class java.net.URLClassLoader (jdk.internal.loader.ClassLoaders$AppClassLoader and java.net.URLClassLoader are in module java.base of loader 'bootstrap')

which I do not get when using Java 8.

In the :messages I see the same as you: Header must provide a Content-Length property.

2m commented 5 years ago

I also noticed that Java 8 enforcement works when using regular Archlinux java packages. It did not work when I was using jabba for Java management.

isomarcte commented 5 years ago

@2m I think we may have several different issues here.

In the :messages I see the same as you: Header must provide a Content-Length property.

I am suspicious that this is not related to metals at all, but to coc.nvim, though I'm not sure. I think it may have been a transient issue related to a JavaScript dependency...but again I'm not really sure. However, as of this morning I am no longer getting that error. With the latest metals build on AUR, I am able to open a Scala file with the system Java set to 11 and metals does not report an error about the version being wrong.

Can you check to see if you are still having the issue? If you are, the new metals package echos to stderr on startup the Canonical path to the Java binary it will use, can you run > metals-vim and see what the output is? You'll have to Ctrl-C the process because metals just waits for input from a LSP plugin otherwise, but you should at lease see the canonical path. It should look something like this.

[simon]clear: 1 ~ λ > archlinux-java get
java-11-openjdk
[simon]~ λ > metals-vim
Java binary selected: /usr/lib/jvm/java-8-openjdk/jre/bin/java

I also noticed that Java 8 enforcement works when using regular Archlinux java packages. It did not work when I was using jabba for Java management.

I'm aware of this issue and it is related to #3. It is a tricky problem to solve in the general case. In order to run a JVM binary with an arbitrary java executable we need to know where that executable is in the first place. The current package knows the normal installation paths for the Arch Linux OpenJDK 8 binaries and thus can use that to determine how to setup the PATH and launch metals with the correct binary. See Arch Linux's documentation on starting a JVM executable with a specific Java version..

So in order to support running metals with Java versions installed by jabba, I'd need to encode their installation paths. We certainly can do that, and in fact I've been planning on doing that for the other JDK binaries in AUR, but I just haven't got around to it yet. There are some other minor problems to solve, such as if there are two valid Java 8 installations which one do we choose, but they are certainly solvable as well. I'll see if I can get around to adding jabba support in the next couple days.

2m commented 5 years ago

In the :messages I see the same as you: Header must provide a Content-Length property.

With the latest metals AUR package I get this error. But that is because, I believe, metals communicate with vim using std in/out. And printing out resolved java version trips it up.

I do not get that error if I manually comment out the

echo "Java binary selected: $(command -v java)"

line from metals-client.

I'll see if I can get around to adding jabba support in the next couple days.

I have switched to jdk11-adoptopenjdk AUR package instead of using jabba and now Java 8 is correctly selected. So I think we can close this, and we can postpone jabba support for way later. :)

isomarcte commented 5 years ago

@2m you need to update to 0.5.2-3. That changes the echo to go to stderr rather stdout to fix that problem. I noticed it yesterday.

2m commented 5 years ago

I see. That makes sense! Thanks.

On Fri, May 17, 2019, 16:07 David Strawn notifications@github.com wrote:

@2m https://github.com/2m you need to update to 0.5.2-3. That changes the echo to go to stderr rather stdout to fix that problem. I noticed it yesterday.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/isomarcte/metals-pkgbuild/issues/7?email_source=notifications&email_token=AADHBRVRXVE6AKOW6NWJXDLPV2U2RA5CNFSM4HNM3ALKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVUWNZY#issuecomment-493446887, or mute the thread https://github.com/notifications/unsubscribe-auth/AADHBRTFTYHWFMJ3YKMVUO3PV2U2RANCNFSM4HNM3ALA .