Skip to content
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.

Sign up for GitHub

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

Jump to bottom

asdf does not support sh #114

Closed
axelson opened this issue Feb 7, 2020 · 2 comments · Fixed by #118
Closed

asdf does not support sh #114

axelson opened this issue Feb 7, 2020 · 2 comments · Fixed by #118

Comments

@axelson
Copy link
Member

axelson commented Feb 7, 2020 •
edited
Loading

David Sweeney in this Elixir Forum post recently discovered the issue that our language_server.sh is no longer sh compatible because asdf (added in #78) is not currently sh compatible.

The error manifests itself with an error like:
/home/my_home/.asdf/asdf.sh: 8: /home/my_home/.asdf/asdf.sh: Bad substitution

I have raised an issue on asdf to determine if asdf is meant to support sh/dash, or only bash and other shells.

I see a few possible ways forward but would like to collect other's thoughts about them:

  1. Change language_server.sh to run bash instead of sh (via #!/usr/bin/env bash)
  2. Revert Support asdf-vm setup in wrapper scripts #78
  3. Wait for asdf to support sh

I'm currently leaning towards option 1 because bash is very widespread at this point and I'd imagine most systems will have it and for any that don't, the user will most likely understand how to fix the error.
@Trevoke
Copy link
Member

Trevoke commented Feb 7, 2020

I think it's fine to require bash on a system running elixir-ls --- sh compatibility is desirable, but not a need for us. As you said, bash is pretty much today's sh.

@axelson axelson mentioned this issue Feb 9, 2020
Merged
@axelson
Copy link
Member Author

axelson commented Feb 10, 2020

Agreed

axelson added a commit that referenced this issue Feb 10, 2020
@axelson
Change running scripts to run bash instead of sh (#118)
a9054b4 
While this lowers compatability somewhat, any system that a developer is
actively using is expected to have bash installed. This change is required
because asdf does not currently support sh, only bash and other more advanced
shells.

Fixes #114
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change running scripts to run bash instead of sh
2 participants
@axelson @Trevoke