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

Add "consolidated" to path in erl helper #680

Merged
merged 1 commit into from Jun 25, 2019

Conversation

hrubi
Copy link
Contributor

@hrubi hrubi commented Jun 14, 2019

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.

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
Copy link
Owner

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

@bitwalker bitwalker closed this Jun 25, 2019
@hrubi
Copy link
Contributor Author

hrubi commented Jun 25, 2019

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:

-pa "${CONSOLIDATED_DIR}" \

@bitwalker
Copy link
Owner

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!

@bitwalker bitwalker reopened this Jun 25, 2019
@bitwalker bitwalker merged commit 706e49f into bitwalker:master Jun 25, 2019
@hrubi hrubi deleted the consolidated branch June 25, 2019 19:02
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.

None yet

2 participants