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

Ensure the remote control node and server use the same Elixir executable #795

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zachallaun
Copy link
Collaborator

This PR closes #776.

Prior, we perform version manager detection twice: once in the shell when starting the Lexical server, and once in Elixir when starting the remote control node. This PR deduplicates this logic by storing the Elixir executable path used to start the server in the application environment, which is then added to the project as an attribute to be used when the remote control node starts.

@zachallaun zachallaun force-pushed the za-deduplicate-vm-detection branch 4 times, most recently from 52a7640 to 2ba3ce3 Compare July 19, 2024 17:03
@zachallaun
Copy link
Collaborator Author

I'm not 100% sure that storing the :elixir_executable in %Project{} is the right way to go. An alternative might be to use :lexical instead of :server as the config app, and then change RemoteControl.Port to do something like:

exe = Application.get_env(:lexical, :elixir_executable, System.find_executable("elixir"))

instead of using project.elixir_executable.

@scohen
Copy link
Collaborator

scohen commented Jul 28, 2024

An alternative might be to use :lexical instead of :server as the config app,

There is no :lexical app though, I don't think this will be allowed.

@zachallaun
Copy link
Collaborator Author

An alternative might be to use :lexical instead of :server as the config app,

There is no :lexical app though, I don't think this will be allowed.

That settles that. The existing direction probably makes the most sense then. I really dislike the idea of something in the remote control app accessing the server app config.

We could set it in the remote control app config, but that also doesn't feel awesome.

Erlang is now a core plugin, so it doesn't need to (and can't) be installed separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants