search

erlware / relx

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

MR 642 broke escripts args #648

Open juise opened 6 years ago

juise commented 6 years ago

Hi!

The @konyaris in his MR #642 broke the header with erlang vm.args in nodetool:

the result looks like: %%!+K true +A32 +zdbbl 8192 +W w -connect_all false -hidden -proto_dist inet_tls -ssl_dist_opt server_cacertfile /opt/abc/spool/secure/cluster/cert.pem -ssl_dist_opt server_certfile /opt/abc/spool/secure/node/cert.pem -ssl_dist_opt server_keyfile /opt/abc/spool/secure/node/key.pem -ssl_dist_opt client_cacertfile /opt/abc/spool/secure/cluster/cert.pem -ssl_dist_opt client_certfile /opt/abc/spool/secure/node/cert.pem -ssl_dist_opt client_keyfile /opt/abc/spool/secure/node/key.pem -rsh ssh -env ERL_LIBS /opt/abc -env ERL_CRASH_DUMP /opt/abc/erl_crash.dump -env ABC_ROOT /opt/abc -env ABC_SPOOL /opt/abc/spool -env ABC_SNAME ghead -env ABC_PARENT_SNAME ghead -env ABC_API_PORT 10001+K true +A32 +zdbbl 8192 +W w -connect_all false -hidden -proto_dist inet_tls -ssl_dist_opt server_cacertfile /opt/abc/spool/secure/cluster/cert.pem -ssl_dist_opt server_certfile /opt/abc/spool/secure/node/cert.pem -ssl_dist_opt server_keyfile /opt/abc/spool/secure/node/key.pem -ssl_dist_opt client_cacertfile /opt/abc/spool/secure/cluster/cert.pem -ssl_dist_opt client_certfile /opt/abc/spool/secure/node/cert.pem -ssl_dist_opt client_keyfile /opt/abc/spool/secure/node/key.pem -rsh ssh -env ERL_LIBS /opt/abc -env ERL_CRASH_DUMP /opt/abc/erl_crash.dump -env ABC_ROOT /opt/abc -env ABC_SPOOL /opt/abc/spool -env ABC_SNAME chead1 -env ABC_PARENT_SNAME ghead -env ABC_API_PORT 10001

Right now the header duplicated, so all node communications are broken.

@konyaris could you please fix this behaviour? Or @lrascao could you please revert this PR in case @konyaris don't fix it?

lrascao commented 6 years ago

@juise , thanks for the heads up, could you please check whats missing from the test proposed in #643 that would have caught this?

juise commented 6 years ago

@lrascao I guess, we should make some fixes in https://github.com/erlware/relx/pull/640/files#diff-9fdfcb9e5a6367b624fded64ddce559bR1493 not in #643. I don't know why my test don't caught this misbehavirour. I need some time to find out, which arg on duplication in header broke behaviour

lrascao commented 6 years ago

ok, any help finding the regression and plugging it would be greatly appreciated

konyaris commented 6 years ago

I think problem occurred because VM_ARGS changed between two runs. ABC_SNAME ghead changed to ABC_SNAME chead1. Is it valid to change VM_ARGS between two run of same nodetool?

juise commented 6 years ago

@konyaris yeah, we configure vm.args and sys.config trought environment variables, relx support it. So it's valid use case.

konyaris commented 6 years ago

I have created a pull request: https://github.com/erlware/relx/pull/649