Supersonido / rebar_mix

rebar3 plugin for building Elixir dependencies with mix
Apache License 2.0
53 stars 20 forks source link

Plugin fails if Elixir lib dir contains spaces. #26

Open ElectronicRU opened 3 years ago

ElectronicRU commented 3 years ago

The detection uses IO.puts and removes spaces, which fails if e.g. under default Windows parameters, Elixir is installed under C:\Program Files. Using IO.write and not removing spaces would be the fix.

tsloughter commented 3 years ago

Thanks. Pls send a PR if you have time.

mark-04 commented 2 years ago

I had the same issue Here in rebar_mix/src/rebar_mix_utils.erl, in the function get_lib_dir (line 42-53) we have the following case expression

case rebar_utils:sh("elixir -e \"IO.puts :code.lib_dir(:elixir)\"", [return_on_error]) of
    {ok, ElixirLibs_} ->
        filename:join(re:replace(ElixirLibs_, "\\s+", "", [global,{return,list}]), "../");
    _ ->
        erlang:error(missing_elixir)
end;

The insteresting part is re:replace(ElixirLibs_, "\\s+", "", [global,{return,list}]) It basically takes the path to Elixir and removes all whitespaces from it. And so if our path happens to start with, for instance, C:\Program Files (default when installing Elixir on windows), it becomes C:\ProgramFiles, resulting in an error. I spent quite some time trying to figure out what the problem was, so I thought it is worth mentioning it

I'm not sure if it is a bug. I assume there is a valid reason why we would remove whitespaces from the path, and so I don't know if we should do something about it or keep it as is. I do think that it should be mentioned in the README, though. (e.g. in the requiremets section, we might include something like "Make sure that the path to your Elixir installation contains no significant whitespaces". I beleive, that could save a lot of debagging time for the future users).

ferd commented 2 years ago

No there's no good reason to remove whitespace from a path, it just makes it wrong. The proper fix is to escape and/or quote the path adequately.