[erlang-patches] The 'start_phases' entry in .app files is incorrectly set to 'undefined' when generating a release

[Juan Jose Comellas]

Would it be an option to modify the function that validates the value
assigned to 'start_phases' so that 'undefined' is considered an
acceptable value? The relevant line is:

https://github.com/erlang/otp/blob/master/lib/sasl/src/systools_make.erl#L681

Juanjo


On Tue, Jan 3, 2012 at 5:31 PM, Ulf Wiger <ulf@REDACTED> wrote:
>
> Not my call.
>
> I can well understand if the reasons behind the special treatment of 'start_phases' have been forgotten by those maintaining the code.
>
> A closer analysis might reveal whether backward compatibility has already been sufficiently compromised that it is now time to Make It Right (™).
>
> Even the old AXD 301 project, for which the oddity was introduced in the first place, took the first opportunity to make the code fully compatible with the new semantics.
>
> BR,
> Ulf W
>
> On 3 Jan 2012, at 21:22, Juan Jose Comellas wrote:
>
>> I can make the change and add it to the branch I created, but the
>> question is: should I? I'm not really that well informed about the
>> history of this value. All I can say is that the parts that care about
>> the 'start_phases' entry in reltool and systools assume that it will
>> be an empty list when it is not set and fail otherwise.
>>
>> Let me know if I have to modify my patch to make it acceptable for
>> inclusion in OTP.
>>
>> Juanjo
>>
>>
>> On Tue, Jan 3, 2012 at 8:22 AM, Ulf Wiger <ulf@REDACTED> wrote:
>>>
>>> Hmm, there's an old catch here. Someone will have to hollar if it is no
>>> longer relevant - but if it isn't, some documentation patch is called for.
>>>
>>> In http://www.erlang.org/doc/apps/kernel/application.html#Module:start-2
>>>
>>> StartType defines the type of start:
>>>
>>> normal if it's a normal startup.
>>> normal also if the application is distributed and started at the current
>>> node due to a failover from another node, and the application specification
>>> key start_phases == undefined.
>>> {takeover,Node} if the application is distributed and started at the current
>>> node due to a takeover from Node, either because application:takeover/2 has
>>> been called or because the current node has higher priority than Node.
>>> {failover,Node} if the application is distributed and started at the current
>>> node due to a failover from Node, and the application specification
>>> key start_phases /= undefined.
>>>
>>> Note that StartType = {failover, Node} is only used if start_phases /=
>>> undefined. I will guess that it is for this reason that start_phases is
>>> actually set to undefined as default. If it isn't, it was a happy
>>> coincidence, since it actually preserves backward compatibility.
>>>
>>> This particular oddity was introduced many years ago, in the 90s - possibly
>>> even before Erlang was released as Open Source, so the legacy argument may
>>> not be that relevant.
>>>
>>> OTOH, I don't think many people use the start_phases attribute, so even new
>>> code might break if the Mod:start/2 function is suddenly called with a
>>> StartType that was never seen before.
>>>
>>> BR,
>>> Ulf W
>>>
>>>
>>> On 3 Jan 2012, at 05:21, Juan Jose Comellas wrote:
>>>
>>> The default value for 'start_phases' in .app files should be [], but
>>> it is incorrectly set to 'undefined' when generating a release. This
>>> happens when the original .app file does not set a value for
>>> 'start_phases' explicitly.
>>>
>>> The reltool application defines its own copy of a record to handle
>>> .app files (#app_info{}, defined in lib/reltool/src/reltool.hrl) that
>>> has different default values than the one used by systools
>>> (#application{}, defined in lib/sasl/src/systools.hrl). In particular,
>>> the 'start_phases' entry is assumed by all of OTP to be [] when it is
>>> not explicitly set but reltool sets it to 'undefined' by default. This
>>> causes badmatch errors when trying to generate release upgrades.
>>>
>>> Without this patch, all of the rebar examples that generate release
>>> upgrades that I've found on the web will fail. The bug was introduced
>>> in R14A.
>>>
>>> git fetch git://github.com/jcomellas/otp.git jc-fix-default-start-phases
>>>
>>> Juanjo
>>> _______________________________________________
>>> erlang-patches mailing list
>>> erlang-patches@REDACTED
>>> http://erlang.org/mailman/listinfo/erlang-patches
>>>
>>>
>