spike: incorporate namespaced injectable application

[mhanberg]

Problem

We aren't sure how Lexical's namespacing and injectable application (RemoteControl in Lexical parlance) are going to work with packaging the
LSP with Burrito.

Solution

Burrito

Let's start with a description of how Burrito works.

Burrito starts as a normal mix release, but adds a step (Burrito.wrap/1) that does several things

• Downloads the precompiled OTP builds from the beam machine (https://beammachine.cloud) (it has builds for each OS and architecture and can package your project for any of them)
• Discovers any NIFs that use elixir_make and cross compiles them for the target.
• Packages your mix release, the cross compiled NIFs, and the downloaded OTP into a single executable.

That final executable you can just send to anyone and they can run it. The Linux builds are portable as they are statically linked with OpenSSL and musl-libc

With that in mind, the way that we mix (heh) Burrito and the namespacing/injectable application are with another release step and squeeze it between the mandatory :assemble step and the Burrito.wrap/ step.

This step is basically a short function that builds the injectable application (hereby known as the Engine) and copies its beam files into our applications priv directory. The burrito step
will then copy that all into the executable and we're golden!

defmodule Expert.Release do
  def assemble(release) do
    engine_path = Path.expand("../../../engine", __DIR__)

    {_, 0} =
      System.cmd("mix", ["build"],
        cd: engine_path,
        env: [
          {"MIX_ENV", to_string(Mix.env())}
        ]
      )

    source = Path.join([engine_path, "_build/#{Mix.env()}"])

    dest =
      Path.join([
        release.path,
        "lib",
        "#{release.name}-#{release.version}",
        "priv"
      ])

    File.cp_r!(source, dest)

    release
  end
end

The Engine and Namespacing

I called the "injectable" application "Engine". Thought it sounded cool, but just a name, not tied to it.

The repository is set up as a monorepo, with directories for expert, engine, and namespace, each being normal mix projects.

The namespace project is a slightly modified version for the mix task of the same name from Lexical.

I still need to do a proper diff to recall the things I changed, but the non-superficial (parameterizing things like project path) changed
are related to finding the beam files to namespace. Previously, it wasn't finding consolidated protocol implementations and was only looking
at the project's direct dependencies.

These changes allow it to find the protocols and also will namespace all of the project's dependencies, which I assume will be required. I believe
that Lexical does further namespacing as part of its packaging steps, which is probably where it finds the rest of the deps.

Building

The Engine project has a mix alias of "build" which compiles the project and performs the namespacing. This is the task that the release step executes.

In order to use the engine locally, we set an environment variable before starting the application to the location of the engine's beam files.

This is captured as a bash script and can be easily run as bin/start --port 9000 to start it in TCP mode on port 9000

#!/usr/bin/env bash

cd "$(dirname "$0")"/.. || exit 1

(
  cd ../engine || exit

  mix build
)

EXPERT_ENGINE_PATH="../engine/_build/${MIX_ENV:-dev}/" mix run --no-halt -e "Application.ensure_all_started(:expert)" -- "$@"

Note

TCP mode is the way I traditionally test Next LS will in local development. Since it doesn't utilize stdio, you are free to throw in print statements wherever you want or even start it with iex. In my neovim config, I conditionally choose TCP or stdio via an environment variable, so I just start my editor like NEXTLS_LOCAL=1 nvim and it boots up nicely. Even printing from the engine will output in the terminal you started ran the start script.

When we boot up the runtime, we get the engine path from this environment variable, or from the applications priv dir. We then create a -pa entry for each directory using a wildcard.

Here is an abbreviated snippet that demonstrates what is being done

engine_path =
  System.get_env("EXPERT_ENGINE_PATH", to_string(dir)) |> Path.expand()

consolidated =
  Path.wildcard(Path.join(engine_path, "lib/*/{consolidated}"))
  |> Enum.flat_map(fn ep -> ["-pa", ep] end)

rest =
  Path.wildcard(Path.join(engine_path, "lib/*/{ebin}"))
  |> Enum.flat_map(fn ep -> ["-pa", ep] end)

engine_path_args = rest ++ consolidated

args =
  [elixir_exe] ++ engine_path_args ++ [
      "--no-halt",
      "--sname",
      sname,
      "--cookie",
      Node.get_cookie(),
      "-S",
      "mix",
      "loadpaths",
      "--no-compile"
    ]

port =
  Port.open(
    {:spawn_executable, exe},
    [
      :use_stdio,
      :stderr_to_stdout,
      :binary,
      :stream,
      cd: working_dir,
      env: env,
      args: args
    ]
  )

Demos

To help prove out the ideas, I got some diagnostics and document symbols going. Both implementations are just hacked up versions copy pasted from Next LS and are mostly for demonstration, not to vie for inclusion in this form.

The document symbols is the real demo, as it showcases using Spitfire, GenLSP, and Schematic (dependencies of Engine) inside the host application.

CleanShot.2024-09-29.at.19.03.52.mp4

Note

At the end of the video, if you pause when I open up the :LspInfo screen, you'll notice the path to Next LS is actually the path to the burrito wrapped expert executable

Observations

This all mostly worked first try, but with two oddities.

I believe (please correct me if I am wrong), that Lexical namespaces all the code that is included in the remote control app and shared between remote control and the main LSP node.

I opted to only namespace the Engine code, as to reduce complexity surface area of the namespacing, and in doing so, when the document symbols RPC made to the engine returned, the GenLSP
structs were (obviously in hindsight), namespaced structs (XPGenLSP).

Fortunately, since all GenLSP structures are described by Schematic, I could easily serialize them into plain maps in the Engine, and then deserialize them back into structs in Expert.

This is slightly goofy, as they go from structs -(engine)-> maps -(expert)-> structs -(expert, to the editor)-> maps.

The other oddity actually came before I got this working. I believe this quirk is actually known, as I have a vague memory of Steve describing an issue with the :patch library and namespacing.

I believe that I ran into the same problem with Schematic and GenLSP. GenLSP uses the convention of having a schematic/0 function on the module to create the schematic. Some modules (like document symbols) actually are recursive types (the
:children field on the struct is a list of document symbols, so they use a lazy_schematic, which is basically just an MFA tuple.

This MFA tuple is in the code as apply(__MODULE__, :schematic, []). During namespacing, that tuple gets turned into :xp_schematic, which when run during runtime explodes with an undefined function error.

I think this is due to usage of apply/3, with this note from the docs "Failure: error_handler:undefined_function/3 is called if the applied function is not exported." For some reason the statically defined functions get namespaced as well,
but end up still working at runtime. But if you boot up iex and try, it fails. If you were to run DocumentSymbols.schematic(), it works.

I worked around this by updating GenLSP to name that function schema instead. After I made that change (pushed up to a branch), everything works well.

Usage

To test the project locally, clone it and from the root you can run the following to boot it into TCP mode and connect.

just deps expert
just deps engine
just start

To build a local exe and run it in stdio mode, running the following will build you an exe for your OS and arch in the burrito_out directory.

just build-local

"Just start" it you say! I'm not entirely sure how you would start it up with Emacs or VSCode just to test it, but with Neovim you can just pretend that its Next LS and set the cmd setting with elixir-tools.nvim

{
    "elixir-tools/elixir-tools.nvim",
    version = "*",
    dev = true,
    event = { "BufReadPre", "BufNewFile" },
    config = function()
      local elixir = require("elixir")
      local nextls_opts
      if vim.env.NEXTLS_LOCAL == "1" then
        nextls_opts = {
          enable = true,
          port = 9000,
          spitfire = false,
          init_options = {
            experimental = {
              completions = {
                enable = true,
              },
            },
          },
        }
      else
        nextls_opts = {
          enable = true,
          cmd = "/home/mitchell/src/expert/expert/burrito_out/expert_linux_amd64",
          spitfire = false,
          init_options = {
            experimental = {
              completions = {
                enable = true,
              },
            },
          },
        }
      end

      elixir.setup {
        nextls = nextls_opts,
        credo = { enable = false },
        elixirls = { enable = false },
      }
    end,
    dependencies = {
      "nvim-lua/plenary.nvim",
      "mhanberg/workspace-folders.nvim",
    },
},

Next Steps

I believe that this proves out we can pretty easily bundle the Lexical style injectable application with Burrito.

I think the next step will be productionize the meat of the PR and to add in the basic core functionality of the Lexical remote control to the Engine.

I'm not super familiar with the Remote Control application yet, but I imagine this is like the processes/worker that start inside the project node and get ready to receive requests or RPCs from the server. From there we can get it compiling code and from there we can start baking in actual features.

I can't remember where the Lexical document store code lives (the server or the remote control), but we can probably start baking that in as well.

TODOS

• still figure out the utf 16 stuff, aka does it belong in genlsp (can probably punt)
• figure out how we want to do testing. NextLS's integration tests use GenLSPs test sdk
• probably tons of more stuff

[mhanberg]

Forgot to mention that the "just" commands use a command runner called "just" and are defined the the "justfile"

You can install it via "brew install just" or find the method for your OS https://github.com/casey/just?tab=readme-ov-file#packages

[jonatanklosko]

Just to check I understand correctly, we are spawning the runtime node using user local Elixir/OTP and we load the engine modules compiled with expert-defined Elixir/OTP (same as bundled with burrito)? It could be an issue because the .beam source may not be compatible across versions (they bytecode is more stable across OTP versions, but the underlying Elixir source, not necessarily).

[mhanberg]

In practice we would compile these with an older version of elixir/OTP than the main server so they'll be compatible with the users versions.

Or compile a matrix of them and use the exact-ish versions the user is using.

[jonatanklosko]

Got it, makes sense :) For OTP an older version should be fine, for Elixir we may need several versions (if my understanding is correct that the Elixir source internals may not be backward compatible). cc @josevalim

[lukaszsamson]

There were some (rather unintended) breaking changes around struct fields making beams compiled on v1.x not work on v1.x+1. That's why with ElixirLS I went with building the server exactly for project OTP/elixir combo

[mhanberg]

would require more machinery and problem surface area but we could also bundle the source code and build it on the users machine and cache the beam files per release+versions

[jonatanklosko]

If we load consolidated protocol modules from the engine, and the user code has its own implementations for the same module, wouldn't we fail to resolve the user implementations, since the consolidated module doesn't know about it?

[mhanberg]

Shouldn't matter in practice, depending on how we implement "implementations" for protocols.

Meaning, if there are similarly named modules in our consolidated folder, its functions will be loaded and the protocol interface is static so when finding code and functions that are loaded, they'd. They'd be the same as the users.

[jonatanklosko]

Let's consider an example, say we have Engine.Foo implementing the Inspect protocol. So we have the implementation module Elixir.Inspect.Engine.Foo.beam, and the consolidated Elixir.Inspect.beam module.

Now, the user defines User.Bar and implements the Inspect protocol, so we have Elixir.Inspect.User.Bar.beam and we skip consolidation when compiling user code.

With that, the Elixir.Inspect.beam we load is consolidated, but it is only aware of Elixir.Inspec.Engine.Foo implementation, not Elixir.Inspect.User.Bar.

[lukaszsamson]

I also had that issue in ElixirLS. I needed to disable consolidation in the server to workaround it. @scohen how did you solve it in Lexical?

[jonatanklosko]

I think disabling consolidation for both user code and the injected code is justified here. We have two sources of implementations, so no single place to consolidate. But happy to hear a smarter solution :)

[lukaszsamson]

The scenario I had in mind was this:

• we are running the project node with stdlib protocols already consolidated
• there's an implementation of a stdlib protocol like Inspect for some project struct Foo and it is used in a macro/custom mix compile task/module callback like @after_compile

I need to check if this is a valid and supported scenario. If it is, we need to make sure that

1. The mix compile does not warn with the #{inspect(protocol)} protocol has already been consolidated. It's a warning only and can be disabled with :ignore_already_consolidated compiler option.
2. Make sure the protocol implementation is actually available

Note mix compile --force will reconsolidate protocols witch will contaminate our consolidation path with project implementations

[lukaszsamson]

If you're running in dev mode, how do you handle the test directory?

In elixir-ls it behaves like ordinary mix compile - if you run in dev then test paths are not compiled

[mhanberg]

Next LS only supports a subset of features in test files, mostly doesn't support function/module definition/references and diagnostics.

[scohen]

I see, lexical does index test files (this powers go to definition/ references) and uses the per file compiler on exs files while you’re editing them. Running in the test env by default allows it to compile code in test/support if that’s added as an option, but there are issues that this causes. Maybe this is a bad approach

[scohen]

It would be cool if the Code compiler had an option that produces diagnostics

[scohen]

I believe (please correct me if I am wrong), that Lexical namespaces all the code that is included in the remote control app and shared between remote control and the main LSP node.

Lexical actually namespaces all the code that's built, including erlang artifacts like applications, etc. It does this even for deps. It is not just confined to remote_control, which is important when you're working on the language server and its common modules.

I opted to only namespace the Engine code, as to reduce complexity surface area of the namespacing, and in doing so, when the document symbols RPC made to the engine returned, the GenLSP structs were (obviously in hindsight), namespaced structs (XPGenLSP).

When lexical runs, it actively removes any namespaced modules from things like document symbols, completions, etc, so if you're not doing this, you'll see those modules and structs in the output.

[mhanberg]

Lexical actually namespaces all the code that's built, including erlang artifacts like applications, etc. It does this even for deps. It is not just confined to remote_control, which is important when you're working on the language server and its common modules.

Sorry, that is what I was trying to say. It namespaces the lexical server as well as the remote control app (with deps as well, that was implied)

When lexical runs, it actively removes any namespaced modules from things like document symbols, completions, etc, so if you're not doing this, you'll see those modules and structs in the output.

To clarify, that is not what I am saying. I am saying that the data structure that the engine creates itself (not the symbols it was gathering) was namespaced, which was still namespaced when sent back to the expert server.

it actively removes any namespaced modules from things like document symbols, completions, etc

Next LS does this as well, but is unrelated to my comments.

[scohen]

To clarify, that is not what I am saying. I am saying that the data structure that the engine creates itself (not the symbols it was gathering) was namespaced, which was still namespaced when sent back to the expert server.

Given the first point, this is expected, no? I think everything in the codebase will (and should) have namespacing applied to it. I think we want all the modules in the project to have namespacing applied, whether they're in the engine or the project node.

[mhanberg]

Given the first point, this is expected, no?

My original comment wasn't to say that "whoa what is happening", I had commented it was obvious in hindsight. Also I'm not sure which comment you are referring to when you say "first point".

I think everything in the codebase will (and should) have namespacing applied to it. I think we want all the modules in the project to have namespacing applied, whether they're in the engine or the project node.

This I'm not entirely convinced is necessary, other than to combat the observation I made. The purpose to my understanding of the namespacing is to ensure our injected code does not interfere with the users code. But in the server (the Expert mix project) don't get injected and doesn't have that problem.

[mhanberg]

I will spike this PR further to see how namespacing everything looks tho, @scohen

[scohen]

In Lexical, making distinctions between what should and shouldn't be namespaced was fairly fraught, given that both server and the project node use some common modules, so it was easier to namespace everything. It's not strictly necessary, but since namespacing is applied after compilation, common modules would have namespacing applied, and their names changed, and if the server modules didn't have it applied, they wouldn't be able to call the common modules.

I suppose you could have a task that just namespaces calls a module makes without rewriting its name, but i'm too lazy for that distinction.

FYI, you can "undo" namespacing in iex via alias LXical, as: Lexical.

[scohen]

Can we use mix to do this? My preference would be to not introduce additional tooling.

[scohen]

I recently came to the realization (check my work here) that this script is not necessary for what we need to do. All we need to do is have the project node monitor the server node and exit if it detects a :nodedown.

[scohen]

(this method sometimes leaves orphaned beam processes lingering)

[scohen]

This MFA tuple is in the code as apply(MODULE, :schematic, []). During namespacing, that tuple gets turned into :xp_schematic, which when run during runtime explodes with an undefined function error.

Ah yes. This is because namespacing thinks this is an app name. Schematic must live in the :schematic app. Question, does schematic need to define an app?

[scohen]

@mhanberg given this is a spike, how much feedback do you want here?

[mhanberg]

@mhanberg given this is a spike, how much feedback do you want here?

Mostly just on the namespacing. The spike was to prove out combining the namespacing and remote node stuff with burrito.

Once we're cool with that, I think we can spike out integrating the foundations of lexical runtime node code, meaning the parts in the server that starts the runtime node and the actual engine code.

So the gen server that starts the runtime node in this PR is from next ls, but just to have something to test. As well as the code inside the engine project, just threw it in there to test that it was working at all.

How does that sound? Any thoughts?

[scohen]

How does that sound? Any thoughts?

Sounds fantastic!

[mhanberg]

I got it working with namespacing the Expert project as well, which accomplishes the goal of interfacing with with the Engines modules without having to "know" they are namspaced (meaning, :rpc.call(node, Engine.Worker, :enqueue_compiler, [opts]) instead of :rpc.call(node, XPert.Worker, :enqueue_compiler, [opts])

There is still some DX I want to prove out first but i'll get this all pushed up soon.

[mhanberg]

Copy pasting the latest commit message here: cc @scohen @lukaszsamson @jonatanklosko

By namespacing the Expert application as well, we can seamlessly
communicate with the Engine application without having have knowledge of
the namespaced module names.

Doing this naively proved to come with some tradeoffs. The namespacing
as it was before would also namespace the application files. This causes
the application config to get a little hairy. Lexical will also
namespace the config files, but that works because they seem to do this
when "packaging", which copies the files into a release like structure.

I wanted to be able to run the project as "vanilla" as possible, so to
do so, a modification to the namespace mix task allows us to namespace
with or without changing the applications. Namespacing the applications
is not necessary for the Expert codebase, as the namespacing is done to
allow seamless communication with the engine, and not to avoid
collisions with user code.

This works nicely, but since we are namespacing the beam files in place,
when we make a change and recompile, mix recompiles everything from
scratch.

To avoid doing this, tooling in the justfile (just is analogous to make)
will swap the original compilation artifacts in and out with the
namespaced ones so mix will continue to incrementally compile.

It will namespace everything rather than incrementally namespace, but
this happens fast enough that it doesn't seem to be a problem as of
now.

[mhanberg]

There are actually some quirks I'm finding with creating the burrito release introduced by the namespacing, will figure that out.

[scohen]

@mhanberg I want to make sure I understand

Namespacing the applications is not necessary for the Expert codebase, as the namespacing is done to allow seamless communication with the engine, and not to avoid collisions with user code.

Does the above only apply to running expert on the command line without the packaging step or does that include when expert is actually packaged.

If it applies post-packaging, what happens when expert edits itself while it's running its own code in the remote control app, or, or when the authors or contributors to a library that the remote_control app uses as a dependency uses expert?

Namespacing was written with this thought in mind, and if we can keep this functionality, then that's good enough.

[scohen]

There are actually some quirks I'm finding with creating the burrito release introduced by the namespacing,

This was my concern. If you need any help, ping me, I'm back.

[lukaszsamson]

powershell

$env:EXPERT_RELEASE_MODE="burrito"; $env:BURRITO_TARGET="windows_amd64"; $env:MIX_ENV="prod"; mix release

cmd

set EXPERT_RELEASE_MODE=burrito && set BURRITO_TARGET=windows_amd64 && set MIX_ENV=prod && mix release

[lukaszsamson]

windows_arm is not a valid target?

[mhanberg]

Not in practice, I don't believe. I think ARM windows is an insiders release and there are no ARM windows laptops that are for sale.

[lukaszsamson]

Aren't we namespacing erlang modules?

[mhanberg]

Yes, unclear what happened here but its changed in more recent commits

[mhanberg]

Okay, all up and working now. Copying pasting some commit messages here for visibility

spike(namespace): parameterize transformers

This patch primarily parameterizes the transformers to accept the apps
and the root modules they need to work on as parameters instead as
global state.

This increases the flexibility of the project by hoisting this
configuration to the boundary, allowing it to be overridden/customized
by CLI options.

This was accomplished by passing an "opts" parameter to each transformer
function (which were consequently renamed from "apply" to "run" to avoid
a naming collision with Kernel.apply/2).

Incidental changes are moving the transformers from the mix task
directory to the regular lib directory and to add unit tests to ensure
that there weren't any regressions.

Something to note, I don't think that the `configs`, `scripts`, or
`boots` transformers are needed any longer, but that conclusion should
be double checked.

You can see this taking effect in the justfile

build-expert: (build "expert" "--include-app expert --exclude-root Expert --exclude-app burrito --exclude-app req --exclude-app finch --exclude-app nimble_options --exclude-app nimble_pool --exclude-app namespace --exclude-root Jason --include-root Engine")

I believe this could be further improved (something like --only-app gen_lsp instead of all the include and excludes), but decided since it works and this is a spike, we can improve it later.

The justfile now has documentation for each of the public recipes (each task is a called a recipe), and you can see them all with just --list.

Further, I have added other dependencies to the .tool-versions file, like zig and just, so you can install them that way, but if you use Nix, you can simply enter the nix dev shell nix develop and it'll install them all for you there.

I have added a README to the root of the project that has some basic getting started stuff that should help further understand how to get started.

[mhanberg]

@mhanberg I want to make sure I understand

Namespacing the applications is not necessary for the Expert codebase, as the namespacing is done to allow seamless communication with the engine, and not to avoid collisions with user code.

Does the above only apply to running expert on the command line without the packaging step or does that include when expert is actually packaged.

If it applies post-packaging, what happens when expert edits itself while it's running its own code in the remote control app, or, or when the authors or contributors to a library that the remote_control app uses as a dependency uses expert?

Namespacing was written with this thought in mind, and if we can keep this functionality, then that's good enough.

Yes, everything should work as expected.

The engine codebase is the only code that is being injected into the runtime, so it is fully namespaced. Th expert codebase is namespaced to prefix calls to the Engine root module, so it can do RPC with it without needing to know the namespacing has happened.

[scohen]

The engine codebase is the only code that is being injected into the runtime

Is this a temporary condition, or is this how you see things working going forward?

[mhanberg]

The engine codebase is the only code that is being injected into the runtime

Is this a temporary condition, or is this how you see things working going forward?

That's how I expected we'd move forward.

Is there a concern with this approach? I was under the assumption this is how lexical works.

[scohen]

Is there a concern with this approach? I was under the assumption this is how lexical works.

Maybe i'm misunderstanding, is Engine equivalent to remote_control? If so yes, that's right, but the remote_control lib depends on other, common libraries that are useful across all the projects (for example in the server). I think your approach allows namespacing to occur on the application level, so maybe this isn't a problem.

[scohen]

FYI, the common lib defines things like the document store, and the lexical-specific concepts of ranges, positions and documents that allow for automatic conversion to LSP constructs without having to do math on the various fields.

[mhanberg]

Roger, yes Engine is the equivalent of remote control, I should have said that more explicitly.

This should be fine then, the common libraries will just be projects in the monorepo and both the Expert and Engine projects can declare them as dependencies and should all just work.

I'll actually test that out just to be sure, now is the time haha.

[scohen]

Fantastic, In that case, we should also be namespacing Engine's dependencies and their dependencies so that those projects can use Expert. I think that's possible, and maybe already taken care of (I apologize for not being more involved, work is hectic and there's a couple lexical issues I want to address before we do the big integration).

[mhanberg]

maybe already taken care of

Yes, the Engine codebase is completely namespaced, meaning every .beam and .app file has namespacing applied to it (except for the namespace code itself, but I'm realizing I need to change that, but that is very simple).

I apologize for not being more involved

No worries, I totally get it!

[mhanberg]

The latest commit adds tests using GenLSPs test functionality.

Copying the commit message here:

spike(tests): add integration tests for expert

This patch introduces integration tests using GenLSPs builtin testing
sdk.

The test sdk starts an in process instance of the server and
communicates with it via the TCP adapter. In tests, the test is the
"client" instead of a text editor.

Tests however are not namespaced. From what I could see, tests in
Lexical don't seem to be namespaced. Attempting to do so I believe would
be a rabbit hole, as you'd have to namespace the in memory compiled
tests before they run.

---

This also adjusts how the project is namespaced in development.
Previously, we would copy the compiled beam files to another directory
and namespace the ones in the standard one, then copy the normal files
back before compiling again to achieve normal incremental compiles.

Now, it leaves the normal files in place, but copies them to another
directory and namespaces them there. Then when we start the server, we
change the MIX_BUILD_PATH to the new directory.

---

One last thing to note is that the :engine atom is not properly
namespaced in the Expert project, as we aren't namespacing applications.
But, it was attempting to start the Engine application via
Application.ensure_all_started(:engine). To get around this, I put a
function in the Engine project that calls that instead, and the Expert
projects RPC call is Engine.ensure_all_started().

[mhanberg]

Closing, but leaving the branch for posterity