erlware / relx

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

Adding powershell start scripts for windows, fix nodetool ping output #796

Closed reganheath closed 4 years ago

reganheath commented 4 years ago

I have created powershell versions of the bin_windows and extended_bin_windows start scripts called bin_windows_ps and extended_bin_windows_ps plus included a psutil utility script.

To enable use {start_script_type, powershell} in rebar.config relx section. Works with {extended_start_script, true} and {extended_start_script, false}

Resolves #657 by doing env replacement on .src and handling RELX_REPLACE_OS_VARS.

Resolves #470 by NOT writing erl.ini and instead setting the ERL_LIBS environment variable.

The extension support in extended_bin_windows was not ideal, these implement it fully (as in extended_bin).

I have also fixed "nodetool ping ..etc.." which was performing the ping correctly, but then outputting:

Other: ["ping"]
Usage: nodetool {ping|stop|restart|reboot|rpc|rpcterms|eval [Terms]} [RPC]

as if it didn't understand the ping command.

I have NOT added any common tests for this. 3 tests appear to be failing, but seem unrelated to my changes - rather I suspect they haven't been run on windows in a while?

I also have not built a release from this and worked out how to include it in rebar3 for a full test.

I have tested the powershell scripts by including them in a release, however I have no idea how to use upgrade/downgrade (relup) so I have not tested it but simply implemented what was already in the existing scripts.

Note, this was based on the 4.0.0 branch.

reganheath commented 4 years ago

I will have a look at shelltests/ and see what I can add :)

reganheath commented 4 years ago

I will have a look at shelltests/ and see what I can add :)

This looks complicated. I think it's going to require a new github workflow, wintests folder, powershell script and so on, right?

tsloughter commented 4 years ago

@reganheath I was thinking just a github action that ran under windows, built a powershell release (simply add an example release to the relx repo, could even be under shelltests/ with the other examples but just wouldn't have a .test file so shelltest would ignore it) and started it.

We can worry about that later if its too much, I wasn't sure how github action windows works anyway and if it gives you powershell by default and lets you just run commands like that or not.

reganheath commented 4 years ago

Well.. that was frustrating!

I have implemented a test. I had to implement my own erlang installation process - as the existing action only supports linux (I may contribute my implementation to that project later).

The test even found a few bugs, now fixed, and it even passes with OTP 20 and 22, however..

With OTP 19 - relx fails to build: This looks like an actual bug and I am kinda surprised it's not happening in other versions? Run: https://github.com/erlware/relx/runs/694952533?check_suite_focus=true Error:

===> Compiling _checkouts/relx/src/rlx_relup.erl failed
_checkouts/relx/src/rlx_relup.erl:27: this clause cannot match because a previous clause at line 22 always matches

With OTP 21 - nodetool fails to find epmd.exe os:find_executable simply fails (depite all my efforts to place it in the system PATH - not just local environment). This is especially frustrating as this is the version I have installed, and it has no issue without epmd.exe being in the PATH. Run: https://github.com/erlware/relx/runs/694977835?check_suite_focus=true https://github.com/erlware/relx/runs/694991175?check_suite_focus=true

Debug Output / Error:

*** DEBUG PATH = D:\a\relx\relx\shelltests\erl\bin;D:\a\relx\relx\shelltests\erl\erts-10.0\bin;C:\Program Files\PowerShell\7; ..etc..

*** DEBUG nodetool d:\a\relx\relx\shelltests\erl\erts-10.0\bin\escript.exe D:\a\relx\relx\shelltests\powershell_release\_build\default\rel\powershell_release\bin\nodetool -sname powershell_release -setcookie powershell_release_cookie rpc erlang is_alive

Could not find epmd.

With OTP 23 - I have a bug running erlcall to continue debugging Run: https://github.com/erlware/relx/runs/695034370?check_suite_focus=true Output:

*** Ping service
Illegal argument 'erlang'.
ferd commented 4 years ago

Dang that's dedication, very appreciated. Nice find on the bug. Re: the erlcall bug, be aware that @tsloughter found out issues with the OTP stuff in OTP 23 and has recently changed the 4.0.0 branch accordingly: https://github.com/erlware/relx/pull/798 -- don't know if that could explain the erlcall issue.

tsloughter commented 4 years ago

Really great!

My first guess on the erl_call failure because it says erlang is illegal is bad quoting? For ping it'll run erl_call ... -a erlang is_alive.

tsloughter commented 4 years ago

In the rebar3 github actions I just use choco install erlang I see you are doing something more specific in your install script. Is that necessary or would choco be good enough?

tsloughter commented 4 years ago

Whoa, that build failure is definitely a real issue:

format_error({no_upfrom_release_found, Vsn})->
    io_lib:format("No previous version of release found for building relup to version ~s", [Vsn]);
format_error({no_upfrom_release_found, undefined}) ->
    io_lib:format("No earlier release for relup found", []);
format_error({no_upfrom_release_found, Vsn}) ->
    io_lib:format("Upfrom release version (~s) for relup not found", [Vsn]);

I have no idea how this was only caught by otp-19 on windows...

tsloughter commented 4 years ago

@reganheath do you mind just ripping out that first clause, as in remove these two lines in your PR:

format_error({no_upfrom_release_found, Vsn})->
    io_lib:format("No previous version of release found for building relup to version ~s", [Vsn]);
reganheath commented 4 years ago

In the rebar3 github actions I just use choco install erlang I see you are doing something more specific in your install script. Is that necessary or would choco be good enough?

I've never used it - I'll give it a go.

reganheath commented 4 years ago

@reganheath do you mind just ripping out that first clause, as in remove these two lines in your PR:

format_error({no_upfrom_release_found, Vsn})->
    io_lib:format("No previous version of release found for building relup to version ~s", [Vsn]);

Sure.

reganheath commented 4 years ago

I have no idea how this was only caught by otp-19 on windows...

OTP bug introduced in 20 perhaps?

reganheath commented 4 years ago

In the rebar3 github actions I just use choco install erlang I see you are doing something more specific in your install script. Is that necessary or would choco be good enough?

I think this will install Erlang 22.3 and not OTP 19, 20, 21, etc as required by the matrix.

reganheath commented 4 years ago

Turns out the OTP 21 issue is a regression in that version, see:

So, we cannot test on this version

reganheath commented 4 years ago

Really great!

My first guess on the erl_call failure because it says erlang is illegal is bad quoting? For ping it'll run erl_call ... -a erlang is_alive.

You were correct :)

reganheath commented 4 years ago

Question see wintest.yml, what do we want to do in this MR for the install_erlang step. I have provided 3 options, 2 of which will work now and the 3rd being the correct version once my MR for gleam-lang/setup-erlang is accepted.

Update my pull request has been merged to gleam-lang/setup-erlang so our path is clear :)

tsloughter commented 4 years ago

@reganheath awesome work! Is this ready to merge now?

ferd commented 4 years ago

Yeah that's really awesome work. It's good to see that part of the code get some real love instead of bare-minimum maintenance :)

reganheath commented 4 years ago

@reganheath awesome work! Is this ready to merge now?

Yeah that's really awesome work. It's good to see that part of the code get some real love instead of bare-minimum maintenance :)

Awesome, thanks guys!