erlware / relx

Sane, simple release creation for Erlang
http://erlware.github.io/relx
Apache License 2.0
697 stars 232 forks source link

Warning from systools:make_relup/4 crashes relx #554

Closed sirihansen closed 7 years ago

sirihansen commented 7 years ago

If systools:make_relup/4 returns {ok, Relup, Module, Warnings}, where Warnings=/=[], then relx crashes with function_clause in rlx_prv_relup:format_error/1, since it does not handle the tag relup_script_generation_warn.

Apart from the bug, what is the reasoning behind failing the relup generation due to warnings? Could it be considered to allow the warnings_as_errors option here, and only fail if this is set?

lrascao commented 7 years ago

Thanks for the report, i'll put in a PR to fix it asap.

Apart from the bug, what is the reasoning behind failing the relup generation due to warnings? Could it be considered to allow the warnings_as_errors option here, and only fail if this is set?

Isn't the warnings_as_errors option a erlc one? i suppose we could use in the context of release generation as well, another alternative would be not to error out in this case and report a successful release with the warnings exposed.

sirihansen commented 7 years ago

Yes, the warnings_as_errors option is actually an erlc option, but since erlc calls systools for certain files, systools (e.g. systools:make_relup/4) also obeys this option. I think it might be used in this situation.

lrascao commented 7 years ago

@tsloughter @ferd is there a way for relx to obtain rebar3 options residing in the rebar.config or do they have to be explicitly passed?

lrascao commented 7 years ago

@sirihansen #559 addresses the issue you've raised, with it relx is formatting only the erts_vsn_changed warning and just directly printing out any others, do you think the message is clear enough for these cases?

sirihansen commented 7 years ago

I think you might want to call Module:format_warning/1 (in the same way as relup_script_generation_error causes call to Module:format_error/1) - see the description of this under http://erlang.org/doc/man/systools.html#make_relup-4.

lrascao commented 7 years ago

@sirihansen #559 has been updated

lrascao commented 7 years ago

closing as #559 has since been merged