release_handler

[Gunilla Arendt]

Hi Serge,

Yes, the config_change/1 and change_application_data/2 calls should be 
done in the opposite order.

Note however that release_handler:eval_appup_script/4 is actually not
used by release_handler (the process), but is primarily intended to be
called by the release_handler (the module) functions upgrade_app/2
and downgrade_app/2,3. These two functions were added rather recently
to simplify testing of .appup files. (the release_handler process
calls do_install_release/3 to install a new release.)

I will correct the bug and try to clarify the documentation.

Thanks,
Gunilla


Serge Aleynikov wrote:
> OTP team:
> 
> I've been studying the code of the sasl's release_handler, and came up 
> with this question that perhaps someone qualified would be able to answer.
> 
> As far as I understand, the following functions do the following tasks:
> 
> application_controller:prep_config_change()
>   - Fetch OLD applications' environment from
>     application controller's internal ETS table.
> 
> application_controller:config_change(EnvBefore)
>   - Notify running applications (via config_change/3 callback) about
>     configuration changes derived from comparing EnvBefore with the
>     content of the application controller's ETS table storing
>     applications' environment.
> 
> application_controller:change_application_data(AppSpecs, Config)
>   - Rereads applications' environment from config files stored on disk
>     and stores it to the application controller's ETS table.
> 
> Now with this background let's look at how the release handler does the 
> application upgrade:
> 
> ==[begin]== sasl/src/release_handler.erl =====
> ...
> eval_appup_script(App, ToVsn, ToDir, Script) ->
>     EnvBefore = application_controller:prep_config_change(),
>     AppSpecL = read_appspec(App, ToDir),
>     Res = release_handler_1:eval_script(Script,
>                     [], % [AppSpec]
>                     [{App, ToVsn, ToDir}],
>                     []), % [Opt]
>     case Res of
>     {ok, _Unpurged} ->
>         application_controller:config_change(EnvBefore),
>         application_controller:change_application_data(AppSpecL,[]);
>     _Res ->
>         ignore
>     end,
>     Res.
> ...
> ==[end]=======================================
> 
> Unless I am wrong, from this code it does *not* look like application's 
> config_change/3 callback will ever be invoked by 
> application_controller:config_change(EnvBefore) because EnvBefore has 
> the same applications' environment information as the current content of 
> the applications' controller's ETS table.  It seems to me that at least 
> the config_change/1 and change_application_data/2 calls should be done 
> in the opposite order, or better, do something like this:
> 
> ...
> {ok, _Unpurged} ->
>     Files =
>         case init:get_argument(config) of
>         {ok, [ ConfFileNames ]} ->
>             [begin
>                 S = filename:basename(F,".config"),
>                 filename:join(filename:dirname(F), S ++ ".config")
>              end || F <- ConfFileNames],
>         {ok, _} ->
>             []
>         end,
>     application_controller:change_application_data(AppSpecL, Files);
>     application_controller:config_change(EnvBefore);
> _Res ->
>     ignore
> ...
> 
> I'd appreciate some clarification on the subject.
> 
> Regards,
> 
> Serge
> 
>