fix: Use application version instead of the release version

scripts/update_appup.escript

@@ -25,22 +25,33 @@ Usage:
 
 Options:
 
+  --check         Don't update the appfile, just check that they are complete
+  --prev-tag      Specify the previous release tag. Otherwise the previous patch version is used
   --repo          Upsteam git repo URL
   --remote        Get upstream repo URL from the specified git remote
   --skip-build    Don't rebuild the releases. May produce wrong results
   --make-command  A command used to assemble the release
   --release-dir   Release directory
+  --src-dirs      Directories where source code is found. Defaults to '{src,apps,lib-*}/**/'
 ".
 
+-record(app,
+        { modules       :: #{module() => binary()}
+        , version       :: string()
+        }).
+
 default_options() ->
     #{ clone_url    => find_upstream_repo("origin")
      , make_command => "make emqx-rel"
      , beams_dir    => "_build/emqx/rel/emqx/lib/"
+     , check        => false
+     , prev_tag     => undefined
+     , src_dirs     => "{src,apps,lib-*}/**/"
      }.
 
 main(Args) ->
-    put(update_appup_valid, true),
     #{current_release := CurrentRelease} = Options = parse_args(Args, default_options()),
+    init_globals(Options),
     case find_pred_tag(CurrentRelease) of
         {ok, Baseline} ->
             main(Options, Baseline);
@@ -51,6 +62,8 @@ main(Args) ->
 
 parse_args([CurrentRelease = [A|_]], State) when A =/= $- ->
     State#{current_release => CurrentRelease};
+parse_args(["--check"|Rest], State) ->
+    parse_args(Rest, State#{check => true});
 parse_args(["--skip-build"|Rest], State) ->
     parse_args(Rest, State#{make_command => "true"});
 parse_args(["--repo", Repo|Rest], State) ->
@@ -65,23 +78,27 @@ parse_args(_, _) ->
     fail(usage()).
 
 main(Options, Baseline) ->
-    {CurrDir, PredDir} = prepare(Baseline, Options),
+    {CurrRelDir, PredRelDir} = prepare(Baseline, Options),
     log("~n===================================~n"
         "Processing changes..."
         "~n===================================~n"),
-    CurrBeams = hashsums(find_beams(CurrDir)),
-    PredBeams = hashsums(find_beams(PredDir)),
-    Upgrade = diff_releases(CurrBeams, PredBeams),
-    Downgrade = diff_releases(PredBeams, CurrBeams),
-    Apps = maps:keys(Upgrade),
-    lists:foreach( fun(App) ->
-                           %% TODO: Here we can find new and deleted apps and handle them accordingly
-                           #{App := AppUpgrade} = Upgrade,
-                           #{App := AppDowngrade} = Downgrade,
-                           process_app(Baseline, App, AppUpgrade, AppDowngrade)
-                   end
-                 , Apps
-                 ),
+    CurrAppsIdx = index_apps(CurrRelDir),
+    PredAppsIdx = index_apps(PredRelDir),
+    %% log("Curr: ~p~nPred: ~p~n", [CurrApps, PredApps]),
+    AppupChanges = find_appup_actions(CurrAppsIdx, PredAppsIdx),
+    case getopt(check) of
+        true ->
+            case AppupChanges of
+                [] ->
+                    ok;
+                _ ->
+                    set_invalid(),
+                    log("ERROR: The appup files are incomplete. Missing changes:~n   ~p", [AppupChanges])
+            end;
+        false ->
+            update_appups(AppupChanges)
+    end,
+    check_appup_files(),
     warn_and_exit(is_valid()).
 
 warn_and_exit(true) ->
@@ -94,66 +111,92 @@ warn_and_exit(false) ->
     log("~nERROR: Incomplete appups found. Please inspect the output for more details.~n"),
     halt(1).
 
-process_app(_, App, {[], [], []}, {[], [], []}) ->
-    %% No changes, just check the appup file if present:
-    case locate(App, ".appup.src") of
-        {ok, AppupFile} ->
-            _ = read_appup(AppupFile),
-            ok;
-        undefined ->
-            ok
-    end;
-process_app(PredVersion, App, Upgrade, Downgrade) ->
-    case locate(App, ".appup.src") of
-        {ok, AppupFile} ->
-            update_appup(PredVersion, AppupFile, Upgrade, Downgrade);
-        undefined ->
-            case create_stub(App) of
-                false ->
-                    set_invalid(),
-                    log("ERROR: External dependency '~p' contains changes, but the appup.src file is NOT updated.
-       Create a patch to the upstream to resolve this issue.~n", [App]),
-                    ok;
-                AppupFile ->
-                    update_appup(PredVersion, AppupFile, Upgrade, Downgrade)
-            end
-    end.
+prepare(Baseline, Options = #{make_command := MakeCommand, beams_dir := BeamDir}) ->
+    log("~n===================================~n"
+        "Baseline: ~s"
+        "~n===================================~n", [Baseline]),
+    log("Building the current version...~n"),
+    bash(MakeCommand),
+    log("Downloading and building the previous release...~n"),
+    {ok, PredRootDir} = build_pred_release(Baseline, Options),
+    {BeamDir, filename:join(PredRootDir, BeamDir)}.
 
-create_stub(App) ->
-    case locate(App, ".app.src") of
-        {ok, AppSrc} ->
-            AppupFile = filename:basename(AppSrc) ++ ".appup.src",
-            Default = {<<".*">>, []},
-            render_appfile(AppupFile, [Default], [Default]),
-            AppupFile;
-        undefined ->
-            false
-    end.
+build_pred_release(Baseline, #{clone_url := Repo, make_command := MakeCommand}) ->
+    BaseDir = "/tmp/emqx-baseline/",
+    Dir = filename:basename(Repo, ".git") ++ [$-|Baseline],
+    %% TODO: shallow clone
+    Script = "mkdir -p ${BASEDIR} &&
+              cd ${BASEDIR} &&
+              { [ -d ${DIR} ] || git clone --branch ${TAG} ${REPO} ${DIR}; } &&
+              cd ${DIR} &&" ++ MakeCommand,
+    Env = [{"REPO", Repo}, {"TAG", Baseline}, {"BASEDIR", BaseDir}, {"DIR", Dir}],
+    bash(Script, Env),
+    {ok, filename:join(BaseDir, Dir)}.
 
-update_appup(PredVersion, File, UpgradeChanges, DowngradeChanges) ->
-    log("INFO: Updating appup: ~s~n", [File]),
-    {_, Upgrade0, Downgrade0} = read_appup(File),
-    Upgrade = update_actions(PredVersion, UpgradeChanges, Upgrade0),
-    Downgrade = update_actions(PredVersion, DowngradeChanges, Downgrade0),
-    render_appfile(File, Upgrade, Downgrade),
-    %% Check appup syntax:
-    _ = read_appup(File).
+find_upstream_repo(Remote) ->
+    string:trim(os:cmd("git remote get-url " ++ Remote)).
 
-render_appfile(File, Upgrade, Downgrade) ->
-    IOList = io_lib:format("%% -*- mode: erlang -*-\n{VSN,~n  ~p,~n  ~p}.~n", [Upgrade, Downgrade]),
-    ok = file:write_file(File, IOList).
+find_pred_tag(CurrentRelease) ->
+    case getopt(prev_tag) of
+        undefined ->
+            {Maj, Min, Patch} = parse_semver(CurrentRelease),
+            case Patch of
+                0 -> undefined;
+                _ -> {ok, semver(Maj, Min, Patch - 1)}
+            end;
+        Tag ->
+            {ok, Tag}
+    end.
 
-update_actions(PredVersion, Changes, Actions) ->
-    lists:map( fun(L) -> do_update_actions(Changes, L) end
-             , ensure_pred_versions(PredVersion, Actions)
-             ).
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%% Appup action creation and updating
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 
-do_update_actions(_, Ret = {<<".*">>, _}) ->
-    Ret;
-do_update_actions(Changes, {Vsn, Actions}) ->
-    {Vsn, process_changes(Changes, Actions)}.
+find_appup_actions(CurrApps, PredApps) ->
+    maps:fold(
+      fun(App, CurrAppIdx, Acc) ->
+              case PredApps of
+                  #{App := PredAppIdx} -> find_appup_actions(App, CurrAppIdx, PredAppIdx) ++ Acc;
+                  _                    -> Acc %% New app, nothing to upgrade here.
+              end
+      end,
+      [],
+      CurrApps).
+
+find_appup_actions(_App, AppIdx, AppIdx) ->
+    %% No changes to the app, ignore:
+    [];
+find_appup_actions(App, CurrAppIdx, PredAppIdx = #app{version = PredVersion}) ->
+    {OldUpgrade, OldDowngrade} = find_old_appup_actions(App, PredVersion),
+    Upgrade = merge_update_actions(diff_app(App, CurrAppIdx, PredAppIdx), OldUpgrade),
+    Downgrade = merge_update_actions(diff_app(App, PredAppIdx, CurrAppIdx), OldDowngrade),
+    if OldUpgrade =:= Upgrade andalso OldDowngrade =:= Downgrade ->
+            %% The appup file has been already updated:
+            [];
+       true ->
+            [{App, {Upgrade, Downgrade}}]
+    end.
 
-process_changes({New0, Changed0, Deleted0}, OldActions) ->
+find_old_appup_actions(App, PredVersion) ->
+    {Upgrade0, Downgrade0} =
+        case locate(App, ".appup.src") of
+            {ok, AppupFile} ->
+                {_, U, D} = read_appup(AppupFile),
+                {U, D};
+            undefined ->
+                {[], []}
+        end,
+    {ensure_version(PredVersion, Upgrade0), ensure_version(PredVersion, Downgrade0)}.
+
+merge_update_actions(Changes, Vsns) ->
+    lists:map(fun(Ret = {<<".*">>, _}) ->
+                      Ret;
+                 ({Vsn, Actions}) ->
+                      {Vsn, do_merge_update_actions(Changes, Actions)}
+              end,
+              Vsns).
+
+do_merge_update_actions({New0, Changed0, Deleted0}, OldActions) ->
     AlreadyHandled = lists:flatten(lists:map(fun process_old_action/1, OldActions)),
     New = New0 -- AlreadyHandled,
     Changed = Changed0 -- AlreadyHandled,
@@ -162,6 +205,7 @@ process_changes({New0, Changed0, Deleted0}, OldActions) ->
         OldActions ++
         [{delete_module, M} || M <- Deleted].
 
+
 %% @doc Process the existing actions to exclude modules that are
 %% already handled
 process_old_action({purge, Modules}) ->
@@ -174,11 +218,6 @@ process_old_action(LoadModule) when is_tuple(LoadModule) andalso
 process_old_action(_) ->
     [].
 
-ensure_pred_versions(PredVersion, Versions) ->
-    {Maj, Min, Patch} = parse_semver(PredVersion),
-    PredVersions = [semver(Maj, Min, P) || P <- lists:seq(0, Patch)],
-    lists:foldl(fun ensure_version/2, Versions, PredVersions).
-
 ensure_version(Version, Versions) ->
     case lists:keyfind(Version, 1, Versions) of
         false ->
@@ -188,6 +227,7 @@ ensure_version(Version, Versions) ->
     end.
 
 read_appup(File) ->
+    %% NOTE: appup file is a script, it may contain variables or functions.
     case file:script(File, [{'VSN', "VSN"}]) of
         {ok, Terms} ->
             Terms;
@@ -195,14 +235,69 @@ read_appup(File) ->
             fail("Failed to parse appup file ~s: ~p", [File, Error])
     end.
 
-diff_releases(Curr, Old) ->
-    Fun = fun(App, Modules, Acc) ->
-                  OldModules = maps:get(App, Old, #{}),
-                  Acc#{App => diff_app_modules(Modules, OldModules)}
-          end,
-    maps:fold(Fun, #{}, Curr).
+check_appup_files() ->
+    AppupFiles = filelib:wildcard(getopt(src_dirs) ++ "/*.appup.src"),
+    lists:foreach(fun read_appup/1, AppupFiles).
+
+update_appups(Changes) ->
+    lists:foreach(
+      fun({App, {Upgrade, Downgrade}}) ->
+              do_update_appup(App, Upgrade, Downgrade)
+      end,
+      Changes).
+
+do_update_appup(App, Upgrade, Downgrade) ->
+    case locate(App, ".appup.src") of
+        {ok, AppupFile} ->
+            render_appfile(AppupFile, Upgrade, Downgrade);
+        undefined ->
+            case create_stub(App) of
+                {ok, AppupFile} ->
+                    render_appfile(AppupFile, Upgrade, Downgrade);
+                false ->
+                    set_invalid(),
+                    log("ERROR: Appup file for the external dependency '~p' is not complete.~n       Missing changes: ~p", [App, Upgrade])
+            end
+    end.
+
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%% Appup file creation
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 
-diff_app_modules(Modules, OldModules) ->
+render_appfile(File, Upgrade, Downgrade) ->
+    IOList = io_lib:format("%% -*- mode: erlang -*-\n{VSN,~n  ~p,~n  ~p}.~n", [Upgrade, Downgrade]),
+    ok = file:write_file(File, IOList).
+
+create_stub(App) ->
+    case locate(App, ".app.src") of
+        {ok, AppSrc} ->
+            AppupFile = filename:basename(AppSrc) ++ ".appup.src",
+            Default = {<<".*">>, []},
+            render_appfile(AppupFile, [Default], [Default]),
+            AppupFile;
+        undefined ->
+            false
+    end.
+
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%% application and release indexing
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+
+index_apps(ReleaseDir) ->
+    maps:from_list([index_app(filename:join(ReleaseDir, AppFile)) ||
+                       AppFile <- filelib:wildcard("**/ebin/*.app", ReleaseDir)]).
+
+index_app(AppFile) ->
+    {ok, [{application, App, Properties}]} = file:consult(AppFile),
+    Vsn = proplists:get_value(vsn, Properties),
+    %% Note: assuming that beams are always located in the same directory where app file is:
+    EbinDir = filename:dirname(AppFile),
+    Modules = hashsums(EbinDir),
+    {App, #app{ version       = Vsn
+              , modules       = Modules
+              }}.
+
+diff_app(App, #app{version = NewVersion, modules = NewModules}, #app{version = OldVersion, modules = OldModules}) ->
     {New, Changed} =
         maps:fold( fun(Mod, MD5, {New, Changed}) ->
                            case OldModules of
@@ -210,75 +305,52 @@ diff_app_modules(Modules, OldModules) ->
                                    {New, Changed};
                                #{Mod := _} ->
                                    {New, [Mod|Changed]};
-                               _ -> {[Mod|New], Changed}
+                               _ ->
+                                   {[Mod|New], Changed}
                            end
                    end
                  , {[], []}
-                 , Modules
+                 , NewModules
                  ),
-    Deleted = maps:keys(maps:without(maps:keys(Modules), OldModules)),
+    Deleted = maps:keys(maps:without(maps:keys(NewModules), OldModules)),
+    NChanges = length(New) + length(Changed) + length(Deleted),
+    if NewVersion =:= OldVersion andalso NChanges > 0 ->
+            set_invalid(),
+            log("ERROR: Application '~p' contains changes, but its version is not updated", [App]);
+       true ->
+            ok
+    end,
     {New, Changed, Deleted}.
 
-find_beams(Dir) ->
-    [filename:join(Dir, I) || I <- filelib:wildcard("**/ebin/*.beam", Dir)].
-
-prepare(Baseline, Options = #{make_command := MakeCommand, beams_dir := BeamDir}) ->
-    log("~n===================================~n"
-        "Baseline: ~s"
-        "~n===================================~n", [Baseline]),
-    log("Building the current version...~n"),
-    bash(MakeCommand),
-    log("Downloading and building the previous release...~n"),
-    {ok, PredRootDir} = build_pred_release(Baseline, Options),
-    {BeamDir, filename:join(PredRootDir, BeamDir)}.
-
-build_pred_release(Baseline, #{clone_url := Repo, make_command := MakeCommand}) ->
-    BaseDir = "/tmp/emqx-baseline/",
-    Dir = filename:basename(Repo, ".git") ++ [$-|Baseline],
-    %% TODO: shallow clone
-    Script = "mkdir -p ${BASEDIR} &&
-              cd ${BASEDIR} &&
-              { [ -d ${DIR} ] || git clone --branch ${TAG} ${REPO} ${DIR}; } &&
-              cd ${DIR} &&" ++ MakeCommand,
-    Env = [{"REPO", Repo}, {"TAG", Baseline}, {"BASEDIR", BaseDir}, {"DIR", Dir}],
-    bash(Script, Env),
-    {ok, filename:join(BaseDir, Dir)}.
+-spec hashsums(file:filename()) -> #{module() => binary()}.
+hashsums(EbinDir) ->
+    maps:from_list(lists:map(
+                     fun(Beam) ->
+                             File = filename:join(EbinDir, Beam),
+                             {ok, Ret = {_Module, _MD5}} = beam_lib:md5(File),
+                             Ret
+                     end,
+                     filelib:wildcard("*.beam", EbinDir)
+                    )).
 
-find_upstream_repo(Remote) ->
-    string:trim(os:cmd("git remote get-url " ++ Remote)).
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%% Global state
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 
-find_pred_tag(CurrentRelease) ->
-    {Maj, Min, Patch} = parse_semver(CurrentRelease),
-    case Patch of
-        0 -> undefined;
-        _ -> {ok, semver(Maj, Min, Patch - 1)}
-    end.
+init_globals(Options) ->
+    ets:new(globals, [named_table, set, public]),
+    ets:insert(globals, {valid, true}),
+    ets:insert(globals, {options, Options}).
 
--spec hashsums(file:filename()) -> #{App => #{module() => binary()}}
-              when App :: atom().
-hashsums(Files) ->
-    hashsums(Files, #{}).
-
-hashsums([], Acc) ->
-    Acc;
-hashsums([File|Rest], Acc0) ->
-    [_, "ebin", Dir|_] = lists:reverse(filename:split(File)),
-    {match, [AppStr]} = re(Dir, "^(.*)-[^-]+$"),
-    App = list_to_atom(AppStr),
-    {ok, {Module, MD5}} = beam_lib:md5(File),
-    Acc = maps:update_with( App
-                          , fun(Old) -> Old #{Module => MD5} end
-                          , #{Module => MD5}
-                          , Acc0
-                          ),
-    hashsums(Rest, Acc).
+getopt(Option) ->
+    maps:get(Option, ets:lookup_element(globals, options, 2)).
 
 %% Set a global flag that something about the appfiles is invalid
 set_invalid() ->
-    put(update_appup_invalid, false).
+    ets:insert(globals, {valid, false}).
 
 is_valid() ->
-    get(update_appup_invalid).
+    ets:lookup_element(globals, valid, 2).
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %% Utility functions
@@ -298,7 +370,8 @@ semver(Maj, Min, Patch) ->
 %% Locate a file in a specified application
 locate(App, Suffix) ->
     AppStr = atom_to_list(App),
-    case filelib:wildcard("{src,apps,lib-*}/**/" ++ AppStr ++ Suffix) of
+    SrcDirs = getopt(src_dirs),
+    case filelib:wildcard(SrcDirs ++ AppStr ++ Suffix) of
         [File] ->
             {ok, File};
         [] ->