erlware / relx

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

No longer require hooks/extensions must be added to overlays #865

Open lrascao opened 3 years ago

lrascao commented 3 years ago

This is a breaking change in the relx configuration section, the change is small and i think the gain outweighs the hassle.

No longer demand that developers add a hook/extension and also correctly configure the required overlay copy instructions as this now gets automatically added. Start script extensions now also support descriptions.

Extensions: previous:

        {extended_start_script_extensions, [
            {bar, "extensions/bar"},
            {foo, "extensions/foo"},
            {baz, "extensions/baz"}]},
        {overlay, [
            {copy, "./bar", "bin/extensions/bar"},
            {copy, "./foo", "bin/extensions/foo"},
            {copy, "./baz", "bin/extensions/baz"}]}
after:
        {extended_start_script_extensions, [
            % auto copied over to `bin/extensions/bar` with empty description
            {bar, "./bar"},
            % auto copied over to `bin/extensions/foo` with description `"foo description"`
            {foo, "./foo", "foo description"},
            % auto copied over to `bin/extensions/baz` with description `"baz description"`
            {baz, "./baz", "bin/extensions/baz", "baz description"}
        ]}

Hooks: previous:

        {extended_start_script_hooks, [
            {pre_start, [
                {custom, "hooks/pre_start"}
            ]},
            {post_start, [
                wait_for_vm_start,
                {pid, "foo.pid"},
                wait_for_vm_start,
                {custom, "hooks/post_start"}
            ]},
            {pre_stop, [
                {custom, "hooks/pre_stop"}
            ]},
            {post_stop, [
                {custom, "hooks/post_stop"}
            ]},
            {status, [
                {custom, "hooks/status"}
            ]}
        ]},
        {overlay, [
            {copy, "./hooks/status", "bin/hooks/status"},
            {copy, "./hooks/{pre,post}_{start,stop}", "bin/hooks/"}
        ]}
after:
        {extended_start_script_hooks, [
            {pre_start, [
                % auto copied over to `bin/hooks/pre_start`
                {custom, "hooks/pre_start"}
            ]},
            {post_start, [
                wait_for_vm_start,
                {pid, "foo.pid"},
                wait_for_vm_start,
                {custom, "hooks/post_start"},
                % auto copied over to `bin/hooks/extra/post_start`
                {custom, "hooks/post_start2", "bin/hooks/extra/post_start"}
            ]},
            {pre_stop, [
                {custom, "hooks/pre_stop"}
            ]},
            {post_stop, [
                {custom, "hooks/post_stop"}
            ]},
            {status, [
                {custom, "hooks/status"}
            ]}
        ]}
lrascao commented 3 years ago

It does break extension scripts, previously you said: {name, <where the start script can find the extension>} and now: {name, <extension script location>}

I think the breakage is worth it as it:

To ease the migration we can error out when unable to find the source script, update the doc and link it in the error

ferd commented 3 years ago

Maybe. The thing is I have a hard time with breakage since we just went to 4.0.0, and now we'd bump to 5.0.0 right away. I get it's worth it, but it's forcing more breakage into how everyone is building releases. Would it make sense to name the options extended_start_script_extensions_src and extended_start_script_hooks_src where the distinction is that the *_src version assumes you want the source path bundled in the proper place? Would the burden be too crappy to deal with there?

I'm hoping to avoid the case where someone had all their releases broken when upgrading the last rebar3 point version, and gets to break them all over again on the next point version, but maybe this is too narrow of a concern and not that many people use extensions anyway?

lrascao commented 3 years ago

but maybe this is too narrow of a concern and not that many people use extensions anyway?

yeah, that's what i'm thinking, the few people that do use them will probably be ok and won't mind the change as it makes using them generally easier. cc @Vagabond @uwiger, yours are the only projects that i know of that make use of extension scripts, what are your thoughts on this?

We can go with the keep retro-compat route, however cruft will pile on and in the long run things will just get harder to reason

ferd commented 3 years ago

Yeah. I mean I'm okay with removing cruft, I think part of the challenge of maintaining this stuff is in how many edge cases we create. I'm fine with forcing the breaking change if typical users are good with it, I'd say, and there aren't too many of them. This might be a good candidate for it.

lrascao commented 3 years ago

There's no big rush with this as it's an improvement over existing functionality, we can wait and see what people have to say

tsloughter commented 3 years ago

Don't think we can have breakage like this. Any way to support both?

lrascao commented 3 years ago

Suppose we can keep the previous rebar.config format and add a new one, something like:

       % old format
       {extended_start_script_extensions, [
            {bar, "extensions/bar"}
       ]},
       {extended_start_script_hooks, [
           {pre_start, [
              {custom, "hooks/pre_start"}
           ]}
       ]},

        % new format
        {extended_start_script, [
           {extensions, [
              % auto copied over to `bin/extensions/baz` with description `"baz description"`
              {baz, "./baz", "bin/extensions/baz", "baz description"}
           ]},
           {hooks, [
               {post_start, [
                             wait_for_vm_start,
                             {pid, "foo.pid"},
                             wait_for_vm_start,
                             {custom, "hooks/post_start"},
                             {custom, "hooks/post_start2", "bin/hooks/extra/post_start"}
               ]}
           ]}
       ]}
paulo-ferraz-oliveira commented 3 years ago

Assuming there's breakage, are the required changes documented somewhere?

lrascao commented 3 years ago

yeah, they're documented here and here, they'd need to be updated though

paulo-ferraz-oliveira commented 3 years ago

@lrascao, the links are broken. They point to https://github.com/erlware/relx/pull/here.

Edit: in any case I'll check rebar3.org, thanks. Edit 2: ah, I see, but that'd assume that I'd look at the doc.s after the breakage hit me 😄 I guess that's Okay, but the release notes should probably mention it.

ferd commented 3 years ago

Yeah I'd feel better if we just add the new format at some point I guess. Prevents breakage at least.

lrascao commented 3 years ago

roger, i'll keep old tests and expand them with this new format then @tsloughter good with the proposed format above?

lrascao commented 3 years ago

gonna push forward with this format, hopefully this week i can squeeze this in

tsloughter commented 3 years ago

@lrascao any update? I'm going to make a release soon, this will probably be bumped to the next release unless you are close?

lrascao commented 3 years ago

damn, I forgot all about this, better not block this release

tsloughter commented 3 years ago

Ok, will do.

tsloughter commented 2 years ago

oops, didnt meAn to close, thought github knew to switch the target...

tsloughter commented 2 years ago

Ah, I was supposed to rename the default branch and not switch the default... well this is going to be a pain.