From 0932920d3612a259c8f66710cb16bccfb1da32e9 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Thu, 25 Nov 2021 15:02:18 -0300 Subject: [PATCH 1/2] chore(appup): make update_appup.escript output only differences for external dependencies Currently, the update_appup.escript outputs as an error the full appup file for external dependencies, even if all the changes are already contained in the depency. Here, we make it only output the missing actions to be inserted, to aid in seeing what are the differences. --- scripts/update_appup.escript | 77 ++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 8 deletions(-) diff --git a/scripts/update_appup.escript b/scripts/update_appup.escript index 9fc8f618e..1de4ab899 100755 --- a/scripts/update_appup.escript +++ b/scripts/update_appup.escript @@ -99,8 +99,31 @@ main(Options, Baseline) -> [] -> ok; _ -> - set_invalid(), - log("ERROR: The appup files are incomplete. Missing changes:~n ~p", [AppupChanges]) + Diffs = + lists:filtermap( + fun({App, {Upgrade, Downgrade, OldUpgrade, OldDowngrade}}) -> + DiffUp = diff_appup_instructions(Upgrade, OldUpgrade), + DiffDown = diff_appup_instructions(Downgrade, OldDowngrade), + case {DiffUp, DiffDown} of + {[], []} -> + %% no diff for external dependency + false; + _ -> + Diffs = #{ up => DiffUp + , down => DiffDown + }, + {true, {App, Diffs}} + end + end, + AppupChanges), + case Diffs =:= [] of + true -> + ok; + false -> + set_invalid(), + log("ERROR: The appup files are incomplete. Missing changes:~n ~p", + [Diffs]) + end end; false -> update_appups(AppupChanges) @@ -189,9 +212,35 @@ find_appup_actions(App, CurrAppIdx, PrevAppIdx = #app{version = PrevVersion}) -> %% The appup file has been already updated: []; true -> - [{App, {Upgrade, Downgrade}}] + [{App, {Upgrade, Downgrade, OldUpgrade, OldDowngrade}}] end. +%% For external dependencies, show only the changes that are missing +%% in their current appup. +diff_appup_instructions(ComputedChanges, PresentChanges) -> + lists:foldr( + fun({Vsn, ComputedActions}, Acc) -> + case find_matching_version(Vsn, PresentChanges) of + undefined -> + [{Vsn, ComputedActions} | Acc]; + PresentActions -> + DiffActions = ComputedActions -- PresentActions, + case DiffActions of + [] -> + %% no diff + Acc; + _ -> + [{Vsn, DiffActions} | Acc] + end + end + end, + [], + ComputedChanges). + +%% TODO: handle regexes +find_matching_version(Vsn, PresentChanges) -> + proplists:get_value(Vsn, PresentChanges). + find_old_appup_actions(App, PrevVersion) -> {Upgrade0, Downgrade0} = case locate(ebin_current, App, ".appup") of @@ -270,12 +319,12 @@ check_appup_files() -> update_appups(Changes) -> lists:foreach( - fun({App, {Upgrade, Downgrade}}) -> - do_update_appup(App, Upgrade, Downgrade) + fun({App, {Upgrade, Downgrade, OldUpgrade, OldDowngrade}}) -> + do_update_appup(App, Upgrade, Downgrade, OldUpgrade, OldDowngrade) end, Changes). -do_update_appup(App, Upgrade, Downgrade) -> +do_update_appup(App, Upgrade, Downgrade, OldUpgrade, OldDowngrade) -> case locate(src, App, ".appup.src") of {ok, AppupFile} -> render_appfile(AppupFile, Upgrade, Downgrade); @@ -284,8 +333,20 @@ do_update_appup(App, Upgrade, Downgrade) -> {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~n", [App, Upgrade]) + DiffUp = diff_appup_instructions(Upgrade, OldUpgrade), + DiffDown = diff_appup_instructions(Downgrade, OldDowngrade), + case {DiffUp, DiffDown} of + {[], []} -> + %% no diff for external dependency; ignore + ok; + _ -> + set_invalid(), + Diffs = #{ up => DiffUp + , down => DiffDown + }, + log("ERROR: Appup file for the external dependency '~p' is not complete.~n Missing changes: ~100p~n", [App, Diffs]), + log("NOTE: Some changes above might be already covered by regexes.~n") + end end end. From 93caddd448b75af2cc720da9a6e5bcae8fe70ae0 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Mon, 29 Nov 2021 10:23:54 -0300 Subject: [PATCH 2/2] refactor(review): factor out common functionality --- scripts/update_appup.escript | 42 +++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/scripts/update_appup.escript b/scripts/update_appup.escript index 1de4ab899..7674a941c 100755 --- a/scripts/update_appup.escript +++ b/scripts/update_appup.escript @@ -102,16 +102,11 @@ main(Options, Baseline) -> Diffs = lists:filtermap( fun({App, {Upgrade, Downgrade, OldUpgrade, OldDowngrade}}) -> - DiffUp = diff_appup_instructions(Upgrade, OldUpgrade), - DiffDown = diff_appup_instructions(Downgrade, OldDowngrade), - case {DiffUp, DiffDown} of - {[], []} -> - %% no diff for external dependency + case parse_appup_diffs(Upgrade, OldUpgrade, + Downgrade, OldDowngrade) of + ok -> false; - _ -> - Diffs = #{ up => DiffUp - , down => DiffDown - }, + {diffs, Diffs} -> {true, {App, Diffs}} end end, @@ -237,6 +232,23 @@ diff_appup_instructions(ComputedChanges, PresentChanges) -> [], ComputedChanges). +%% For external dependencies, checks if any missing diffs are present +%% and groups them by `up' and `down' types. +parse_appup_diffs(Upgrade, OldUpgrade, Downgrade, OldDowngrade) -> + DiffUp = diff_appup_instructions(Upgrade, OldUpgrade), + DiffDown = diff_appup_instructions(Downgrade, OldDowngrade), + case {DiffUp, DiffDown} of + {[], []} -> + %% no diff for external dependency; ignore + ok; + _ -> + set_invalid(), + Diffs = #{ up => DiffUp + , down => DiffDown + }, + {diffs, Diffs} + end. + %% TODO: handle regexes find_matching_version(Vsn, PresentChanges) -> proplists:get_value(Vsn, PresentChanges). @@ -333,17 +345,13 @@ do_update_appup(App, Upgrade, Downgrade, OldUpgrade, OldDowngrade) -> {ok, AppupFile} -> render_appfile(AppupFile, Upgrade, Downgrade); false -> - DiffUp = diff_appup_instructions(Upgrade, OldUpgrade), - DiffDown = diff_appup_instructions(Downgrade, OldDowngrade), - case {DiffUp, DiffDown} of - {[], []} -> + case parse_appup_diffs(Upgrade, OldUpgrade, + Downgrade, OldDowngrade) of + ok -> %% no diff for external dependency; ignore ok; - _ -> + {diffs, Diffs} -> set_invalid(), - Diffs = #{ up => DiffUp - , down => DiffDown - }, log("ERROR: Appup file for the external dependency '~p' is not complete.~n Missing changes: ~100p~n", [App, Diffs]), log("NOTE: Some changes above might be already covered by regexes.~n") end