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

[Ulf Wiger]

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