search

erlware / relx

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

Refactor creation of .erlang.cookie for use in start/console procedures #690

Closed tolbrino closed 5 years ago

tolbrino commented 5 years ago

Previously the all relx commands would try to create that file as part of the internal search for an ERTS path. This could lead to hard crashes as described in #696 if for some reason the Erlang node can't write that file. Therefore, this change moves the creation of the .erlang.cookie file just before executing the commands start or console, where the file actually matters. Moreover, the creation is optional and won't stop the procedure, just log an error if it occurred.

Fixes #696

tsloughter commented 5 years ago

This breaking the tests somehow.

tsloughter commented 5 years ago

@tolbrino ping

tolbrino commented 5 years ago

I will look at this next week.

On March 13, 2019 4:58:46 PM UTC, Tristan Sloughter notifications@github.com wrote:

@tolbrino ping

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/erlware/relx/pull/690#issuecomment-472512645

tsloughter commented 5 years ago

@lrascao can you help take a look at these failing tests?

lrascao commented 5 years ago

they all appear to fail due to the same thing:

{{badmatch,{ok,"v1"}}, [{rlx_extended_bin_SUITE,replace_os_vars_coretest,8, [{file,"relx/_build/test/lib/relx/test/rlx_extended_bin_SUITE.erl"}, {line,1484}]}

something must've changed in the refactor that dropped the extra quotes, this might only be a matter of adapting to the new result if the change was intended, @tolbrino?

{ok, "\"v1\""} = sh(Bin ++ " eval '{ok, V} = application:get_env(goal_app, var1), V.'", Vars1)
tsloughter commented 5 years ago

@tolbrino are all the changes this is making to the test suite really necessary? I don't see any logic changing but the tests fail so made me think the PR has more than it needs?

Just hoping to get a release of rebar3/relx soon with the main fix here.

tolbrino commented 5 years ago

@tsloughter I removed the test changes and will put them into a different PR.

tsloughter commented 5 years ago

Thanks!

tsloughter commented 5 years ago

@tolbrino I just tried this in a docker container and it is still crashing when not able to create the cookie file.

tsloughter commented 5 years ago

@tolbrino actually, no, this one is due to relx_get_nodename().