bitwalker / distillery

Simplify deployments in Elixir with OTP releases!
MIT License
2.96k stars 396 forks source link

Management script does `cd "$RELEASE_ROOT_DIR"` #682

Open hrubi opened 5 years ago

hrubi commented 5 years ago

Steps to reproduce

$ MIX_ENV=prod mix release

$ cat > script.ex <<\EOF
IO.puts("hello from script.ex")
EOF

$ ./_build/prod/rel/myapp/bin/myapp eval --file ./script.ex
▸  Could not load /home/hrubi/tmp/myapp/_build/prod/rel/myapp/script.ex: could not load /home/hrubi/tmp/myapp/_build/prod/rel/myapp/script.ex                                                                                                 
▸      (elixir) lib/code.ex:1147: Code.find_file/2
▸      (elixir) lib/code.ex:706: Code.eval_file/2
▸      (distillery) lib/mix/lib/releases/runtime/control.ex:704: Mix.Releases.Runtime.Control.eval/2
▸      (distillery) lib/entry.ex:44: Mix.Releases.Runtime.Control.main/1
▸      (stdlib) erl_eval.erl:680: :erl_eval.do_apply/6
▸      (elixir) src/elixir.erl:258: :elixir.eval_forms/4
▸      (elixir) lib/code.ex:232: Code.eval_string/3
▸      (elixir) lib/kernel/cli.ex:503: Kernel.CLI.wrapper/1

Description of issue

I found it inconvenient that when running the release management script commands which take a path as an argument (e.g. the eval or a custom one), that the path is interpreted as relative to the RELEASE_ROOT_DIR. It is counter-intuitive for the end user of the management script commands as he has to know that the relative path is treated that way or to use absolute path.

I'm not yet sure about the motivation why the _build/prod/rel/<name>/releases/<version>/myapp.sh does cd into the release root nor what are the implications of not doing so. The source code in question is in priv/templates/release_rc_main.eex

Distillery: 2.0.14 Erlang: 21.1.1 OS: Linux

bitwalker commented 5 years ago

The reason is to make sure that any paths in the management scripts are relative to the release root, rather than somewhere else on the file system. I think this specific issue could be addressed in a backwards-compatible way by storing the original working directory and restoring it when calling commands that will evaluate user-provided arguments to custom commands.

Actually removing that line is a bigger change, as it would take some time to figure out if there is anything that depends on the current behavior or not, and would be a breaking change for users at this point (though I imagine most would be in favor of the change in general, as would I, but having it happen without warning isn't desirable).