Merge branch 'siri/sasl/delete_module/OTP-11540'

* siri/sasl/delete_module/OTP-11540:
  [sasl] Remove undocumented upgrade instruction

6 files changed, 92 insertions, 21 deletions

diff --git a/lib/sasl/doc/src/appup.xml b/lib/sasl/doc/src/appup.xml
index 95f315d269..f0f41b0c7e 100644
--- a/lib/sasl/doc/src/appup.xml
+++ b/lib/sasl/doc/src/appup.xml
@@ -180,15 +180,28 @@
       <c>Mod</c> when upgrading, and vice versa when downgrading.</p>
     <pre>
 {add_module, Mod}
+{add_module, Mod, DepMods}
   Mod = atom()
+  DepMods = [Mod]
     </pre>
     <p>Loads a new module <c>Mod</c>.</p>
+    <p><c>DepMods</c> defaults to [] and defines which other modules
+      <c>Mod</c> is dependent on. In <c>relup</c>, instructions
+      related to these modules will come before the instruction for
+      loading <c>Mod</c> when upgrading, and vice versa when
+      downgrading.</p>
     <pre>
 {delete_module, Mod}
+{delete_module, Mod, DepMods}
   Mod = atom()
     </pre>
     <p>Deletes a module <c>Mod</c> using the low-level instructions
       <c>remove</c> and <c>purge</c>.</p>
+    <p><c>DepMods</c> defaults to [] and defines which other modules
+      <c>Mod</c> is dependent on. In <c>relup</c>, instructions
+      related to these modules will come before the instruction for
+      removing <c>Mod</c> when upgrading, and vice versa when
+      downgrading.</p>
     <pre>
 {add_application, Application}
 {add_application, Application, Type}
diff --git a/lib/sasl/src/systools_rc.erl b/lib/sasl/src/systools_rc.erl
index 76f753c3d0..11e097996c 100644
--- a/lib/sasl/src/systools_rc.erl
+++ b/lib/sasl/src/systools_rc.erl
@@ -32,7 +32,6 @@
 %% {load_module, Mod, PrePurge, PostPurge, [Mod]}
 %% {add_module, Mod}
 %% {add_module, Mod, [Mod]}
-%% {remove_module, Mod, PrePurge, PostPurge, [Mod]}
 %% {restart_application, Appl}
 %% {add_application, Appl, Type}
 %% {remove_application, Appl}
@@ -59,7 +58,7 @@
 
 %% High-level instructions that contain dependencies
 %%
--define(DEP_INSTRS, [update, load_module, add_module, remove_module]).
+-define(DEP_INSTRS, [update, load_module, add_module, delete_module]).
 
 %%-----------------------------------------------------------------
 %% translate_scripts(Scripts, Appls, PreAppls) -> Res
@@ -107,9 +106,6 @@ expand_script([I|Script]) ->
 	     {update, Mod, Change, Mods} when Change==soft,
 					      is_list(Mods) ->
 		 {update, Mod, Change, brutal_purge,brutal_purge, Mods};
-	     {delete_module, Mod} ->
-		 [{remove, {Mod, brutal_purge, brutal_purge}},
-		  {purge, [Mod]}];
 	     {add_application, Application} ->
 		 {add_application, Application, permanent};
 	     _ ->
@@ -301,6 +297,8 @@ normalize_instrs(Script) ->
 		       PostPurge, Mods};
 		 ({add_module, Mod}) ->
 		      {add_module, Mod, []};
+		 ({delete_module, Mod}) ->
+		      {delete_module, Mod, []};
 		 (I) ->
 		      I
 	      end, Script).
@@ -412,7 +410,7 @@ translate_add_module_instrs(Before, After) ->
 %%-----------------------------------------------------------------
 
 %%-----------------------------------------------------------------
-%% Translates update, load_module and remove_module, and reorder the
+%% Translates update, load_module and delete_module, and reorder the
 %% instructions according to dependencies. Leaves other instructions
 %% unchanged.
 %%-----------------------------------------------------------------
@@ -538,7 +536,7 @@ get_dependent_instructions(G, WCs, Mod) ->
 %% Instructions are in order of dependency.
 %% Appls = [#application]
 %%
-%% Instructions translated are: update, load_module, and remove_module 
+%% Instructions translated are: update, load_module, and delete_module
 %%
 %% Before =	[{load_object_code, ...}]
 %% After = 	[{suspend, ...}] ++ CodeInstrs ++ [{resume, ...}]
@@ -576,17 +574,19 @@ translate_dep_to_low(Mode, Instructions, Appls) ->
 				end, RevUpdateMods)}]
 	end,
 
-    LoadRemoveInstrs = 
+    LoadRemoveInstrs0 =
 	filtermap(fun({update, Mod, _, _, _, PreP, PostP, _}) ->
 			  {true, {load, {Mod, PreP, PostP}}};
 		     ({load_module, Mod, PreP, PostP, _}) ->
 			  {true, {load, {Mod, PreP, PostP}}};
-		     ({remove_module, Mod, PreP, PostP, _}) ->
-			  {true, {remove, {Mod, PreP, PostP}}};
+		     ({delete_module, Mod, _}) ->
+			  {true,[{remove, {Mod, brutal_purge, brutal_purge}},
+				 {purge, [Mod]}]};
 		     (_) -> false
 		  end,
 		  Instructions),
-    RevLoadRemoveInstrs = lists:reverse(LoadRemoveInstrs),
+    LoadRemoveInstrs = lists:flatten(LoadRemoveInstrs0),
+    RevLoadRemoveInstrs = lists:flatten(lists:reverse(LoadRemoveInstrs0)),
 
     %% The order of loading object code is unimportant. The order
     %% chosen is the order of dependency.
@@ -781,10 +781,10 @@ check_op({add_module, Mod, Mods}) ->
     check_mod(Mod),
     check_list(Mods),
     lists:foreach(fun(M) -> check_mod(M) end, Mods);
-check_op({remove_module, Mod, PrePurge, PostPurge, Mods}) ->
+check_op({delete_module, Mod}) ->
+    check_mod(Mod);
+check_op({delete_module, Mod, Mods}) ->
     check_mod(Mod),
-    check_purge(PrePurge),
-    check_purge(PostPurge),
     check_list(Mods),
     lists:foreach(fun(M) -> check_mod(M) end, Mods);
 check_op({remove_application, Appl}) ->
diff --git a/lib/sasl/test/release_handler_SUITE_data/lib/README b/lib/sasl/test/release_handler_SUITE_data/lib/README
index ffb8c5120b..5d17950b0b 100644
--- a/lib/sasl/test/release_handler_SUITE_data/lib/README
+++ b/lib/sasl/test/release_handler_SUITE_data/lib/README
@@ -21,7 +21,7 @@ start version, includes b_lib and b_server
 
 b-2.0:
 can be upgraded to from b-1.0.
-Removes b_lib (soft_purge) and updates b_server (brutal_purge)
+Removes b_lib (brutal_purge) and updates b_server (soft_purge)
 * The diff in purge method is important for test "check_and_purge", in
   order to check that the purge option to check_install_release works
   for both methods.
diff --git a/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/ebin/b.appup b/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/ebin/b.appup
index 001255a88c..9df590e63f 100644
--- a/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/ebin/b.appup
+++ b/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/ebin/b.appup
@@ -1,6 +1,6 @@
 %% -*- erlang -*-
 {"2.0",
- [{"1.0",[{remove_module,b_lib,soft_purge,soft_purge,[]},
-	  {update,b_server,{advanced,[]}}]}],
+ [{"1.0",[{delete_module,b_lib},
+	  {update,b_server,{advanced,[]},soft_purge,soft_purge,[]}]}],
  [{"1.0",[{add_module,b_lib},
-	  {update,b_server,{advanced,[]}}]}]}.
+	  {update,b_server,{advanced,[]},soft_purge,soft_purge,[]}]}]}.
diff --git a/lib/sasl/test/systools_rc_SUITE.erl b/lib/sasl/test/systools_rc_SUITE.erl
index 5efab7c028..1afef986d2 100644
--- a/lib/sasl/test/systools_rc_SUITE.erl
+++ b/lib/sasl/test/systools_rc_SUITE.erl
@@ -22,14 +22,16 @@
 -include_lib("sasl/src/systools.hrl").
 -export([all/0,groups/0,init_per_group/2,end_per_group/2, 
 	 syntax_check/1, translate/1, translate_app/1,
-	 translate_emulator_restarts/1]).
+	 translate_emulator_restarts/1,
+	 translate_add_delete_module/1]).
 
 %%-----------------------------------------------------------------
 %% erl -compile systools_rc_SUITE @i ../src/ @i ../../test_server/include/
 %% c(systools_rc_SUITE, [{i, "../src"}, {i, "../../test_server/include"}]).
 %%-----------------------------------------------------------------
 all() -> 
-    [syntax_check, translate, translate_app, translate_emulator_restarts].
+    [syntax_check, translate, translate_app, translate_emulator_restarts,
+     translate_add_delete_module].
 
 groups() -> 
     [].
@@ -707,3 +709,59 @@ translate_emulator_restarts(_Config) ->
      restart_emulator] = X6,
 
     ok.
+
+translate_add_delete_module(_Config) ->
+    PreApps =
+	[#application{name = test,
+		      description = "TEST",
+		      vsn = "0.1",
+		      modules = [foo,bar,baz,old_mod],
+		      regs = [],
+		      mod = {sasl, []}}],
+    Apps =
+	[#application{name = test,
+		      description = "TEST",
+		      vsn = "1.0",
+		      modules = [foo,bar,baz,new_mod],
+		      regs = [],
+		      mod = {sasl, []}}],
+    S1 = [
+	  {delete_module, old_mod},
+	  {add_module, new_mod},
+	  {load_module, foo}
+	 ],
+    {ok, X1} = systools_rc:translate_scripts([S1], Apps, PreApps),
+    [{load_object_code,{test,"1.0",[new_mod,foo]}},
+     point_of_no_return,
+     {remove,{old_mod,brutal_purge,brutal_purge}},
+     {purge,[old_mod]},
+     {load,{new_mod,brutal_purge,brutal_purge}},
+     {load,{foo,brutal_purge,brutal_purge}}] = X1,
+
+    S2 = [
+	  {delete_module, old_mod},
+	  {add_module, new_mod, [foo]},
+	  {load_module, foo}
+	 ],
+    {ok, X2} = systools_rc:translate_scripts([S2], Apps, PreApps),
+    [{load_object_code,{test,"1.0",[new_mod,foo]}},
+     point_of_no_return,
+     {remove,{old_mod,brutal_purge,brutal_purge}},
+     {purge,[old_mod]},
+     {load,{foo,brutal_purge,brutal_purge}},
+     {load,{new_mod,brutal_purge,brutal_purge}}] = X2,
+
+    S3 = [
+	  {delete_module, old_mod, [new_mod]},
+	  {add_module, new_mod, [foo]},
+	  {load_module, foo}
+	 ],
+    {ok, X3} = systools_rc:translate_scripts([S3], Apps, PreApps),
+    [{load_object_code,{test,"1.0",[new_mod,foo]}},
+     point_of_no_return,
+     {load,{foo,brutal_purge,brutal_purge}},
+     {load,{new_mod,brutal_purge,brutal_purge}},
+     {remove,{old_mod,brutal_purge,brutal_purge}},
+     {purge,[old_mod]}] = X3,
+
+    ok.
diff --git a/system/doc/design_principles/appup_cookbook.xml b/system/doc/design_principles/appup_cookbook.xml
index 45f144e8b7..22c48db855 100644
--- a/system/doc/design_principles/appup_cookbook.xml
+++ b/system/doc/design_principles/appup_cookbook.xml
@@ -132,7 +132,7 @@ code_change(_Vsn, Chs, _Extra) ->
       loaded.</p>
     <p>Thus, <c>ch3</c> must be loaded before <c>m1</c> is, in
       the upgrade case, and vice versa in the downgrade case. We say
-      that <c>m1</c><em>is dependent on</em><c>ch3</c>. In a release
+      that <c>m1</c> <em>is dependent on</em> <c>ch3</c>. In a release
       handling instruction, this is expressed by the element
       <c>DepMods</c>:</p>
     <code type="none">