Merge pull request #724 from tsloughter/upgrade_with_locks

properly update locks for deps and ignore skip messages for locks

src/rebar_digraph.erl

@@ -77,7 +77,7 @@ cull_deps(Graph, Vertices) ->
 
 cull_deps(Graph, Vertices, Seen) ->
     Vertices1 = lists:keysort(2, Vertices),
-    {Solution, Levels, Discarded} = {dict:new(), dict:new(), []},
+    {Solution, Levels, Discarded} = {dict:new(), dict:new(), sets:new()},
     cull_deps(Graph, Vertices1, Levels, Solution, Seen, Discarded).
 
 format_error(no_solution) ->
@@ -91,7 +91,7 @@ cull_deps(_Graph, [], Levels, Solution, _, Discarded) ->
     {_, Vertices} = lists:unzip(dict:to_list(Solution)),
     LvlVertices = [{Profile, {Parent, App, Vsn, dict:fetch(App, Levels)}}
                   || {Profile, {Parent,App,Vsn}} <- Vertices],
-    {ok, LvlVertices, Discarded};
+    {ok, LvlVertices,  sets:to_list(Discarded)};
 cull_deps(Graph, [{Profile, Level, Vs} | Vertices], Levels, Solution, Seen, Discarded) ->
     {NV, NS, LS, DS} =
         lists:foldl(fun({Parent, Name, Vsn}, {Acc, SolutionAcc, LevelsAcc, DiscardedAcc}) ->
@@ -134,7 +134,7 @@ handle_neighbors(Profile, Level, Parent, OutNeighbors, Vertices
                                  {ok, _} -> % conflict resolution!
                                      %% Warn on different version
                                      {NewVertices,
-                                      [Value|Discarded1]};
+                                      sets:add_element(Value, Discarded1)};
                                  error ->
                                      %% We check Seen separately because we don't care
                                      %% to warn if the exact same version of a package
@@ -143,7 +143,7 @@ handle_neighbors(Profile, Level, Parent, OutNeighbors, Vertices
                                      case sets:is_element(Name, Seen) of
                                          true ->
                                              {NewVertices,
-                                              [Value|Discarded1]};
+                                              sets:add_element(Value, Discarded1)};
                                          false ->
                                              {[{Parent, Name, Vsn} | NewVertices],
                                               Discarded1}
@@ -167,7 +167,7 @@ maybe_add_to_solution(Profile, Level, Key, {Name, Vsn}=Value, Parent
             %% Warn on different version
             {Solution,
              Levels,
-             [Value|Discarded]};
+             sets:add_element(Value, Discarded)};
         error ->
             %% We check Seen separately because we don't care to warn if the exact
             %% same version of a package was already part of the solution but we do
@@ -176,7 +176,7 @@ maybe_add_to_solution(Profile, Level, Key, {Name, Vsn}=Value, Parent
                 true ->
                     {Solution,
                      Levels,
-                     [Value|Discarded]};
+                     sets:add_element(Value, Discarded)};
                 false ->
                     {dict:store(Key, {Profile, {Parent, Name, Vsn}}, Solution),
                      dict:store(Key, Level, Levels),

src/rebar_prv_install_deps.erl

@@ -194,12 +194,16 @@ handle_profile_pkg_level(PkgDeps, AllApps, Seen, Upgrade, Locks, State) ->
             {ok, [], []} ->
                 throw({rebar_digraph, no_solution});
             {ok, [], Discarded} ->
-                [warn_skip_pkg(Pkg, State) || Pkg <- Discarded, not(pkg_locked(Pkg, Locks))],
+                [warn_skip_pkg(Pkg, State) || Upgrade =:= false,
+                                              Pkg <- Discarded,
+                                              not(pkg_locked(Pkg, Locks))],
                 [];
             {ok, Solution, []} ->
                 Solution;
             {ok, Solution, Discarded} ->
-                [warn_skip_pkg(Pkg, State) || Pkg <- Discarded, not(pkg_locked(Pkg, Locks))],
+                [warn_skip_pkg(Pkg, State) || Upgrade =:= false,
+                                              Pkg <- Discarded,
+                                              not(pkg_locked(Pkg, Locks))],
                 Solution
         end,
 
@@ -218,7 +222,7 @@ cull_compile(TopSortedDeps, ProjectApps) ->
 
 pkg_locked({_, Name, _, _}, Locks) ->
     pkg_locked(Name, Locks);
-pkg_locked({_, Name, _}, Locks) ->
+pkg_locked({Name, _}, Locks) ->
     pkg_locked(Name, Locks);
 pkg_locked(Name, Locks) ->
     false =/= lists:keyfind(Name, 1, Locks).
@@ -338,30 +342,12 @@ update_seen_src_dep(AppInfo, _Profile, _Level, SrcDeps, PkgDeps, SrcApps, State,
 
 update_unseen_src_dep(AppInfo, Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrade, Seen, Locks) ->
     {NewSeen, State1} = maybe_lock(Profile, AppInfo, Seen, State, Level),
-    {NewSrcDeps, NewPkgDeps, NewSrcApps, State2, NewLocks}
-        = case Upgrade of
-              true ->
-                  handle_upgrade(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps,
-                                 Level, State1, Seen, Locks);
-              _ ->
-                  {_, AppInfo1} = maybe_fetch(AppInfo, Profile, false, Seen, State1),
-                  handle_dep(AppInfo1, Profile, SrcDeps, PkgDeps, SrcApps,
-                             Level, State1, Locks)
-          end,
+    {_, AppInfo1} = maybe_fetch(AppInfo, Profile, Upgrade, Seen, State1),
+    {NewSrcDeps, NewPkgDeps, NewSrcApps, State2, NewLocks} =
+        handle_dep(AppInfo1, Profile, SrcDeps, PkgDeps, SrcApps,
+                   Level, State1, Locks),
     {NewSrcDeps, NewPkgDeps, NewSrcApps, State2, NewSeen, NewLocks}.
 
-handle_upgrade(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps, Level, State, Seen, Locks) ->
-    Name = rebar_app_info:name(AppInfo),
-    case lists:keyfind(Name, 1, Locks) of
-        false ->
-            {_, AppInfo1} = maybe_fetch(AppInfo, Profile, true, Seen, State),
-            handle_dep(AppInfo1, Profile, SrcDeps, PkgDeps, SrcApps,
-                       Level, State, Locks);
-        _StillLocked ->
-            handle_dep(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps,
-                       Level, State, Locks)
-    end.
-
 handle_dep(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) ->
     DepsDir = profile_dep_dir(State, Profile),
     {AppInfo1, NewSrcDeps, NewPkgDeps, NewLocks, State1} =
@@ -399,11 +385,12 @@ handle_dep(State, Profile, DepsDir, AppInfo, Locks, Level) ->
 
     %% Upgrade lock level to be the level the dep will have in this dep tree
     Deps = rebar_state:get(S5, {deps, default}, []),
-    NewLocks = [{DepName, Source, LockLevel+Level} ||
-                   {DepName, Source, LockLevel} <- rebar_state:get(S5, {locks, default}, [])],
+    NewLocks = Locks++[{DepName, Source, LockLevel+Level} ||
+                          {DepName, Source, LockLevel} <- rebar_state:get(S5, {locks, default}, [])],
     AppInfo3 = rebar_app_info:deps(AppInfo2, rebar_state:deps_names(Deps)),
-    {SrcDeps, PkgDeps} = parse_deps(rebar_app_info:name(AppInfo3), DepsDir, Deps, S5, Locks, Level+1),
-    {AppInfo3, SrcDeps, PkgDeps, Locks++NewLocks, State}.
+    {SrcDeps, PkgDeps} = parse_deps(rebar_app_info:name(AppInfo3), DepsDir, Deps
+                                   ,S5, NewLocks, Level+1),
+    {AppInfo3, SrcDeps, PkgDeps, NewLocks, State}.
 
 -spec maybe_fetch(rebar_app_info:t(), atom(), boolean(),
                   sets:set(binary()), rebar_state:t()) -> {boolean(), rebar_app_info:t()}.