-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
elixir-ls: fix elixir stdlib code detection, elixir: fix deterministic builds and 1.17 max OTP #423588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
elixir-ls: fix elixir stdlib code detection, elixir: fix deterministic builds and 1.17 max OTP #423588
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
169 changes: 169 additions & 0 deletions
169
pkgs/development/beam-modules/elixir-ls/launch.sh.patch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| diff --git i/scripts/launch.sh w/scripts/launch.sh | ||
| index 21afbb1e..975cbdf0 100755 | ||
| --- i/scripts/launch.sh | ||
| +++ w/scripts/launch.sh | ||
| @@ -1,125 +1,4 @@ | ||
| -#!/bin/sh | ||
| -# Actual launcher. This does the hard work of figuring out the best way | ||
| -# to launch the language server or the debug adapter. | ||
| - | ||
| -# Running this script is a one-time action per project launch, so we opt for | ||
| -# code simplicity instead of performance. Hence some potentially redundant | ||
| -# moves here. | ||
| - | ||
| - | ||
| -did_relaunch=$1 | ||
| - | ||
| -# Get the user's preferred shell | ||
| -preferred_shell=$(basename "$SHELL") | ||
| - | ||
| -# Get current dirname | ||
| -dirname=$(dirname "$0") | ||
| - | ||
| -case "${did_relaunch}" in | ||
| - "") | ||
| - if [ "$preferred_shell" = "bash" ]; then | ||
| - >&2 echo "Preferred shell is bash, relaunching" | ||
| - exec "$(which bash)" "$0" relaunch | ||
| - elif [ "$preferred_shell" = "zsh" ]; then | ||
| - >&2 echo "Preferred shell is zsh, relaunching" | ||
| - exec "$(which zsh)" "$0" relaunch | ||
| - elif [ "$preferred_shell" = "fish" ]; then | ||
| - >&2 echo "Preferred shell is fish, launching launch.fish" | ||
| - exec "$(which fish)" "$dirname/launch.fish" | ||
| - else | ||
| - >&2 echo "Preferred shell $preferred_shell is not supported, continuing in POSIX shell" | ||
| - fi | ||
| - ;; | ||
| - *) | ||
| - # We have an arg2, so we got relaunched | ||
| - ;; | ||
| -esac | ||
| - | ||
| -# First order of business, see whether we can setup asdf | ||
| -echo "Looking for asdf install" >&2 | ||
| - | ||
| -readlink_f () { | ||
| - cd "$(dirname "$1")" > /dev/null || exit 1 | ||
| - filename="$(basename "$1")" | ||
| - if [ -h "$filename" ]; then | ||
| - readlink_f "$(readlink "$filename")" | ||
| - else | ||
| - echo "$(pwd -P)/$filename" | ||
| - fi | ||
| -} | ||
| - | ||
| -export_stdlib_path () { | ||
| - which_elixir_expr=$1 | ||
| - stdlib_path=$(eval "$which_elixir_expr") | ||
| - stdlib_real_path=$(readlink_f "$stdlib_path") | ||
| - ELX_STDLIB_PATH=$(echo "$stdlib_real_path" | sed "s/\(.*\)\/bin\/elixir/\1/") | ||
| - export ELX_STDLIB_PATH | ||
| -} | ||
| - | ||
| -# Check if we have the asdf binary for version >= 0.16.0 | ||
| -if command -v asdf >/dev/null 2>&1; then | ||
| - asdf_version=$(asdf --version 2>/dev/null) | ||
| - version=$(echo "$asdf_version" | grep -Eo '[0-9]+\.[0-9]+\.[0-9]+') | ||
| - major=$(echo "$version" | cut -d. -f1) | ||
| - minor=$(echo "$version" | cut -d. -f2) | ||
| - # If the version is less than 0.16.0 (i.e. major = 0 and minor < 16), use legacy method. | ||
| - if [ "$major" -eq 0 ] && [ "$minor" -lt 16 ]; then | ||
| - ASDF_DIR=${ASDF_DIR:-"${HOME}/.asdf"} | ||
| - ASDF_SH="${ASDF_DIR}/asdf.sh" | ||
| - if test -f "$ASDF_SH"; then | ||
| - >&2 echo "Legacy pre v0.16.0 asdf install found at $ASDF_SH, sourcing" | ||
| - # Source the old asdf.sh script for versions <= 0.15.0 | ||
| - . "$ASDF_SH" | ||
| - else | ||
| - >&2 echo "Legacy asdf not found at $ASDF_SH" | ||
| - fi | ||
| - else | ||
| - >&2 echo "asdf executable found at $(command -v asdf). Using ASDF_DIR=${ASDF_DIR}, ASDF_DATA_DIR=${ASDF_DATA_DIR}." | ||
| - fi | ||
| - export_stdlib_path "asdf which elixir" | ||
| -else | ||
| - # Fallback to old method for version <= 0.15.x | ||
| - ASDF_DIR=${ASDF_DIR:-"${HOME}/.asdf"} | ||
| - ASDF_SH="${ASDF_DIR}/asdf.sh" | ||
| - if test -f "$ASDF_SH"; then | ||
| - >&2 echo "Legacy pre v0.16.0 asdf install found at $ASDF_SH, sourcing" | ||
| - # Source the old asdf.sh script for versions <= 0.15.0 | ||
| - . "$ASDF_SH" | ||
| - export_stdlib_path "asdf which elixir" | ||
| - else | ||
| - >&2 echo "asdf not found" | ||
| - >&2 echo "Looking for mise executable" | ||
| - | ||
| - # Look for mise executable | ||
| - if command -v mise >/dev/null 2>&1; then | ||
| - >&2 echo "mise executable found at $(command -v mise), activating" | ||
| - eval "$($(command -v mise) env -s "$preferred_shell")" | ||
| - export_stdlib_path "mise which elixir" | ||
| - else | ||
| - >&2 echo "mise not found" | ||
| - >&2 echo "Looking for rtx executable" | ||
| - | ||
| - # Look for rtx executable | ||
| - if command -v rtx >/dev/null 2>&1; then | ||
| - >&2 echo "rtx executable found at $(command -v rtx), activating" | ||
| - eval "$($(command -v rtx) env -s "$preferred_shell")" | ||
| - export_stdlib_path "rtx which elixir" | ||
| - else | ||
| - >&2 echo "rtx not found" | ||
| - >&2 echo "Looking for vfox executable" | ||
| - | ||
| - # Look for vfox executable | ||
| - if command -v vfox >/dev/null 2>&1; then | ||
| - >&2 echo "vfox executable found at $(command -v vfox), activating" | ||
| - eval "$($(command -v vfox) activate "$preferred_shell")" | ||
| - else | ||
| - >&2 echo "vfox not found" | ||
| - export_stdlib_path "which elixir" | ||
| - fi | ||
| - fi | ||
| - fi | ||
| - fi | ||
| -fi | ||
| +#!/usr/bin/env bash | ||
|
|
||
| # In case that people want to tweak the path, which Elixir to use, or | ||
| # whatever prior to launching the language server or the debug adapter, we | ||
| @@ -138,29 +17,18 @@ fi | ||
| # script so we can correctly configure the Erlang library path to | ||
| # include the local .ez files, and then do what we were asked to do. | ||
|
|
||
| -if [ -z "${ELS_INSTALL_PREFIX}" ]; then | ||
| - SCRIPT=$(readlink_f "$0") | ||
| - SCRIPTPATH=$(dirname "$SCRIPT") | ||
| -else | ||
| - SCRIPTPATH=${ELS_INSTALL_PREFIX} | ||
| -fi | ||
| +SCRIPT=$(readlink -f "$0") | ||
| +SCRIPTPATH=$(dirname "$SCRIPT")/../libexec | ||
|
|
||
| export MIX_ENV=prod | ||
| # 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 | ||
|
|
||
| default_erl_opts="-kernel standard_io_encoding latin1 +sbwt none +sbwtdcpu none +sbwtdio none" | ||
|
|
||
| -if [ "$preferred_shell" = "bash" ]; then | ||
| - source "$dirname/exec.bash" | ||
| -elif [ "$preferred_shell" = "zsh" ]; then | ||
| - source "$dirname/exec.zsh" | ||
| -else | ||
| - if [ -z "$ELS_ELIXIR_OPTS" ] | ||
| - then | ||
| - # in posix shell does not support arrays | ||
| - >&2 echo "ELS_ELIXIR_OPTS is not supported in current shell" | ||
| - fi | ||
| - exec elixir --erl "$default_erl_opts $ELS_ERL_OPTS" "$SCRIPTPATH/launch.exs" | ||
| -fi | ||
| +# ensure elixir stdlib can be found | ||
| +ELX_STDLIB_PATH=${ELX_STDLIB_PATH:-@elixir@/lib/elixir} | ||
| +export ELX_STDLIB_PATH | ||
| + | ||
| +source "$SCRIPTPATH/exec.bash" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, this line is weird.
By default, it will use
hexto fetch the source again, and run with that instead of anything from Nixpkgs' package.With
ELS_LOCAL=1it 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.patchwe need.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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