elixir-ls does not have up-to-date script files

digyx commented 10 months ago

Describe the bug

elixir-ls introduced a launch.fish file and made changes to the launch.sh file to support the fish shell in 0.17.0. Even though I have 0.17.10 installed, the old version of the launch.sh file is there and the launch.fish file is missing entirely.

[nix-shell:/var/home/digyx/dotfiles]$ which elixir-ls | cut -d "/" -f -4 | xargs -I % ls -1 "%/lib"
debugger.bat
debugger.sh
elixir_ls_debugger-0.17.10.ez
elixir_ls_utils-0.17.10.ez
elixir_sense-2.0.0.ez
erl2ex_vendored-0.0.10.ez
erlex_vendored-0.2.6.ez
jason_v-1.4.0.ez
language_server-0.17.10.ez
language_server.bat
language_server.sh
launch.sh
path_glob_vendored-0.1.1.ez

If the user has rtx installed, then it will attempt to activate the rtx config with fish even though the current active shell is sh, causing the launch to fail. This was fixed and closed in v0.17.0, but the problem still persists in the nix package.

Steps To Reproduce

Steps to reproduce the behavior:

Run nix-shell -p elixir-ls
Run which elixir-ls | cut -d "/" -f -4 | xargs -I % ls -1 "%/lib"
See that the launch.fish file is missing
Run which elixir-ls | cut -d "/" -f -4 | xargs -I % cat "%/lib/launch.sh"
Compare the output to the one here in the 0.17.10 release. Note the case statement near the top (line 16) which determines which shell to relaunch in (if any). The fish option is missing in the nix version, but it's present in the repository.

Expected behavior

Files are the same as in the scripts/ directory of the elixir-ls repository https://github.com/elixir-lsp/elixir-ls/tree/v0.17.10/scripts

Notify maintainers

@ankhers @Br1ght0ne @DianaOlympos @gleber @happysalada @minijackson @yurrriq

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
output here

❯ nix-shell -p nix-info --run "nix-info -m"

system: "x86_64-linux"
host os: Linux 6.6.7-200.fc39.x86_64, Fedora Linux, 39.20231222.0 (Sericea), nobuild
multi-user?: yes
sandbox: yes
version: nix-env (Nix) 2.18.1
nixpkgs: /home/digyx/.nix-defexpr/channels/nixpkgs

Add a :+1: reaction to issues you find important.

happysalada commented 10 months ago

hey, thank you for the report!

in the install phase of elixir-ls, we do try to copy everything that is released into the derivation cp -Rv release $out/lib

The fix could be straightforward, if you know where the launch.fish file is and where it should be copied.

I'm a bit short on time at the moment, but I would be happy to review a PR if anyone is interested in contributing.

dpatterbee commented 9 months ago

The underlying issue here is that we build elixir-ls using the deprecated .ez based release system. which doesn't seem to have added the fish launch script. I believe this is also the issue that we can't build elixir-ls with elixir 1.16 either, as the old build system makes use of a function removed in elixir 1.16.

I don't know how complex the fix is here, it could be as simple as appending a 2 to this line, but I feel like all of those substitutions would need double checked.

happysalada commented 9 months ago

@dpatterbee very nice find! I'll try to make a PR when I get time, if anyone beats me to it, Happy to help get it merged.

happysalada commented 9 months ago

@dpatterbee we made some tests for your idea in https://github.com/NixOS/nixpkgs/pull/282842 unfortunately it's not as straightfoward as just changing the expression. elixir-ls tries to install itself from source, which is less than ideal. Just to say that there is a bit more to figure out that riddle.

NixOS / nixpkgs