bitwalker / distillery

Simplify deployments in Elixir with OTP releases!
MIT License
2.96k stars 396 forks source link

Add "consolidated" to path in `erl` helper #680

Closed hrubi closed 5 years ago

hrubi commented 5 years ago

It's on path in all other conditional branches, so there's no reason it's not set in the case of bundled erts and the default boot file.

bitwalker commented 5 years ago

It isn't needed here because of what it means to use the start_clean boot script, namely that we aren't loading any applications that would contain consolidated protocols, it consists only of the core Erlang and Elixir applications, e.g. kernel, stdlib, compiler, elixir, and iex

hrubi commented 5 years ago

All the modules are loaded with start_clean, so one can use any module e.g. with the eval management command and the evaluated expression or script can make use of the consolidated protocols. So I still think it makes sense.

Before this change:

% _build/prod/rel/myapp/bin/myapp eval 'IO.inspect(Protocol.consolidated?(String.Chars))'
false

After this change:

% _build/prod/rel/myapp/bin/myapp eval 'IO.inspect(Protocol.consolidated?(String.Chars))'
true

Also the path to consolidated protocols is already added in the case of host erts and default start_clean bootfile: https://github.com/bitwalker/distillery/blob/ce5b3298367e59d2017090b69308b4745fff2369/priv/libexec/erts.sh#L102

bitwalker commented 5 years ago

True, we did change the behavior of start_clean awhile back so that all modules are loaded. I guess it doesn't really cost anything to add it, and is important for eval, so I'll go ahead and merge. Thanks for following up!