IMO, just using makeWrapper make the binary too inflexible, I'd prefer to have a wrapper where I can override some of the values (e.g.: log level). My attempt for such wrapper: https://github.com/NixOS/nixpkgs/pull/107424/files#diff-cf18665ca6814490f3609b1c0c15fad90da2fd9e4d2867619e1f48ba92f2bd59

@jlesquembre Thanks for chiming in. I took a look at your PR and made a few comments. I think the approach with the script is pretty nice, but the downside is, each new option requires a change to the script so it isn't that flexible. I think it's pretty common to use JAVA_OPTS for this kind of thing, so in this version, the user can specify JAVA_OPTS="-Xmx1g -Dlog.level=ERROR". The other options (-configuration and -data) can actually be overridden by just passing as arguments to bin/jdt-language-server.

@fzakaria Could you maybe weigh in on this? @jlesquembre has created #107424 which adds some things to this PR.

I didn't do clear enough how to override the options with the script. I updated the comment in the script, hopefully is more clear now. With the script versin you can override any option (-data, -Xms1g, ...) without changing the script, (e.g.: jdt-ls.sh -data ... will override the -data value). I also added a new option -java-opts, which is basically doing the same that your JAVA_OPTS, but as a CLI argument (I find it a bit more convenient to use an argument when calling it from inside my editor ). If you are on java 9+ , instead of JAVA_OPTS, you could use JDK_JAVA_OPTIONS, which is supported by the jdk. One last question, I don't get how you can override the -data in your version, since you are using makeWrapper, calling bin/jdt-language-server -data ... will not expand to java ... -data /dir1 -data /dir2?

With the script versin you can override any option (-data, -Xms1g, ...) without changing the script, (e.g.: jdt-ls.sh -data ... will override the -data value).

I know but this isn't what I meant. I meant that adding new options -Xsome-other-option would require you updating the script to support them. Now that you've added the -java-opts parameter, this has been addressed. Still, it is perhaps a little inconsistent to treat some options as special (e.g. -Xms, -Xmx, -Dlog.level), and allow them directly to be passed, while others must be passed via -java-opts. There are two different ways to specify them: jdt-ls.sh -java-opts "-Xms1g" or jdt-ls.sh -Xms1g. Not a major issue, just pointing that out.

If you are on java 9+ , instead of JAVA_OPTS, you could use JDK_JAVA_OPTIONS, which is supported by the jdk.

This is good to know, I wasn't aware. If we are sure this will be run with jdk15 then we can actually switch to that option. Otherwise, we should keep JAVA_OPTS.

One last question, I don't get how you can override the -data in your version, since you are using makeWrapper, calling bin/jdt-language-server -data ... will not expand to java ... -data /dir1 -data /dir2

makeWrapper forwards the arguments so it does expand to that! Look at the shell script produced by my derivation (do nix-build -A jdt-language-server; cat result/bin/jdt-language-server).

#! /nix/store/kl6lr3czkbnr6m5crcy8ffwfzbj8a22i-bash-4.4-p23/bin/bash -e
mkdir -p ${XDG_CACHE_HOME:-$HOME/.cache}/jdt-language-server
install -Dm 1777 -t ${XDG_CACHE_HOME:-$HOME/.cache}/jdt-language-server/config /nix/store/snncbxah6ihw50wbzhrj6jazypbzcxn4-jdt-language-server-0.67.0/share/config/*
exec "/nix/store/v5bq0y1fraw4648b48vfx5c4snfvsk8j-openjdk-15.0.1-ga/bin/java"  -Declipse.application=org.eclipse.jdt.ls.core.id1 -Dosgi.bundles.defaultStartLevel=4 -Declipse.product=org.eclipse.jdt.ls.core.product -Dlog.level=ALL -noverify $JAVA_OPTS -jar /nix/store/snncbxah6ihw50wbzhrj6jazypbzcxn4-jdt-language-server-0.67.0/share/java/plugins/org.eclipse.equinox.launcher_1.6.0.v20200915-1508.jar --add-modules=ALL-SYSTEM --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED -configuration "${XDG_CACHE_HOME:-$HOME/.cache}/jdt-language-server/config" -data "${XDG_CACHE_HOME:-$HOME/.cache}/jdt-language-server/data$PWD" "$@"

makeWrapper forwards the arguments so it does expand to that!

I get that adding an argument it's ok, but how to overwrite an argument? Maybe I'm missing something, but if the (simplified) version of the script produced by your derivation (result/bin/jdt-language-server) is:

exec "java" -Dlog.level=ALL -jar launcher.jar -data $HOME/.cache/jdt.ls/data "$@"

running

jdt-language-server -Dlog.level=INFO -data /tmp/jdtls_data -foo bar

expands to

exec "java" -Dlog.level=ALL -jar launcher.jar -data $HOME/.cache/jdt.ls/data -Dlog.level=INFO -data tmp/jdtls_data -foo bar

been able to add the -foo arguments it's nice, but it's not possible to overwrite the -Dlog.level nor the -data arguments

I know but this isn't what I meant. I meant that adding new options -Xsome-other-option would require you updating the script to support them. Now that you've added the -java-opts parameter, this has been addressed. Still, it is perhaps a little inconsistent to treat some options as special (e.g. -Xms, -Xmx, -Dlog.level), and allow them directly to be passed, while others must be passed via -java-opts. There are two different ways to specify them: jdt-ls.sh -java-opts "-Xms1g" or jdt-ls.sh -Xms1g. Not a major issue, just pointing that out.

I agree that having multiple ways to update some arguments can be confusing. I'm not opposed to remove -Xms and -Xmx, maybe is better that way. I wanted to provide some sensible defaults, while providing an option to overwrite those values, but maybe I'm over-engineering it.
But I still think it would be a good idea to keep the -java-opts AND the JAVA_OPTS (or JDK_JAVA_OPTIONS). You will use only one, but depending where you call the command, one is more convenient. For example, in a vim script, feels more natural to use the argument, while in a shell script the env variable looks fine.

Thanks for your reply.

been able to add the -foo arguments it's nice, but it's not possible to overwrite the -Dlog.level nor the -data arguments

I'm not sure if we are talking about two different things. All I meant was that the rightmost argument will override previous values, so the second instance of -data wins. You can try this out by setting a different value and seeing that jdt ls creates the files there.

Now I get it, thanks for clarifying it. I didn't know that the second argument wins. Is that always the case? Not sure if we can rely on that behavior, I didn't find anything about it in jdt.ls documentation.
What about JAVA_OPTS? It also looks like that the second option has preference here, but it seems fragile, I think that relying on the order of the options is not a good idea.

Strange, I have a different sha256: 0ll5rgd8i8wfg2zz0ciapakl66qqaw344129qj72cyiixkgjh31g

I've been using this since around Dec 19th

Creating a runtime path is unnecessary.
The following flag should be enough to prevent jdtls from writing to the nix store:

-Dosgi.sharedConfiguration.area.readOnly=true

@mrcjkb Thanks for the tip I was not aware. This solves for the option -configuration "${runtimePath}/config" and I could instead do -configuration $out/share/config, but it does not solve for -data "${runtimePath}/data$PWD". How would you recommend storing the per-project runtime data?

@matt-snider I don't think specifying where to store the per-project runtime data should be the responsibility of the launcher script.
I don't know how it is done in VSCode. But in NeoVim, I specify the -data directory based on the current working directory (see the following example).

@mrcjkb I agree with you on this and I think making the -data parameter a caller's concern is the right choice. However, the readonly configuration does not work. I added the option you suggested as well as several others mentioned here. JDT LS still attempts to write logs to the config location:

The configuration area at '/nix/store/1dsx8w2aikc07209iymwlmwv9z5d4x1d-jdt-language-server-1.8.0/share/config' is not writable.  Please choose a writable location using the '-configuration' command line option.

There is also this issue.

If you still think this should work, perhaps you could try it out locally and send me the invocation that works.

See most recent change where I moved -data to a caller-specified option

@matt-snider that's strange.
I have a package built from the milestone binaries in my NUR repo that works fine.
It does have some additional flags that are not set in your launcher, namely:

-Dosgi.checkConfiguration=true 
-Dosgi.configuration.cascaded=true

And I use the -Dosgi.sharedConfiguration.area option instead of the --configuration flag.
Some of the flags you have set are missing in my wrapper, too (e.g. the module path configurations and the log level).
I don't have time today, but I hope to check out your branch this weekend and play around with it.

@mrcjkb Ahh that works! Thank you for the tip that's much cleaner now. You can see the change here.

The problem was that I was setting all of the flags you did including -Dosgi.sharedConfiguration.area=$out/share/config but didn't leave out -configuration.

It would be great if you and any others could test out the derivation and make sure it still works properly for their use cases @Jumziey @winston0410

@matt-snider I'm a week late, but I just installed it from your branch, and it works for me in NeoVim.