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

[Ulf Wiger]

I would suggest that the special handling of {start_phases, undefined} be removed, and documentation updated accordingly.

But this is a decision that OTP will have to make.

BR,
Ulf W

On 5 Jan 2012, at 16:04, Juan Jose Comellas wrote:

> 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
>>>> 
>>>> 
>>