elixir-ls: fix elixir stdlib code detection, elixir: fix deterministic builds and 1.17 max OTP#423588
Conversation
7079e3d to
6cd2198
Compare
6cd2198 to
2b32c9a
Compare
|
I'm on vacation this week, so I likely won't have a chance to review. If it works I'm happy 😊 |
|
It appears something changed in the most recent staging merge to master, and now some mixRelease packages are failing. e.g. next-ls, livebook and probably the others.
|
|
Ok, tracked down the failures to a regression in shebang patching. The fix for this is in staging already, so many erlang/elixir packages are broken until that gets to master. |
|
Successfully created backport PR for |
|
I'm pretty sure the patch is being applied (I copied the Is someone else observing the same? (Nixpkgs |
| # Mix.install prints to stdout and reads from stdin | ||
| # we need to make sure it doesn't interfere with LSP/DAP | ||
| -echo "" | elixir "$SCRIPTPATH/quiet_install.exs" >/dev/null || exit 1 | ||
| +echo "" | @elixir@/bin/elixir "$SCRIPTPATH/quiet_install.exs" >/dev/null || exit 1 |
There was a problem hiding this comment.
Ok, this line is weird.
By default, it will use hex to fetch the source again, and run with that instead of anything from Nixpkgs' package.
With ELS_LOCAL=1 it will run the interpreter in the project in $store/libexec/../, which will fail.
What exactly are we attempting to do here?
There was a problem hiding this comment.
This package needs more changes on top of what is in this PR. This was a first pass at cleanup, but this package is indeed weird in that it's not really a package but a set of wrapper scripts. Ideally we properly package it and set ELS_LOCAL so it uses code from the store instead of downloading. Until then I wouldn't expect setting that env var to work.
I've used these changes without issues though. Do you have a reproducer?
There was a problem hiding this comment.
I've used these changes without issues though. Do you have a reproducer?
I'm more curious what impurity made it work for you, since there is no codepath that uses the .beam built here, it always uses the one re-downloaded into the mix home path, without the 1216.patch we need.
There was a problem hiding this comment.
I understand the problem now, but it's still not clear to me why it worked on my machine.
Anyway, can you try this PR? #427805
The motivation is similar to (and implementation taken partly from) NixOS#271288 --- some rebar3-compiled `.beam` files contain debug information exposing references to `.hrl` files from the Erlang distribution it was compiled with. This results in unnecessary store references. See also NixOS#423588's change to Elixir's `generic-builder.nix` where the same option is used when building to avoid the same issue.
After digging into this, I realized this elixir-ls package is wrong in multiple ways. It's built as a mix release, but packaged as a set of scripts which will Mix.install elixir-ls from git. I plan to clean this up in a future PR, but this should fix the immediate problems.
Closes #422529
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.