bitwalker / distillery

Simplify deployments in Elixir with OTP releases!
MIT License
2.97k stars 398 forks source link

fix loading pidfile config from vm args #600

Closed olgad0110 closed 5 years ago

olgad0110 commented 5 years ago

Summary of changes

Connected with issue https://github.com/bitwalker/distillery/issues/582

Current behaviour: When PID file configuration is loaded from the vm.args file in any of the following formats:

-kernel pidfile "'etc/server.pid'"
-kernel pidfile '"etc/server.pid"'

it creates the following error

{"could not start kernel pid",'Elixir.Mix.Releases.Runtime.Pidfile',{error {invalid_pidfile_config,'etc/server.pid'}}}
could not start kernel pid (Elixir.Mix.Releases.Runtime.Pidfile) ({error {invalid_pidfile_config,etc/server.pid}})

This prevents from having a working pidfile config in vm.args file

Solution: After some debugging I found that in case of the second format ('"etc/server.pid"') the path variable is a list, while in the code there is only check for binary https://github.com/bitwalker/distillery/blob/master/lib/mix/lib/releases/runtime/pidfile.ex#L34 It seems to me that maybe this value is an Erlang charlist, although I don't have much knowledge about Erlang, so I may be wrong.

Feel free to test this solution, it works for me for the format '"etc/server.pid"' in the vm.args file.

NOTE: If this PR will be approved, there should also be an update in the documentation, as now it shows a format of "'etc/server.pid'" (which is not working and is not fixed by this PR)

Checklist

Licensing/Copyright

By submitting this PR, you agree to the following statement, please read before submission!

I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for Distillery (the "Contribution"). My Contribution is licensed under the MIT License.

NOTE: If you submit a PR and remove the statement above, your PR will be rejected. For your PR to be considered, it must contain your agreement to license under the MIT license.

bitwalker commented 5 years ago

Thanks for fixing this!