Correctly construct HEART_COMMAND and run_erl arguments #617
Conversation
The runner script that ships with rebar builds HEART_COMMAND and run_erl arguments that preserve additional arguments the user may have passed: https://github.com/rebar/rebar/blob/master/priv/templates/simplenode.runner#L215-L238 This PR preserves this behavior. In additon, the current code on this line sets $@ but does not do anything with the result: https://github.com/erlware/relx/blob/master/priv/templates/extended_bin#L481-L482 Investigated in response to this ML thread: http://erlang.org/pipermail/erlang-questions/2017-October/093974.html
|
I originally reported the issue. I have confirmed the fix implemented by this pull request. I cloned the rebar3 repo, checked out tag 3.3.6, my current version. Then I modified the rebar.config to use the relx branch in this pull request and built rebar3 escript. The rebar.config change was: % {relx, "3.23.1"}, Next I built my project using the released rebar3, version 3.3.6. Running my application specifying a -config file resulted in the app starting but with the sys.config from the project. Shown here in the ps output: geibs-MacBook-Pro:cse-cmd-processing geib$ _build/default/rel/cse-cmd-processing/bin/cse-cmd-processing start -config=/tmp/my_config Then building release again, with relx fix included and re-running I get: geibs-MacBook-Pro:cse-cmd-processing geib$ _build/default/rel/cse-cmd-processing/bin/cse-cmd-processing start -config=/tmp/my_config |
|
Could you guys check if the smaller change i pushed gets the job done as well? |
|
Hi @lrascao - Part of my change is to address these lines, which use Your set of changes also doesn't quote the contents of In my opinion, my original PR results in code that is easier to understand and is more in-line with the runner script that |
|
agreed, i'll try and whip up a test that ensures no future regressions when i get a chance |
|
@lrascao - I didn't notice there is a test suite. Have you started work? If not I will get a test added. I try to never submit a PR without a test 😄 |
|
i have not, that would be much appreciated, the suite is |
…ins both double quotes and a space character
| @@ -642,28 +623,26 @@ case "$1" in | |||
| export EMU | |||
| export PROGNAME | |||
|
|
|||
| # Store passed arguments since they will be erased by `set` | |||
| ARGS="$@" | |||
There was a problem hiding this comment.
FYI, there is no way to preserve double-quotes and space characters and re-assign the positional parameter variable and correctly pass arguments to erlexec. Believe me, I tried. The side-effect of this is that the erlexec command must be included twice below - once for logging, and once "for real".
|
@lrascao - all set. I added a test that demonstrates that the |
|
the test seems to be failing on Linux, passes on Mac though |
|
Hmm I'll look into it. All of the tests run fine in my environment - up-to-date Arch Linux. I'll try it out on Ubuntu 14. |
|
@lrascao it's good that you test on Ubuntu, since |
|
cool! thanks a bunch! |
|
Awesome. Now I need to fix the issue in In a format string, Using Thanks again 😄 |
|
👍 |
The runner script that ships with
rebarbuildsHEART_COMMANDandrun_erlarguments in such a way to preserve additional arguments the user may have passed:https://github.com/rebar/rebar/blob/master/priv/templates/simplenode.runner#L215-L238
This PR preserves this behavior.
In addition, the current code on this line sets
$@but does not do anything with the result:https://github.com/erlware/relx/blob/master/priv/templates/extended_bin#L481-L482
Investigated in response to this ML thread: http://erlang.org/pipermail/erlang-questions/2017-October/093974.html