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.

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

Should not execute config/runtime.exs #97

Closed
mhanberg opened this issue Jul 6, 2023 · 17 comments
Closed

Should not execute config/runtime.exs #97

mhanberg opened this issue Jul 6, 2023 · 17 comments
Labels
bug Something isn't working

Comments

@mhanberg
Copy link
Collaborator

mhanberg commented Jul 6, 2023

Description

This was noticed by a user who used a dependency in their config/runtime.exs file, which was not loaded yet because their app was not compiled at all.

But, they also showed that by using functions like System.fetch_env!/1, the config will fail and the app will fail to start.

Solution

Need to find a way to start the runtime without it executing any of the Application config files.

@mhanberg mhanberg added the bug Something isn't working label Jul 6, 2023
@lud-wj
Copy link
Contributor

lud-wj commented Jul 6, 2023

I think you will need to evaluate the compile-time config files, otherwise it is possible that some user modules do not compile at all. (For instance, anything that call Application.compile_env! is at risk.

So I believe it is just the runtime.exs that should be skipped.

A very very dirty solution would be to redefine the Mix.Tasks.Loadconfig module, with a no-op function as load_runtime/1.

What is strange is that NextLS is not supposed to boot the user application, is it? If I raise in my runtime.exs and run mix compile it works, because it is not evaluated at compile-time. So why does NextLS evaluates that file instead of its own configuration?

@mhanberg
Copy link
Collaborator Author

mhanberg commented Jul 6, 2023

I think you will need to evaluate the compile-time config files, otherwise it is possible that some user modules do not compile at all.

Yes, I was thinking that even dependencies sometimes need app config when compiling, so they have to run.

What is strange is that NextLS is not supposed to boot the user application, is it?

Next LS starts your project with a bunch of flags to not actually start anything, just load your code and deps into a VM that is then connected to Next LS via distributed erlang.

@mhanberg mhanberg changed the title Should not execute config/*.exs files Should not execute config/runtime.exs Jul 6, 2023
@lud-wj
Copy link
Contributor

lud-wj commented Jul 6, 2023

Would it be imaginable that NextLS starts a custom application in that VM, just adding the elixirc_paths of the user project to its own paths, dynamically or in mix.exs. That way you do not need flags because the mod in .app is not the user's.

That would sill require to copy the config files or something...

@mhanberg
Copy link
Collaborator Author

mhanberg commented Jul 6, 2023

I think a workaround for you right now would be to wrap your entire runtime.exs file in if not System.get_env("NEXT_LS_PARENT_PID") do

this env var is set by Next LS, so if it is present, it means you are in Next LS and can avoid executing anything

mhanberg added a commit that referenced this issue Jul 7, 2023
Adds a dedicated environment variable to indicate to the runtime that
you are in the Next LS runtime.

Should help address #97 as a work around.
mhanberg added a commit that referenced this issue Jul 7, 2023
Adds a dedicated environment variable to indicate to the runtime that
you are in the Next LS runtime.

Should help address #97 as a work around.
@mhanberg
Copy link
Collaborator Author

mhanberg commented Jul 7, 2023

I published 0.5.2 that sets an LSP variable, so you can do if System.get_env("LSP") do

@lud-wj
Copy link
Contributor

lud-wj commented Jul 7, 2023

Using NEXT_LS_PARENT_PID works with a custom run with TCP but it does not work with the default stdio adapter (and letting the extension start it itself)

[Error - 13:06:35] Client NextLS: connection to server is erroring. Shutting down server.
[Error - 13:06:35] Stopping server failed
Error: Client is not running and can't be stopped. It's current state is: starting

I'm using vscode and the last extension version is one week old so I did not try with the "LSP" variable.

Besides, when I start the tool from a shell ( NEXTLS_VERSION=0.5.1 /home/lud/.vscode/extensions/elixir-tools.elixir-tools-0.2.2/bin/nextls --port 9999 ) the vscode output shows that the app is compiling, an I can have the current file symbols, but there is no code formatting nor go-to-definition and the workspace symbols do not work, they always display the symbols for the same module. Not sure if it is expected or related to the runtime.exs problem though.

( Same with NEXTLS_VERSION=0.5.2 )

@mhanberg
Copy link
Collaborator Author

mhanberg commented Jul 7, 2023

The extension version is not tied to the NextLS version. When you start it normally it will fetch the latest version of NextLS.

With regard to your last statement, did you let it finish compiling? Or were those features not working while it was compiling?

@mhanberg
Copy link
Collaborator Author

mhanberg commented Jul 7, 2023

Can you remind me what your exact elixir and OTP versions are?

@lud-wj
Copy link
Contributor

lud-wj commented Jul 7, 2023

IIRC the compilation was long time finished. Plus listing the current file symbols works well.

I'm leaving the office for now but I can run any tests you want later.

Erlang/OTP 25 [erts-13.0] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit:ns]

Elixir 1.14.0 (compiled with Erlang/OTP 25)

mhanberg added a commit that referenced this issue Jul 12, 2023
This mix task does what we actually want, which is to load all of the
code paths and not start the app.

Fixes #97
Closes elixir-lang/elixir#12764
mhanberg added a commit that referenced this issue Jul 12, 2023
This mix task does what we actually want, which is to load all of the
code paths and not start the app.

Fixes #97
Closes elixir-lang/elixir#12764
mhanberg added a commit that referenced this issue Jul 12, 2023
This mix task does what we actually want, which is to load all of the
code paths and not start the app.

Fixes #97
Closes elixir-lang/elixir#12764
@mhanberg
Copy link
Collaborator Author

@lud-wj

Let me know if you get to try this out again and if it fixed your issues.

@lud-wj
Copy link
Contributor

lud-wj commented Jul 17, 2023

Hello, I see a vscode version from 4 days ago. Same errors on my side, telling that the server cannot be started

It works though, if I run the server manually and connect with TCP with this version of the extension:

NEXTLS_VERSION=0.5.4 /home/lud/.vscode/extensions/elixir-tools.elixir-tools-0.3.0/bin/nextls --port 9999

@mhanberg
Copy link
Collaborator Author

The vscode extension version is not tied to the next ls version. It always fetches the latest version of next ls automatically.

But, you now can set the version from the extension. Could you try that just to be sure?

@lud-wj
Copy link
Contributor

lud-wj commented Jul 17, 2023

I can confirm that it works now with this configuration:

  "elixir-tools.credo.enable": false,
  "elixir-tools.nextls.enable": true,
  "elixir-tools.nextls.version": "0.5.4",

Though "Go To Definition" works only for a few cases (full module name, no aliases, and only for some modules). Is there a way to debug that?

@mhanberg
Copy link
Collaborator Author

Delete the .elixir-tools folder so it forces it to rebuild the lookup table.

@lud-wj
Copy link
Contributor

lud-wj commented Jul 17, 2023

No changes unfortunately. I can try to debug to see why, I do not know when though.

@mhanberg
Copy link
Collaborator Author

Make sure they aren't anything in these cases that haven't been implemented yet: https://www.elixir-tools.dev/next-ls/#definition

@lud-wj
Copy link
Contributor

lud-wj commented Jul 17, 2023

Ah yes, that should be it. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants