More progress on upgrades

Only the most complex case is failing, where cross-dependencies would
need to be refetched as an update clears an app of its dependencies and
a different subtree should override it.

src/rebar_prv_install_deps.erl

@@ -44,6 +44,8 @@
 -define(PROVIDER, install_deps).
 -define(DEPS, [app_discovery]).
 
+-define(APP_NAME_INDEX, 2).
+
 -type src_dep() :: {atom(), {atom(), string(), string()}}
              | {atom(), string(), {atom(), string(), string()}}.
 -type pkg_dep() :: {atom(), binary()} | atom().
@@ -135,7 +137,8 @@ handle_deps(Profile, State, Deps) ->
 handle_deps(Profile, State, Deps, Update) when is_boolean(Update) ->
     handle_deps(Profile, State, Deps, Update, []);
 handle_deps(Profile, State, Deps, Locks) when is_list(Locks) ->
-    handle_deps(Profile, State, Deps, false, Locks).
+    Update = rebar_state:get(State, upgrade, false),
+    handle_deps(Profile, State, Deps, Update, Locks).
 
 -spec handle_deps(atom(), rebar_state:t(), list(), boolean() | {true, binary(), integer()}, list())
                  -> {ok, [rebar_app_info:t()], rebar_state:t()} | {error, string()}.
@@ -277,7 +280,7 @@ update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen,
                              end
                      end,
                      {[], PkgDeps, SrcApps, State, Seen, Locks},
-                     lists:sort(SrcDeps)) of
+                     sort_deps(SrcDeps)) of
         {[], NewPkgDeps, NewSrcApps, State1, Seen1, _NewLocks} ->
             {State1, NewSrcApps, NewPkgDeps, Seen1};
         {NewSrcDeps, NewPkgDeps, NewSrcApps, State1, Seen1, NewLocks} ->
@@ -286,13 +289,10 @@ update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen,
 
 handle_update(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) ->
     Name = rebar_app_info:name(AppInfo),
-    ct:pal("update ~p", [Name]),
     case lists:keyfind(Name, 1, Locks) of
         false ->
-            ct:pal("in lock"),
-            case maybe_fetch(AppInfo, true, []) of
+            case maybe_fetch(AppInfo, true, sets:new()) of
                 true ->
-                    ct:pal("fetch!"),
                     handle_dep(AppInfo
                               ,SrcDeps
                               ,PkgDeps
@@ -302,11 +302,9 @@ handle_update(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) ->
                               ,Locks);
 
                 false ->
-                    ct:pal("nofetch"),
                     {SrcDeps, PkgDeps, SrcApps, State, Locks}
             end;
         _StillLocked ->
-            ct:pal("stillocked"),
             {SrcDeps, PkgDeps, SrcApps, State, Locks}
     end.
 
@@ -365,9 +363,9 @@ maybe_fetch(AppInfo, Update, Seen) ->
                              end
                      end,
 
-            case not Exists orelse Update of
+            case not Exists of% orelse Update of
                 true ->
-                    ?INFO("Fetching ~s", [rebar_app_info:name(AppInfo)]),
+                    ?INFO("Fetching ~s (~p)", [rebar_app_info:name(AppInfo), rebar_app_info:source(AppInfo)]),
                     Source = rebar_app_info:source(AppInfo),
                     case rebar_fetch:download_source(AppDir, Source) of
                         {error, Reason} ->
@@ -381,7 +379,7 @@ maybe_fetch(AppInfo, Update, Seen) ->
                             false;
                         false ->
                             Source = rebar_app_info:source(AppInfo),
-                            case rebar_fetch:needs_update(AppDir, Source) of
+                            case Update andalso rebar_fetch:needs_update(AppDir, Source) of
                                 true ->
                                     ?INFO("Updating ~s", [rebar_app_info:name(AppInfo)]),
                                     case rebar_fetch:download_source(AppDir, Source) of
@@ -462,6 +460,14 @@ new_dep(DepsDir, Name, Vsn, Source, State) ->
                                rebar_state:overrides(S, ParentOverrides++Overrides)),
     rebar_app_info:source(Dep1, Source).
 
+sort_deps(Deps) ->
+    %% We need a sort stable, based on the name. So that for multiple deps on
+    %% the same level with the same name, we keep the order the parents had.
+    %% `lists:keysort/2' is documented as stable in the stdlib.
+    %% The list of deps is revered when we get it. For the proper stable
+    %% result, re-reverse it.
+    lists:keysort(?APP_NAME_INDEX, lists:reverse(Deps)).
+
 -spec parse_goal(binary(), binary()) -> pkg_dep().
 parse_goal(Name, Constraint) ->
     case re:run(Constraint, "([^\\d]*)(\\d.*)", [{capture, [1,2], binary}]) of

src/rebar_prv_upgrade.erl

@@ -56,12 +56,12 @@ do(State) ->
                     NewLocks = unlock_higher_than(0, Locks -- [Lock]),
                     State1 = rebar_state:set(State, {deps, default}, [{Name, Source, 0} | NewLocks]),
                     State2 = rebar_state:set(State1, {locks, default}, NewLocks),
-                    rebar_prv_install_deps:do(State2)
+                    State3 = rebar_state:set(State2, upgrade, true),
+                    rebar_prv_install_deps:do(State3)
             end;
         {_, _, Level} when Level > 0 ->
             {error, transitive_dependency};
         false ->
-            ct:pal("deps: ~p", [{Name,Locks}]),
             {error, unknown_dependency}
     end.
 

test/rebar_upgrade_SUITE.erl

@@ -38,13 +38,13 @@ init_per_testcase(Case, Config) ->
     [{expected, normalize_unlocks(Expectations)},
      {mock, fun() -> mock_deps(DepsType, Expanded, []) end},
      {mock_update, fun() -> mock_deps(DepsType, UpExpanded, ToUp) end}
-     | setup_project(Case, Config, Expanded)].
+     | setup_project(Case, Config, Expanded, UpExpanded)].
 
 end_per_testcase(_, Config) ->
     meck:unload(),
     Config.
 
-setup_project(Case, Config0, Deps) ->
+setup_project(Case, Config0, Deps, UpDeps) ->
     DepsType = ?config(deps_type, Config0),
     Config = rebar_test_utils:init_rebar_state(
             Config0,
@@ -54,7 +54,8 @@ setup_project(Case, Config0, Deps) ->
     rebar_test_utils:create_app(AppDir, "Root", "0.0.0", [kernel, stdlib]),
     TopDeps = top_level_deps(Deps),
     RebarConf = rebar_test_utils:create_config(AppDir, [{deps, TopDeps}]),
-    [{rebarconfig, RebarConf} | Config].
+    [{rebarconfig, RebarConf},
+     {next_top_deps, top_level_deps(UpDeps)} | Config].
 
 
 upgrades(top_a) ->
@@ -279,6 +280,7 @@ top_level_deps([{{pkg, Name, Vsn, _URL}, _} | Deps]) ->
 
 mock_deps(git, Deps, Upgrades) ->
     catch mock_git_resource:unmock(),
+    ct:pal("mocked: ~p", [flat_deps(Deps)]),
     mock_git_resource:mock([{deps, flat_deps(Deps)}, {upgrade, Upgrades}]);
 mock_deps(pkg, Deps, Upgrades) ->
     catch mock_pkg_resource:unmock(),
@@ -365,7 +367,10 @@ run(Config) ->
         _ -> {ok, Unlocks}
     end,
     apply(?config(mock_update, Config), []),
+    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, RebarConfig, ["upgrade", App], Expectation
+        Config, NewRebarConfig, ["upgrade", App], Expectation
     ).