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

Release overlays are executed in reverse order as specified in file #1561

Closed
djnym opened this issue Jun 1, 2017 · 7 comments
Closed

Release overlays are executed in reverse order as specified in file #1561

djnym opened this issue Jun 1, 2017 · 7 comments

Comments

@djnym
Copy link
Contributor

djnym commented Jun 1, 2017

I recently added the ability to chmod files in the overlay part of relx, and was confused when it was not working as expected. If I have an overlay config like

{overlay, [ {mkdir, "bin"},
                  {template, "myscript", "bin/myscript"},
                  {chmod, 8#00770, "bin/myscript"} ]}

the chmod fails with enoent because rebar3 appears to be reversing the instructions here. The comment seems to suggest this is for some sort of profile support, but looking at that code I don't quite understand what it's supposed to do. If possible I'd like to keep overlay instructions ordered as above, as opposed to my current work around which is to keep them reversed. If anyone can shed light on why they are reversed and if there might be a better way, let me know and I'll fix it.

@ferd
Copy link
Collaborator

ferd commented Jun 1, 2017

The reversal is because rebar3 provides profile list in order of precedence of most important to least important, because relx consumes them from the reverse order based on how it transforms some. I don't recall exactly why this one specifically is reversed though. It's possible there was a partial patch elsewhere that fixed it but not that one. If you're willing to write a quick patch, I can see about adding the tests for it. Otherwise we'll see to get to it when there's a bit more time, but I'm guessing this never showed up because most overlays are possibly not order-sensitive.

@djnym
Copy link
Contributor Author

djnym commented Jun 1, 2017

Can you give a config example with the profile list and overlays? I think that I may have added the first operation which requires strict ordering (chmoding a file which doesn't exist yet give an error). I think (but haven't tested yet), that the mkdir in my example above is unnecessary (looking through the relx code it looks like it creates directories as needed). Anyway, if you have a simple example of how overlays and profiles are supposed to work I can submit a patch, and if you want to point me in the direction of the testing I can probably also add that.

@ferd
Copy link
Collaborator

ferd commented Jun 1, 2017

The thing I'm wondering if there could be a bug introduced back in 996da46 -- back before that patch, relx had to reverse its argument list in some cases, but the patch changed how merging takes place. This patch (fe7b4aa) introduced the reversal here directly. Maybe @tsloughter can shed light, but if the list is simply reversed, removing that reverse call may be enough.

@djnym
Copy link
Contributor Author

djnym commented Jun 1, 2017

I think maybe the lists:reverse/1 should be around the input list, not around the output. I tested and found that profiles are added after, so I had something list this

{profiles, [{prod, [
                               {overlay, [ {mkdir, "slash/this_is_prod"} ] }
                           ]}]
            }]
}.
{relx, [
            {overlay, [
                 {mkdir, "slash/var/run/minimal"},
                 {mkdir, "slash/var/log/minimal"},
                 {chmod, 8#00770, "slash/etc/init.d/minimal"}, % FIXME: this is in reverse order as rebar3 reverses this
                 {template, "priv/init.script", "slash/etc/init.d/minimal"},
                 {copy, "config/vm.args", "slash/etc/minimal/vm.args"},
                 {copy, "config/sys.config", "slash/etc/minimal/sys.config"},
                 {copy, "priv/pre.sh", "internal/pre.sh"},
                 {copy, "priv/post.sh", "internal/post.sh"},
                 {copy, "priv/preun.sh", "internal/preun.sh"},
                 {copy, "priv/postun.sh", "internal/postun.sh"}
              ]
           }
         ]
}.

If I print this out prior to the merge_overlays/1 call and after I see this

[{overlay,[{mkdir,"slash/var/run/minimal"},
               {mkdir,"slash/var/log/minimal"},
               {chmod,504,"slash/etc/init.d/minimal"},
               {template,"priv/init.script","slash/etc/init.d/minimal"},
               {copy,"config/vm.args","slash/etc/minimal/vm.args"},
               {copy,"config/sys.config","slash/etc/minimal/sys.config"},
               {copy,"priv/pre.sh","internal/pre.sh"},
               {copy,"priv/post.sh","internal/post.sh"},
               {copy,"priv/preun.sh","internal/preun.sh"},
               {copy,"priv/postun.sh","internal/postun.sh"}]},
     {overlay,[{mkdir,"slash/this_is_prod"}]}
]

and post

{overlay,[{mkdir,"slash/this_is_prod"},
                {copy,"priv/postun.sh","internal/postun.sh"},
                {copy,"priv/preun.sh","internal/preun.sh"},
                {copy,"priv/post.sh","internal/post.sh"},
                {copy,"priv/pre.sh","internal/pre.sh"},
                {copy,"config/sys.config","slash/etc/minimal/sys.config"},
                {copy,"config/vm.args","slash/etc/minimal/vm.args"},
                {template,"priv/init.script","slash/etc/init.d/minimal"},
                {chmod,504,"slash/etc/init.d/minimal"},
                {mkdir,"slash/var/log/minimal"},
                {mkdir,"slash/var/run/minimal"}]}

However, if I take the input in the shell and rearrange the lists:reverse/1 I think it gives the correct output

1> Config =                {mkdir,"slash/var/log/minimal"},
1>                {chmod,504,"slash/etc/init.d/minimal"},
1>                {template,"priv/init.script","slash/etc/init.d/minimal"},
1>                {copy,"config/vm.args","slash/etc/minimal/vm.args"},
1>                {copy,"config/sys.config","slash/etc/minimal/sys.config"},
1>                {copy,"priv/pre.sh","internal/pre.sh"},
1>                {copy,"priv/post.sh","internal/post.sh"},
1>                {copy,"priv/preun.sh","internal/preun.sh"},
1>                {copy,"priv/postun.sh","internal/postun.sh"}]},
1>      {overlay,[{mkdir,"slash/this_is_prod"}]}
1> ].
2> {Overlays, Others} =
2>         lists:partition(fun(C) when element(1, C) =:= overlay -> true;
2>                            (_) -> false
2>                         end, Config).
{[{overlay,[{mkdir,"slash/var/run/minimal"},
            {mkdir,"slash/var/log/minimal"},
            {chmod,504,"slash/etc/init.d/minimal"},
            {template,"priv/init.script","slash/etc/init.d/minimal"},
            {copy,"config/vm.args","slash/etc/minimal/vm.args"},
            {copy,"config/sys.config","slash/etc/minimal/sys.config"},
            {copy,"priv/pre.sh","internal/pre.sh"},
            {copy,"priv/post.sh","internal/post.sh"},
            {copy,"priv/preun.sh","internal/preun.sh"},
            {copy,"priv/postun.sh","internal/postun.sh"}]},
  {overlay,[{mkdir,"slash/this_is_prod"}]}],
 []}
3> NewOverlay = lists:flatmap(fun({overlay, Overlay}) -> Overlay end, lists:reverse(O
verlays)).
[{mkdir,"slash/this_is_prod"},
 {mkdir,"slash/var/run/minimal"},
 {mkdir,"slash/var/log/minimal"},
 {chmod,504,"slash/etc/init.d/minimal"},
 {template,"priv/init.script","slash/etc/init.d/minimal"},
 {copy,"config/vm.args","slash/etc/minimal/vm.args"},
 {copy,"config/sys.config","slash/etc/minimal/sys.config"},
 {copy,"priv/pre.sh","internal/pre.sh"},
 {copy,"priv/post.sh","internal/post.sh"},
 {copy,"priv/preun.sh","internal/preun.sh"},
 {copy,"priv/postun.sh","internal/postun.sh"}]
4>

Which if I understand correctly is the profile overlays first, then the relx overlays, but without the individual overlays being reversed. Seem like a reasonable change?

@ferd
Copy link
Collaborator

ferd commented Jun 1, 2017

Yeah that appears correct and reasonable to me

@djnym
Copy link
Contributor Author

djnym commented Jun 1, 2017

I'll send a PR.

@djnym
Copy link
Contributor Author

djnym commented Jun 1, 2017

Okay here it is #1563

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

No branches or pull requests

2 participants