elixir-ls icon indicating copy to clipboard operation
elixir-ls copied to clipboard

Discussion: Improving the deps handling

Open axelson opened this issue 5 years ago • 3 comments

Current state of affairs:

  • ElixirLS has a separate _build directory (within the .elixir_ls folder)
  • ElixirLS shares the deps folder with the main repository
  • ElixirLS automatically downloads deps when you change your mix.exs
  • ElixirLS sometimes gets stuck with a compilation error, possibly related to the automatic deps downloading

In #197 it has been proposed that ElixirLS should have it's own deps folder (in addition to _build). We have also discussed in Slack the issues with the automatic dep downloading (although I'm not sure we have an actual issue filed for it since it can be resolved by removing the .elixir_ls directory, although that is very annoying). I hypothesize that sharing the deps directory is related to the ElixirLS build getting stuck in a bad state.

My proposal is to:

  1. Give ElixirLS it's own deps folder within the .elixir_ls directory
  2. Remove the automatic dep downloading (since it is not reliable enough and relies on a significant amount of mix private api's)
  3. Instead, when mix.exs is updated with new dependencies show the user a message like "mix.exs has been updated and ElixirLS requires a restart (or restart your entire editor)"
  4. Whenever the server starts ensure that mix deps.get is run (if it is needed)

Any other approaches/feedback?

axelson avatar Apr 12 '20 18:04 axelson

My 2 cents: if the deps directory gets moved into .elixir_ls the auto-download feature should work without issues so I don't know if it's necessary to get rid of it (maybe make it optional?).

Agree 100% with having a separate deps from the main project tho 😃

cblage avatar Apr 12 '20 18:04 cblage

Beware of the noob idea ahead!!!

What if we didn't depend on mix/compiling/downloading at all? I mean, we need code "loaded" into the BEAM instance for completion, references and so on. As far as I can tell, if we could simply load what the main project does with deps/code/etc, than it wouldn't be needed to perform all those complicated tasks.

I know we depend on "compiling" for some diagnostics info like compiler warnings and so on. But, can't we mix the loading of compiled code with a parser (like this one) and provide all the features we currently provide?

Sorry if there is a huge hole on my theory... I am NOT an expert at all on compilers and language servers...

victorolinasc avatar Apr 27 '20 14:04 victorolinasc

@victorolinasc I'm pretty sure that not compiling the code would require a substantial re-write of most of the tooling in ElixirLS/ElixirSense.

We might be able to use MIX_DEPS_PATH which was added in 1.10:

  • MIX_DEPS_PATH - sets the project Mix.Project.deps_path/0 config

Although the lack of support on earlier elixir versions could prove to be an issue (but maybe it is fine if the deps continues to be shared in that case).

My worry after this is implemented is that if there is a failure in deps fetching, it may be more difficult to recover from. But if we make this opt-in via #226 we can have a longer testing period which would significantly help.

axelson avatar Apr 29 '20 17:04 axelson

Closing this for now. Auto dep download is disabled by default and I don't have plans for changing that.

What if we didn't depend on mix/compiling/downloading at all?

With tracer support we are moving closer to being able to do that ;)

lukaszsamson avatar Oct 06 '22 16:10 lukaszsamson