Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix foreground subcommand with shell/input #786

Merged
merged 2 commits into from
Apr 22, 2020

Conversation

shino
Copy link
Contributor

@shino shino commented Apr 21, 2020

Before this PR, foreground subcommand shows shell prompt as console subcommand.

% rebar3 shell
1> relx:build_release(foo, [{release, {foo, "1"}, [kernel,stdlib,sasl]}]).

(default) % _rel/foo/bin/foo foreground
Exec: /home/shino/local/otp/OTP-22.3/erts-10.7/bin/erlexec -noshell -noinput +Bd -boot /home/shino/g/relx/_rel/foo/releases/1/foo -mode interactive -boot_var SYSTEM_LIB_DIR /home/shino/local/otp/OTP-22.3/lib -config /home/shino/g/relx/_rel/foo/releases/1/sys.config -args_file /home/shino/g/relx/_rel/foo/releases/1/vm.args -- foreground
Root: /home/shino/g/relx/_rel/foo
/home/shino/g/relx/_rel/foo
Erlang/OTP 22 [erts-10.7] [source] [64-bit] [smp:16:16] [ds:16:16:10] [async-threads:1]

Eshell V10.7  (abort with ^G)
(foo@shino-xps)1>

By quoting of FOREGROUNDOPTIONS in extended start script, -noshell -noinput +Bd is treated as single arg instead of three separated args. This PR just removes the quoting.
There would be more sophisticated and platform independent ways to resolve the issue, but I don't have much skill about shell script... 😅 So, other approaches are welcome!

@shino
Copy link
Contributor Author

shino commented Apr 21, 2020

Couldn't reproduce ct failure with OTP 22 (22.3 was used on my laptop with arch linux)

@tsloughter
Copy link
Member

Thanks! I knew some of these were going to get screwed up, surprised no test caught it though...

@tsloughter
Copy link
Member

Ugh, these failures are github's fault. This was happening to me too yesterday, it fails to just checkout the code.

Copy link
Member

@tsloughter tsloughter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just need to add the shellcheck disable comments so that shellcheck will not complain with "Double quote to prevent globbing and word splitting."

priv/templates/extended_bin Show resolved Hide resolved
priv/templates/extended_bin Show resolved Hide resolved
Co-Authored-By: Tristan Sloughter <t@crashfast.com>
@shino
Copy link
Contributor Author

shino commented Apr 22, 2020

Thanks for your review and suggestions!
I accepted them and added another commit.

@tsloughter tsloughter merged commit 597d040 into erlware:4.0.0 Apr 22, 2020
@shino shino deleted the fix-foreground-noshell branch April 23, 2020 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants