-
Notifications
You must be signed in to change notification settings - Fork 368
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
fix: elixir installation failed #4144
base: main
Are you sure you want to change the base?
Conversation
doesn't this effectively make it so we'd always be using perhaps I'm a bit tired but this sentence sounds like the exact opposite of how things should behave:
|
Yes that's ugly but get_dependencies only returns strings (eg "erlang") and with nothing in the config its the only option. Your're right, the current implementation seems correct but does not take into account that dependencies might not be in the config (unless i'm missing something). An Maybe we need something more sophisticated, that tries the config first and falls back to |
it sounds like you're misunderstanding the correct behavior. It should be possible for a user to install erlang with homebrew and elixir with mise without also using mise to manage erlang. |
Ah I see... that complicates things. If The culprit is the following which is passed to the So i guess we need to check if the dependencies are part of the ToolSet (in InstallContext?) and add them, otherwise assume they're installed already (via brew or anyhow)? I'll dig a bit more towards a better solution later today. Lazy me would be tempted to just remove the verify function as erlang also does not have one (which makes sense since version check is a pain) 😂 |
yes, I think you're correct. The issue is that we always load the config from the config files and we're not including something that gets passed through command line arguments. Threading this through might be challenging. |
Played around a bit more with this but it seems a bit tricky and ugly if you ask me. The Passing down the tools from the arguments can be done relatively easily via One use-case that is not taken into account is when |
I've thought a few things that I agree with your assessment, I wonder if we should have 1 global Config that is based on the arguments that we could reuse here but only use it in certain situations, like a That's definitely easier said than done though. Maybe as a stop-gap we could have a global mutex for tool arguments that we could populate when Or perhaps I'm wrong and the install context should be passed around to more places than we currently use it. |
Fixes #4114
The
dependency_toolset
function should not rely on dependencies in the Config to build the environment for the CmdLineRunner, especially when used withinstall
.Since this seems specific to Elixir so far and the method is used in other contexts, its probably best to keep the solution exclusive to elixir. If necessary we can move it up to the trait later. WDYT?