Merge pull request #840 from tsloughter/upgrade_children

only upgrade children and transitive children of dep being upgraded

src/rebar_prv_upgrade.erl

@@ -47,7 +47,8 @@ do(State) ->
     Locks = rebar_state:get(State, {locks, default}, []),
     Deps = rebar_state:get(State, deps, []),
     Names = parse_names(ec_cnv:to_binary(proplists:get_value(package, Args, <<"">>)), Locks),
-    case prepare_locks(Names, Deps, Locks, []) of
+    DepsDict = deps_dict(rebar_state:all_deps(State)),
+    case prepare_locks(Names, Deps, Locks, [], DepsDict) of
         {error, Reason} ->
             {error, Reason};
         {Locks0, _Unlocks0} ->
@@ -92,54 +93,77 @@ parse_names(Bin, Locks) ->
         Other -> Other
     end.
 
-prepare_locks([], _, Locks, Unlocks) ->
+prepare_locks([], _, Locks, Unlocks, _Dict) ->
     {Locks, Unlocks};
-prepare_locks([Name|Names], Deps, Locks, Unlocks) ->
+prepare_locks([Name|Names], Deps, Locks, Unlocks, Dict) ->
     AtomName = binary_to_atom(Name, utf8),
     case lists:keyfind(Name, 1, Locks) of
         {_, _, 0} = Lock ->
             case rebar_utils:tup_find(AtomName, Deps) of
                 false ->
                     ?WARN("Dependency ~s has been removed and will not be upgraded", [Name]),
-                    prepare_locks(Names, Deps, Locks, Unlocks);
+                    prepare_locks(Names, Deps, Locks, Unlocks, Dict);
                 Dep ->
-                    {Source, NewLocks, NewUnlocks} = prepare_lock(Dep, Lock, Locks),
+                    {Source, NewLocks, NewUnlocks} = prepare_lock(Dep, Lock, Locks, Dict),
                     prepare_locks(Names, Deps, NewLocks,
-                                  [{Name, Source, 0} | NewUnlocks ++ Unlocks])
+                                  [{Name, Source, 0} | NewUnlocks ++ Unlocks], Dict)
             end;
         {_, _, Level} = Lock when Level > 0 ->
             case rebar_utils:tup_find(AtomName, Deps) of
                 false ->
                     ?PRV_ERROR({transitive_dependency, Name});
                 Dep -> % Dep has been promoted
-                    {Source, NewLocks, NewUnlocks} = prepare_lock(Dep, Lock, Locks),
+                    {Source, NewLocks, NewUnlocks} = prepare_lock(Dep, Lock, Locks, Dict),
                     prepare_locks(Names, Deps, NewLocks,
-                                  [{Name, Source, 0} | NewUnlocks ++ Unlocks])
+                                  [{Name, Source, 0} | NewUnlocks ++ Unlocks], Dict)
             end;
         false ->
             ?PRV_ERROR({unknown_dependency, Name})
     end.
 
-prepare_lock(Dep, Lock, Locks) ->
-    Source = case Dep of
-        {_, SrcOrVsn} -> SrcOrVsn;
-        {_, _, Src} -> Src;
+prepare_lock(Dep, Lock, Locks, Dict) ->
+    {Name1, Source} = case Dep of
+        {Name, SrcOrVsn} -> {Name, SrcOrVsn};
+        {Name, _, Src} -> {Name, Src};
         _ when is_atom(Dep) ->
             %% version-free package. Must unlock whatever matches in locks
             {_, Vsn, _} = lists:keyfind(ec_cnv:to_binary(Dep), 1, Locks),
-            Vsn
+            {Dep, Vsn}
     end,
-    {NewLocks, NewUnlocks} = unlock_higher_than(0, Locks -- [Lock]),
+    Children = all_children(Name1, Dict),
+    {NewLocks, NewUnlocks} = unlock_children(Children, Locks -- [Lock]),
     {Source, NewLocks, NewUnlocks}.
 
 top_level_deps(Deps, Locks) ->
     [Dep || Dep <- Deps, lists:keymember(0, 3, Locks)].
 
-unlock_higher_than(Level, Locks) -> unlock_higher_than(Level, Locks, [], []).
+unlock_children(Children, Locks) ->
+    unlock_children(Children, Locks, [], []).
 
-unlock_higher_than(_, [], Locks, Unlocks) ->
+unlock_children(_, [], Locks, Unlocks) ->
     {Locks, Unlocks};
-unlock_higher_than(Level, [App = {_,_,AppLevel} | Apps], Locks, Unlocks) ->
-    if AppLevel > Level  -> unlock_higher_than(Level, Apps, Locks, [App | Unlocks]);
-       AppLevel =< Level -> unlock_higher_than(Level, Apps, [App | Locks], Unlocks)
+unlock_children(Children, [App = {Name,_,_} | Apps], Locks, Unlocks) ->
+    case lists:member(ec_cnv:to_binary(Name), Children) of
+        true ->
+            unlock_children(Children, Apps, Locks, [App | Unlocks]);
+        false ->
+            unlock_children(Children, Apps, [App | Locks], Unlocks)
+    end.
+
+deps_dict(Deps) ->
+    lists:foldl(fun(App, Dict) ->
+                        Name = rebar_app_info:name(App),
+                        Parent = rebar_app_info:parent(App),
+                        dict:append_list(Parent, [Name], Dict)
+                end, dict:new(), Deps).
+
+all_children(Name, Dict) ->
+    lists:flatten(all_children_(Name, Dict)).
+
+all_children_(Name, Dict) ->
+    case dict:find(ec_cnv:to_binary(Name), Dict) of
+        {ok, Children} ->
+            [Children | [all_children_(Child, Dict) || Child <- Children]];
+        error ->
+            []
     end.

test/rebar_upgrade_SUITE.erl

@@ -9,7 +9,7 @@ groups() ->
     [{all, [], [top_a, top_b, top_c, top_d1, top_d2, top_e,
                 pair_a, pair_b, pair_ab, pair_c, pair_all,
                 triplet_a, triplet_b, triplet_c,
-                tree_a, tree_b, tree_c, tree_c2, tree_ac, tree_all,
+                tree_a, tree_b, tree_c, tree_c2, tree_cj, tree_ac, tree_all,
                 delete_d, promote, stable_lock, fwd_lock,
                 compile_upgrade_parity]},
      {git, [], [{group, all}]},
@@ -327,6 +327,25 @@ upgrades(tree_c2) ->
      {"C", [{"A","1"}, "D", "J", "E",
             {"B","1"}, "F", "G",
             {"C","1"}, "H", {"I", "2"}, "K"]}};
+upgrades(tree_cj) ->
+    {[{"A", "1", [{"D",[{"J", "1",[]}]},
+                  {"E",[{"I","1",[]}]}]},
+      {"B", "1", [{"F",[]},
+                  {"G",[]}]},
+      {"C", "1", [{"H",[]},
+                  {"I","1",[]}]}
+     ],
+     [{"A", "1", [{"D",[{"J", "2", []}]},
+                  {"E",[{"I","1",[]}]}]},
+      {"B", "1", [{"F",[]},
+                  {"G",[]}]},
+      {"C", "1", [{"H",[]},
+                  {"I","1",[]}]}
+     ],
+     ["C","J"],
+     {"C", [{"A","1"}, "D", {"J", "1"}, "E", {"I","1"},
+            {"B","1"}, "F", "G",
+            {"C","1"}, "H"]}};
 upgrades(tree_ac) ->
     {[{"A", "1", [{"D",[{"J",[]}]},
                   {"E",[{"I","1",[]}]}]},
@@ -481,6 +500,7 @@ tree_a(Config) -> run(Config).
 tree_b(Config) -> run(Config).
 tree_c(Config) -> run(Config).
 tree_c2(Config) -> run(Config).
+tree_cj(Config) -> run(Config).
 tree_ac(Config) -> run(Config).
 tree_all(Config) -> run(Config).
 promote(Config) -> run(Config).
@@ -561,13 +581,19 @@ run(Config) ->
         {error, Term} -> {error, Term};
         _ -> {ok, Unlocks}
     end,
-    apply(?config(mock_update, Config), []),
+
+    meck:new(rebar_prv_upgrade, [passthrough]),
+    meck:expect(rebar_prv_upgrade, do, fun(S) ->
+                                               apply(?config(mock_update, Config), []),
+                                               meck:passthrough([S])
+                                       end),
     NewRebarConf = rebar_test_utils:create_config(?config(apps, Config),
                                                   [{deps, ?config(next_top_deps, Config)}]),
     {ok, NewRebarConfig} = file:consult(NewRebarConf),
     rebar_test_utils:run_and_check(
         Config, NewRebarConfig, ["upgrade", App], Expectation
-    ).
+     ),
+    meck:unload(rebar_prv_upgrade).
 
 novsn_pkg(Config) ->
     apply(?config(mock, Config), []),