lexical-lsp / lexical

Lexical is a next-generation elixir language server
869 stars 80 forks source link

mix.exs invoked outside of project root #412

Closed nshafer closed 11 months ago

nshafer commented 11 months ago

When Lexical boots up and starts building the user's project, code in mix.exs is being executed from each dependencies directory so that File.cwd!() returns "project_root/deps/phoenix" for example. So if there is code in mix.exs that is assuming that it's executed from the project root, as is the only way that I know that mix.exs is executed normally by mix commands, then it will crash the build.

This is a weird edge case, but I think it could trip people up and not be obvious about what is wrong. Here's an example mix.exs:

def project do
  [
    app: :myapp,
    version: version(),
    ...
  ]
end

def version() do
  File.read!("VERSION")
  |> String.trim()
end

Starting iex -S mix, mix phx.server and mix release all work perfectly fine, and Mix.Project.config()[:version] returns the contents of the VERSION file.

However when starting VS Code after removing ".lexical" to simulate a first-start, nothing happens at first, except that completions from dependencies don't work. However, once I save with code-format-on-save enabled, it finally reports that Lexical is erroring and then all of vs code hangs with these errors in the bottom right:

image

Looking in View->Output and selecting Lexical shows this in the log:

Starting lexical release in "/mnt/data/projects/elixir/lexical/_build/dev/package/lexical/bin"
warning: redefining module Api179.MixProject (current version defined in memory)
  /mnt/data/projects/phoenix/api179/mix.exs:1: Api179.MixProject (module)

== Compilation error in file lib/floki/deep_text.ex ==
** (File.Error) could not read file "VERSION": no such file or directory
    (elixir 1.15.6) lib/file.ex:358: File.read!/1
    /mnt/data/projects/phoenix/api179/mix.exs:17: Api179.MixProject.version/0
    /mnt/data/projects/phoenix/api179/mix.exs:7: Api179.MixProject.project/0
    (lx_lexical_shared 0.0.1) lib/lexical/project.ex:95: LXical.Project.display_name/1
    (lx_remote_control 0.3.0) lib/lexical/remote_control/build/state.ex:202: LXical.RemoteControl.Build.State.building_label/1
    (lx_remote_control 0.3.0) lib/lexical/remote_control/compilation/tracer.ex:78: LXical.RemoteControl.Compilation.Tracer.progress_message/1
    (lx_remote_control 0.3.0) lib/lexical/remote_control/compilation/tracer.ex:62: LXical.RemoteControl.Compilation.Tracer.maybe_report_progress/1
    (lx_remote_control 0.3.0) lib/lexical/remote_control/compilation/tracer.ex:10: LXical.RemoteControl.Compilation.Tracer.trace/2
could not compile dependency :floki, "mix compile" failed. Errors may have been logged above. You can recompile this dependency with "mix deps.compile floki --force", update it with "mix deps.update floki" or clean it with "mix deps.clean floki"
warning: the log level :warn is deprecated, use :warning instead
  (logger 1.15.6) lib/logger.ex:1137: Logger.elixir_level_to_erlang_level/1
  (logger 1.15.6) lib/logger.ex:591: Logger.compare_levels/2
  (logger_file_backend 0.0.13) lib/logger_file_backend.ex:36: LoggerFileBackend.handle_event/2
  (stdlib 5.1.1) gen_event.erl:802: :gen_event.server_update/4
  (stdlib 5.1.1) gen_event.erl:784: :gen_event.server_notify/4
  (stdlib 5.1.1) gen_event.erl:526: :gen_event.handle_msg/6

Changing the version() function in mix.exs to this in a different editor (since vs code just hangs soon after start) or after disabling lexical:

def version do
  Mix.shell().error("version run from #{File.cwd!()}")
  "1.2.3"
end

allows lexical to start, but View -> Output is full of hundreds if not thousands of lines like the following:

Starting lexical release in "/mnt/data/projects/elixir/lexical/_build/dev/package/lexical/bin"
version run from /mnt/data/projects/phoenix/api179
version run from /mnt/data/projects/phoenix/api179/deps/floki
version run from /mnt/data/projects/phoenix/api179/deps/decimal
version run from /mnt/data/projects/phoenix/api179/deps/mime
version run from /mnt/data/projects/phoenix/api179/deps/nimble_options
...

This can be fixed by being more defensive in version() and not allowing it to crash, of course, and is a valid workaround. But since it isn't needed for normal usage, this might trip up other people. Either the cwd needs to be fixed, or Lexical needs to guard against mix.exs crashing. Or even better, it needs to just call mix.exs once from the root of the project and cache the response.

I also tested this same project with Elixir LS, no crash. Just one log line in View -> Output:

[Warn  - 11:17:21 AM] version run from /mnt/data/projects/phoenix/api179

Lexical compiled from main on 2023-10-12 as of commit #aa1de2d8de0d7754e1080adaeb6245bd63270f13 with elixir "1.13.4-otp-24" and erlang "24.3.4.14" on an Ubuntu 22.04 host, vs code version 1.83.1.

(BTW, I do this pattern often with projects that are built into debian packages, so that other tooling can read the same VERSION and I only have to update one place.)

scohen commented 11 months ago

This is a super interesting issue. Thanks for reporting it. Lexical actually goes through great lengths to stay in your project directory, but compilation changes directories all the time, and lexical can't control that. The issue is actually due to lexical creating a progress message saying "building <your project name>" for your editor to display. The problem is it consults your project module to do this, which causes the error you mentioned. I'll need to investigate the usages of the mix project module, but it looks like we shouldn't really use it inside the Lexical.Project module, as it's unsafe, and instead get it via mix in the remote control app. Calculating the project name can be done when we create the project struct.

The issue was initially baffling to me, but your excellent bug report made it clear what was going on. Thanks for being so thorough

scohen commented 11 months ago

@nshafer, I'm going to fix this, but to make your code bulletproof, I would recommend reading the version like this:

version = __DIR__ |> Path.join("VERSION") |> File.read!()

That will work regardless of where the mix.exs is evaluated from.

I quickly skimmed the mix docs, and couldn't find anything that would guarantee that the mix.exs will be evaluated in the project root (indeed, for umbrella projects, this can happen from the umbrella app or the project root), but if i missed it, please correct my mistake.

nshafer commented 11 months ago

Oh for sure, I don't think that Mix has ever guaranteed where mix.exs is executed from. This is just based off of normal behavior that users might be used to, like me. (I also don't mess with Umbrellas.) It's just more worrying that Lexical doesn't handle any kind of crash in mix.exs... it could be anything else such as plain old syntax errors. Thanks for looking into it!

scohen commented 11 months ago

It's just more worrying that Lexical doesn't handle any kind of crash in mix.exs

A crash in the mix.exs means that compilation will fail, no LSP will be able to do a project compilation if this is the case. It's important to understand that this wasn't really caused by a crash in mix.exs, it was caused because mix.exs was being evaluated outside of your project's context, which shouldn't happen.

nshafer commented 11 months ago

Gotcha. Glad it was fixable, hopefully easily. Thanks again!

scohen commented 11 months ago

of course.