search

erlware / relx

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

Extending a release loses annotations on goals #720

Open phanimahesh opened 5 years ago

phanimahesh commented 5 years ago

Discovered this while writing https://github.com/emqx/emqx-rel/pull/351

What does this result in:

Any {app, load} instructions in a release you are extending from get silently converted to app, which results in the release starting them permanently. This is clearly unexpected behaviour, and improper.

Why does this happen:

The configuration processing of {extend, RelName} happens here: https://github.com/erlware/relx/blob/ba0986b55bbee9b28b9467658312c1d9d93a4b40/src/rlx_config.erl#L190-L202

That uses rlx_release:goals/1 to get the goals, which on surface looks okay: https://github.com/erlware/relx/blob/b69658a37a0cf62a45bc0de89551344b2cf38595/src/rlx_release.erl#L140-L142

HOWEVER,

All releases use rlx_release:goals/2 to set their goals. https://github.com/erlware/relx/blob/b69658a37a0cf62a45bc0de89551344b2cf38595/src/rlx_release.erl#L135-L138

It calls parse_goal0/2 which separates a Goal into a Constraint and Annotations, https://github.com/erlware/relx/blob/b69658a37a0cf62a45bc0de89551344b2cf38595/src/rlx_release.erl#L342-L353

and then calls parse_goal1/3, which ... https://github.com/erlware/relx/blob/b69658a37a0cf62a45bc0de89551344b2cf38595/src/rlx_release.erl#L379-L388

separates annotations into a different key. Goals at this point is just a list of applications. When just the goals are extracted, annotations from the original release are lost here.

Is this public api?

I haven't found any mention of extending releases anywhere in official docs, but it looks like a public api. If it is indeed intended to be public, can I submit documentation updates to it?

I can also submit a PR to fix this issue if it is acceptable. Most of the legwork is done anyway, when I was trying to confirm that it is a bug.

tsloughter commented 5 years ago

Yes, it is intended to be public and documentation would be great as well as a fix for this issue, thanks!

phanimahesh commented 5 years ago

I have absolutely no idea why I picked the commit https://github.com/erlware/relx/commit/ba0986b55bbee9b28b9467658312c1d9d93a4b40. I should have picked up the latest commit, but somewhere along the way made a mistake (probably too many tabs of github relx and picked wrong one), and realized it only after "fixing" it.

The "latest" code is different, specifically the rlx_release:goals/1. I need to redo my tests to see if this issue even exists in the latest version.

tsloughter commented 5 years ago

@phanimahesh did you ever get a chance to redo the test?