Add tests and fixes for packages upgrades

- Track level of packages properly, they're not level 0 anymore (this
  could yield an issue where a src dep takes precedence over a pkg dep)
- Proper stable sort of vertices in the digraph module
- PkgDeps no longer 'see themselves' when fetching and upgrading after
  locking themselves
- Pkg Locks are added to pkg deps rather than source deps
- Updating test cases to support pkg mocking on top of src mocking

src/rebar_digraph.erl

@@ -71,29 +71,36 @@ restore_graph({Vs, Es}) ->
 cull_deps(Graph, Vertices) ->
     cull_deps(Graph,
               Vertices,
+              1,
+              lists:foldl(fun({Key, _}, Levels) -> dict:store(Key, 0, Levels) end,
+                          dict:new(), Vertices),
               lists:foldl(fun({Key, _}=N, Solution) -> dict:store(Key, N, Solution) end,
                           dict:new(), Vertices),
               []).
 
-cull_deps(_Graph, [], Solution, Discarded) ->
+cull_deps(_Graph, [], _Level, Levels, Solution, Discarded) ->
     {_, Vertices} = lists:unzip(dict:to_list(Solution)),
-    {ok, Vertices, Discarded};
-cull_deps(Graph, Vertices, Solution, Discarded) ->
-    {NV, NS, DS} =
-        lists:foldl(fun(V, {NewVertices, SolutionAcc, DiscardedAcc}) ->
-                        OutNeighbors = digraph:out_neighbours(Graph, V),
-                        lists:foldl(fun({Key, _}=N, {NewVertices1, SolutionAcc1, DiscardedAcc1}) ->
+    LvlVertices = [{App,Vsn,dict:fetch(App,Levels)} || {App,Vsn} <- Vertices],
+    {ok, LvlVertices, Discarded};
+cull_deps(Graph, Vertices, Level, Levels, Solution, Discarded) ->
+    {NV, NS, LS, DS} =
+        lists:foldl(fun(V, {NewVertices, SolutionAcc, LevelsAcc, DiscardedAcc}) ->
+                        OutNeighbors = lists:keysort(1, digraph:out_neighbours(Graph, V)),
+                        lists:foldl(fun({Key, _}=N, {NewVertices1, SolutionAcc1, LevelsAcc1, DiscardedAcc1}) ->
                                             case dict:find(Key, SolutionAcc1) of
                                                 {ok, N} -> % already seen
-                                                    {NewVertices1, SolutionAcc1, DiscardedAcc1};
+                                                    {NewVertices1, SolutionAcc1, LevelsAcc, DiscardedAcc1};
                                                 {ok, _} -> % conflict resolution!
-                                                    {NewVertices1, SolutionAcc1, [N|DiscardedAcc1]};
+                                                    {NewVertices1, SolutionAcc1, LevelsAcc, [N|DiscardedAcc1]};
                                                 error ->
-                                                    {[N | NewVertices1], dict:store(Key, N, SolutionAcc1), DiscardedAcc1}
+                                                    {[N | NewVertices1],
+                                                     dict:store(Key, N, SolutionAcc1),
+                                                     dict:store(Key, Level, LevelsAcc1),
+                                                     DiscardedAcc1}
                                             end
-                                    end, {NewVertices, SolutionAcc, DiscardedAcc}, OutNeighbors)
-                    end, {[], Solution, Discarded}, lists:sort(Vertices)),
-    cull_deps(Graph, NV, NS, DS).
+                                    end, {NewVertices, SolutionAcc, LevelsAcc, DiscardedAcc}, OutNeighbors)
+                    end, {[], Solution, Levels, Discarded}, lists:keysort(1, Vertices)),
+    cull_deps(Graph, NV, Level+1, LS, NS, DS).
 
 subgraph(Graph, Vertices) ->
     digraph_utils:subgraph(Graph, Vertices).

src/rebar_pkg_resource.erl

@@ -14,7 +14,7 @@
 lock(_AppDir, Source) ->
     Source.
 
-needs_update(Dir, {pkg, _Name, Vsn, _Url}) ->
+needs_update(Dir, {pkg, _Name, Vsn}) ->
     [AppInfo] = rebar_app_discover:find_apps([Dir], all),
     case rebar_app_info:original_vsn(AppInfo) =:= ec_cnv:to_list(Vsn) of
         true ->

src/rebar_prv_install_deps.erl

@@ -204,8 +204,9 @@ update_pkg_deps(Profile, Pkgs, Packages, Upgrade, Seen, State) ->
 
 handle_pkg_dep(Profile, Pkg, Packages, Upgrade, DepsDir, Fetched, Seen, State) ->
     AppInfo = package_to_app(DepsDir, Packages, Pkg),
-    {NewSeen, NewState} = maybe_lock(Profile, AppInfo, Seen, State, 0),
-    case maybe_fetch(AppInfo, Upgrade, NewSeen, NewState) of
+    Level = rebar_app_info:dep_level(AppInfo),
+    {NewSeen, NewState} = maybe_lock(Profile, AppInfo, Seen, State, Level),
+    case maybe_fetch(AppInfo, Upgrade, Seen, NewState) of
         true ->
             {[AppInfo | Fetched], NewSeen, NewState};
         false ->
@@ -234,7 +235,7 @@ maybe_lock(Profile, AppInfo, Seen, State, Level) ->
             {Seen, State}
     end.
 
-package_to_app(DepsDir, Packages, {Name, Vsn}) ->
+package_to_app(DepsDir, Packages, {Name, Vsn, Level}) ->
     case dict:find({Name, Vsn}, Packages) of
         error ->
             throw(?PRV_ERROR({missing_package, Name, Vsn}));
@@ -244,7 +245,8 @@ package_to_app(DepsDir, Packages, {Name, Vsn}) ->
             {ok, AppInfo} = rebar_app_info:new(Name, Vsn),
             AppInfo1 = rebar_app_info:deps(AppInfo, PkgDeps),
             AppInfo2 = rebar_app_info:dir(AppInfo1, rebar_dir:deps_dir(DepsDir, Name)),
-            rebar_app_info:source(AppInfo2, {pkg, Name, Vsn})
+            AppInfo3 = rebar_app_info:dep_level(AppInfo2, Level),
+            rebar_app_info:source(AppInfo3, {pkg, Name, Vsn})
     end.
 
 -spec update_src_deps(atom(), non_neg_integer(), list(), list(), list(), rebar_state:t(), boolean(), sets:set(binary()), list()) -> {rebar_state:t(), list(), list(), sets:set(binary())}.
@@ -432,6 +434,8 @@ parse_dep({Name, _Vsn, Source, Opts}, {SrcDepsAcc, PkgDepsAcc}, DepsDir, State)
     ?WARN("Dependency option list ~p in ~p is not supported and will be ignored", [Opts, Name]),
     Dep = new_dep(DepsDir, Name, [], Source, State),
     {[Dep | SrcDepsAcc], PkgDepsAcc};
+parse_dep({_Name, {pkg, Name, Vsn}, Level}, {SrcDepsAcc, PkgDepsAcc}, _, _) when is_integer(Level) ->
+    {SrcDepsAcc, [{Name, Vsn} | PkgDepsAcc]};
 parse_dep({Name, Source, Level}, {SrcDepsAcc, PkgDepsAcc}, DepsDir, State) when is_tuple(Source)
                                                                               , is_integer(Level) ->
     Dep = new_dep(DepsDir, Name, [], Source, State),

test/mock_pkg_resource.erl

@@ -47,11 +47,11 @@ mock_lock(_) ->
 %% @doc The config passed to the `mock/2' function can specify which apps
 %% should be updated on a per-name basis: `{update, ["App1", "App3"]}'.
 mock_update(Opts) ->
-    ToUpdate = proplists:get_value(update, Opts, []),
+    ToUpdate = proplists:get_value(upgrade, Opts, []),
     meck:expect(
         ?MOD, needs_update,
         fun(_Dir, {pkg, App, _Vsn}) ->
-            lists:member(App, ToUpdate)
+            lists:member(binary_to_list(App), ToUpdate)
         end).
 
 %% @doc Replicated an unsupported call.

test/rebar_test_utils.erl

@@ -205,6 +205,9 @@ check_results(AppDir, Expected) ->
                 case lists:keyfind(iolist_to_binary(Name), 1, Locks) of
                     false ->
                         error({lock_not_found, Name});
+                    {_LockName, {pkg, _, LockVsn}, _} ->
+                        ?assertEqual(iolist_to_binary(Vsn),
+                                     iolist_to_binary(LockVsn));
                     {_LockName, {_, _, {ref, LockVsn}}, _} ->
                         ?assertEqual(iolist_to_binary(Vsn),
                                      iolist_to_binary(LockVsn))

test/rebar_upgrade_SUITE.erl

@@ -3,7 +3,7 @@
 -include_lib("eunit/include/eunit.hrl").
 -compile(export_all).
 
-all() -> [{group, git}].%, {group, pkg}].
+all() -> [{group, git}, {group, pkg}].
 
 groups() ->
     [{all, [], [top_a, top_b, top_c, top_d1, top_d2, top_e,
@@ -38,7 +38,7 @@ init_per_testcase(Case, Config) ->
     UpExpanded = rebar_test_utils:expand_deps(DepsType, UpDeps),
     [{expected, normalize_unlocks(Expectations)},
      {mock, fun() -> mock_deps(DepsType, Expanded, []) end},
-     {mock_update, fun() -> mock_deps(DepsType, UpExpanded, ToUp) end}
+     {mock_update, fun() -> mock_deps(DepsType, Expanded, UpExpanded, ToUp) end}
      | setup_project(Case, Config, Expanded, UpExpanded)].
 
 end_per_testcase(_, Config) ->
@@ -206,7 +206,7 @@ upgrades(triplet_b) ->
                   {"G",[]}]},
       {"C", "0", [{"H","3",[]},
                   {"I",[]}]}],
-     [{"A", "1", [{"D",[]},
+     [{"A", "2", [{"D",[]},
                   {"E","2",[]}]},
       {"B", "1", [{"F","1",[]},
                   {"G",[]}]},
@@ -223,7 +223,7 @@ upgrades(triplet_c) ->
                   {"G",[]}]},
       {"C", "0", [{"H","3",[]},
                   {"I",[]}]}],
-     [{"A", "1", [{"D",[]},
+     [{"A", "2", [{"D",[]},
                   {"E","2",[]}]},
       {"B", "1", [{"F","1",[]},
                   {"G",[]}]},
@@ -245,7 +245,7 @@ upgrades(tree_a) ->
                   {"E",[{"I","1",[]}]}]},
       {"B", "1", [{"F",[]},
                   {"G",[]}]},
-      {"C", "1", [{"H",[]}]}
+      {"C", "2", [{"H",[]}]}
      ],
      ["C"],
      {"A", [{"A","1"}, "D", "J", "E",
@@ -263,7 +263,7 @@ upgrades(tree_b) ->
                   {"E",[{"I","1",[]}]}]},
       {"B", "1", [{"F",[]},
                   {"G",[]}]},
-      {"C", "1", [{"H",[]}]}
+      {"C", "2", [{"H",[]}]}
      ],
      ["C"],
      {"B", [{"A","1"}, "D", "J", "E",
@@ -363,6 +363,15 @@ mock_deps(pkg, Deps, Upgrades) ->
     catch mock_pkg_resource:unmock(),
     mock_pkg_resource:mock([{pkgdeps, rebar_test_utils:flat_pkgdeps(Deps)}, {upgrade, Upgrades}]).
 
+mock_deps(git, _OldDeps, Deps, Upgrades) ->
+    catch mock_git_resource:unmock(),
+    mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(Deps)}, {upgrade, Upgrades}]);
+mock_deps(pkg, OldDeps, Deps, Upgrades) ->
+    Merged = Deps ++ [Dep || Dep <- OldDeps,
+                             not lists:keymember(element(1, Dep), 1, Deps)],
+    catch mock_pkg_resource:unmock(),
+    mock_pkg_resource:mock([{pkgdeps, rebar_test_utils:flat_pkgdeps(Merged)}, {upgrade, Upgrades}]).
+
 normalize_unlocks({App, Locks}) ->
     {iolist_to_binary(App),
      normalize_unlocks_expect(Locks)};