From 6e149058b5b92a2f9134c625fce0849a8af15c1f Mon Sep 17 00:00:00 2001 From: Luke Bakken Date: Wed, 25 Oct 2017 14:31:34 -0700 Subject: Correctly construct HEART_COMMAND and run_erl arguments 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 --- priv/templates/extended_bin | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/priv/templates/extended_bin b/priv/templates/extended_bin index 87128b7..8bd9726 100755 --- a/priv/templates/extended_bin +++ b/priv/templates/extended_bin @@ -191,12 +191,6 @@ relx_escript() { "$ERTS_DIR/bin/escript" "$ROOTDIR/$scriptpath" $@ } -# Output a start command for the last argument of run_erl -relx_start_command() { - printf "exec \"%s\" \"%s\"" "$RELEASE_ROOT_DIR/bin/$REL_NAME" \ - "$START_OPTION" -} - relx_get_code_paths() { code="{ok, [{release,_,_,Apps}]} = file:consult(\"$REL_DIR/$REL_NAME.rel\"),"\ "lists:foreach(fun(A) ->"\ @@ -454,15 +448,6 @@ cd "$ROOTDIR" # Check the first argument for instructions case "$1" in start|start_boot) - - # Make sure there is not already a node running - #RES=`$NODETOOL ping` - #if [ "$RES" = "pong" ]; then - # echo "Node is already running!" - # exit 1 - #fi - # Save this for later. - CMD=$1 case "$1" in start) shift @@ -475,14 +460,10 @@ case "$1" in HEART_OPTION="start_boot" ;; esac - RUN_PARAM="$@" - - # Set arguments for the heart command - set -- "$SCRIPT_DIR/$REL_NAME" "$HEART_OPTION" - [ "$RUN_PARAM" ] && set -- "$@" "$RUN_PARAM" + RUN_PARAM="$(printf "\'%s\' " "$@")" # Export the HEART_COMMAND - HEART_COMMAND="$RELEASE_ROOT_DIR/bin/$REL_NAME $CMD" + HEART_COMMAND="$RELEASE_ROOT_DIR/bin/$REL_NAME $HEART_OPTION $RUN_PARAM" export HEART_COMMAND test -z "$PIPE_BASE_DIR" || mkdir -m 1777 -p "$PIPE_BASE_DIR" @@ -490,7 +471,7 @@ case "$1" in relx_run_hooks "$PRE_START_HOOKS" "$BINDIR/run_erl" -daemon "$PIPE_DIR" "$RUNNER_LOG_DIR" \ - "$(relx_start_command)" + "exec $RELEASE_ROOT_DIR/bin/$REL_NAME $START_OPTION $RUN_PARAM" relx_run_hooks "$POST_START_HOOKS" ;; -- cgit v1.2.3 From 19ae6a4c98a131aab84ed9922c307ab3f0eebec7 Mon Sep 17 00:00:00 2001 From: Luke Bakken Date: Mon, 30 Oct 2017 10:44:27 -0700 Subject: Add test that demonstrates that fixes preserve an argument that contains both double quotes and a space character --- priv/templates/extended_bin | 26 +++++++++++------------ test/rlx_extended_bin_SUITE.erl | 47 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/priv/templates/extended_bin b/priv/templates/extended_bin index 8bd9726..0482359 100755 --- a/priv/templates/extended_bin +++ b/priv/templates/extended_bin @@ -460,10 +460,10 @@ case "$1" in HEART_OPTION="start_boot" ;; esac - RUN_PARAM="$(printf "\'%s\' " "$@")" + ARGS="$(printf "\'%s\' " "$@")" # Export the HEART_COMMAND - HEART_COMMAND="$RELEASE_ROOT_DIR/bin/$REL_NAME $HEART_OPTION $RUN_PARAM" + HEART_COMMAND="\"$RELEASE_ROOT_DIR/bin/$REL_NAME\" \"$HEART_OPTION\" $ARGS" export HEART_COMMAND test -z "$PIPE_BASE_DIR" || mkdir -m 1777 -p "$PIPE_BASE_DIR" @@ -471,7 +471,7 @@ case "$1" in relx_run_hooks "$PRE_START_HOOKS" "$BINDIR/run_erl" -daemon "$PIPE_DIR" "$RUNNER_LOG_DIR" \ - "exec $RELEASE_ROOT_DIR/bin/$REL_NAME $START_OPTION $RUN_PARAM" + "exec \"$RELEASE_ROOT_DIR/bin/$REL_NAME\" \"$START_OPTION\" $ARGS" relx_run_hooks "$POST_START_HOOKS" ;; @@ -623,20 +623,13 @@ case "$1" in export EMU export PROGNAME - # Store passed arguments since they will be erased by `set` - ARGS="$@" - - # Build an array of arguments to pass to exec later on - # Build it here because this command will be used for logging. - set -- "$BINDIR/erlexec" $FOREGROUNDOPTIONS \ + # Dump environment info for logging purposes + echo "Exec: $BINDIR/erlexec" $FOREGROUNDOPTIONS \ -boot "$BOOTFILE" -mode "$CODE_LOADING_MODE" \ -boot_var ERTS_LIB_DIR "$ERTS_LIB_DIR" \ -config "$RELX_CONFIG_PATH" \ -args_file "$VMARGS_PATH" \ - -pa ${__code_paths} - - # Dump environment info for logging purposes - echo "Exec: $@" -- ${1+$ARGS} + -pa ${__code_paths} -- "$@" echo "Root: $ROOTDIR" # Log the startup @@ -644,7 +637,12 @@ case "$1" in logger -t "$REL_NAME[$$]" "Starting up" # Start the VM - exec "$@" -- ${1+$ARGS} + exec "$BINDIR/erlexec" $FOREGROUNDOPTIONS \ + -boot "$BOOTFILE" -mode "$CODE_LOADING_MODE" \ + -boot_var ERTS_LIB_DIR "$ERTS_LIB_DIR" \ + -config "$RELX_CONFIG_PATH" \ + -args_file "$VMARGS_PATH" \ + -pa ${__code_paths} -- "$@" ;; rpc) # Make sure a node IS running diff --git a/test/rlx_extended_bin_SUITE.erl b/test/rlx_extended_bin_SUITE.erl index 4407cfc..fb95bab 100644 --- a/test/rlx_extended_bin_SUITE.erl +++ b/test/rlx_extended_bin_SUITE.erl @@ -23,6 +23,7 @@ init_per_testcase/2, all/0, start_sname_in_other_argsfile/1, + start_preserves_arguments/1, start_fail_when_no_name/1, start_fail_when_multiple_names/1, start_fail_when_missing_argsfile/1, @@ -75,7 +76,8 @@ init_per_testcase(_, Config) -> {state, State1} | Config]. all() -> - [start_sname_in_other_argsfile, start_fail_when_no_name, start_fail_when_multiple_names, + [start_sname_in_other_argsfile, start_preserves_arguments, + start_fail_when_no_name, start_fail_when_multiple_names, start_fail_when_missing_argsfile, start_fail_when_nonreadable_argsfile, start_fail_when_relative_argsfile, start_fail_when_circular_argsfiles, ping, shortname_ping, longname_ping, attach, pid, restart, reboot, escript, @@ -1440,6 +1442,49 @@ start_sname_in_other_argsfile(Config) -> %% a ping should fail after stopping a node {error, 1, _} = sh(filename:join([OutputDir, "foo", "bin", "foo ping"])). +start_preserves_arguments(Config) -> + LibDir1 = proplists:get_value(lib1, Config), + + rlx_test_utils:create_app(LibDir1, "goal_app", "0.0.1", [stdlib,kernel], []), + + ConfigFile = filename:join([LibDir1, "relx.config"]), + + rlx_test_utils:write_config(ConfigFile, + [{release, {foo, "0.0.1"}, + [goal_app]}, + {lib_dirs, [filename:join(LibDir1, "*")]}, + {generate_start_script, true}, + {extended_start_script, true} + ]), + + PrivDir = proplists:get_value(priv_dir, Config), + OutputDir = filename:join([PrivDir, rlx_test_utils:create_random_name("relx-output")]), + + {ok, _State} = relx:do([{relname, foo}, + {relvsn, "0.0.1"}, + {goals, []}, + {lib_dirs, [LibDir1]}, + {log_level, 3}, + {output_dir, OutputDir}, + {config, ConfigFile}], ["release"]), + + %% now start/stop the release to make sure the extended script is working + %% and preserving the "tricky" argument that contains a string with a space + %% in it + {ok, _} = sh(filename:join([OutputDir, "foo", "bin", "foo start -goal_app baz '\"bat zing\"'"])), + timer:sleep(2000), + BinFile = filename:join([PrivDir, "goal_app.bin"]), + Eval = io_lib:format("{ok,Env}=application:get_env(goal_app,baz),file:write_file(\"~s\",term_to_binary(Env)).", + [BinFile]), + Cmd = lists:flatten(io_lib:format("foo eval '~s'", [Eval])), + {ok, _} = sh(filename:join([OutputDir, "foo", "bin", Cmd])), + {ok, Bin} = file:read_file(BinFile), + "bat zing" = binary_to_term(Bin), + file:delete(BinFile), + {ok, _} = sh(filename:join([OutputDir, "foo", "bin", "foo stop"])), + %% a ping should fail after stopping a node + {error, 1, _} = sh(filename:join([OutputDir, "foo", "bin", "foo ping"])). + start_fail_when_no_name(Config) -> LibDir1 = proplists:get_value(lib1, Config), VmArgs = filename:join([LibDir1, "vm.args"]), -- cgit v1.2.3 From 1b882e0923f3111e8d8478e439700398b4f45eec Mon Sep 17 00:00:00 2001 From: Luke Bakken Date: Mon, 30 Oct 2017 16:44:56 -0700 Subject: Fix printf statement. Backslash is not necessary and dash is picky about it --- priv/templates/extended_bin | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/priv/templates/extended_bin b/priv/templates/extended_bin index 0482359..19d9d29 100755 --- a/priv/templates/extended_bin +++ b/priv/templates/extended_bin @@ -460,7 +460,7 @@ case "$1" in HEART_OPTION="start_boot" ;; esac - ARGS="$(printf "\'%s\' " "$@")" + ARGS="$(printf "'%s' " "$@")" # Export the HEART_COMMAND HEART_COMMAND="\"$RELEASE_ROOT_DIR/bin/$REL_NAME\" \"$HEART_OPTION\" $ARGS" -- cgit v1.2.3