Warnings for Deletions and friendly errors

Apps that are no longer used are not automatically deleted, but we tell
users it can be done. This is safer while we're not sure of the
correctness of these messages.

Error messages are added for transient dependencies and dependencies not
found.

src/rebar_prv_upgrade.erl

@@ -29,11 +29,14 @@ init(State) ->
                                                    {module, ?MODULE},
                                                    {bare, false},
                                                    {deps, ?DEPS},
-                                                   {example, "rebar upgrade cowboy[,ranch]"},
-                                                   {short_desc, "Upgrade dependency."},
-                                                   {desc, ""},
+                                                   {example, "rebar3 upgrade [cowboy[,ranch]]"},
+                                                   {short_desc, "Upgrade dependencies."},
+                                                   {desc, "Upgrade project dependecies. Mentioning no application "
+                                                          "will upgrade all dependencies. To upgrade specific dependencies, "
+                                                          "their names can be listed in the command."},
                                                    {opts, [
-                                                          {package, undefined, undefined, string, "Packages to upgrade."}
+                                                          {package, undefined, undefined, string,
+                                                           "List of packages to upgrade. If not specified, all dependencies are upgraded."}
                                                           ]}])),
     {ok, State1}.
 
@@ -42,11 +45,11 @@ do(State) ->
     {Args, _} = rebar_state:command_parsed_args(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),
+    Names = parse_names(ec_cnv:to_binary(proplists:get_value(package, Args, <<"">>)), Locks),
     case prepare_locks(Names, Deps, Locks, []) of
         {error, Reason} ->
             {error, Reason};
-        {Locks0, _Unlocks0} -> % unlocks may be useful for deletions
+        {Locks0, _Unlocks0} ->
             Deps0 = top_level_deps(Deps, Locks),
             State1 = rebar_state:set(State, {deps, default}, Deps0),
             State2 = rebar_state:set(State1, {locks, default}, Locks0),
@@ -54,6 +57,10 @@ do(State) ->
             Res = rebar_prv_install_deps:do(State3),
             case Res of
                 {ok, State4} ->
+                    info_useless(
+                        [element(1,Lock) || Lock <- Locks],
+                        [rebar_app_info:name(App) || App <- rebar_state:lock(State4)]
+                    ),
                     rebar_prv_lock:do(State4);
                 _ ->
                     Res
@@ -61,6 +68,12 @@ do(State) ->
     end.
 
 -spec format_error(any()) -> iolist().
+format_error({unknown_dependency, Name}) ->
+    io_lib:format("Dependency ~ts not found", [Name]);
+format_error({transitive_dependency, Name}) ->
+    io_lib:format("Dependency ~ts is transient and cannot be safely upgraded. "
+                  "Promote it to your top-level rebar.config file to upgrade it.",
+                  [Name]);
 format_error(Reason) ->
     io_lib:format("~p", [Reason]).
 
@@ -82,7 +95,7 @@ prepare_locks([Name|Names], Deps, Locks, Unlocks) ->
             AtomName = binary_to_atom(Name, utf8),
             case lists:keyfind(AtomName, 1, Deps) of
                 false ->
-                    {error, {unknown_dependency, Name}};
+                    {error, {?MODULE, {unknown_dependency, Name}}};
                 Dep ->
                     Source = case Dep of
                         {_, Src} -> Src;
@@ -90,15 +103,14 @@ prepare_locks([Name|Names], Deps, Locks, Unlocks) ->
                     end,
                     {NewLocks, NewUnlocks} = unlock_higher_than(0, Locks -- [Lock]),
                     prepare_locks(Names,
-                                  %deps_like_locks(Deps, [{Name,Source,0} | NewLocks]),
                                   Deps,
                                   NewLocks,
                                   [{Name, Source, 0} | NewUnlocks ++ Unlocks])
             end;
         {_, _, Level} when Level > 0 ->
-            {error, {transitive_dependency,Name}};
+            {error, {?MODULE, {transitive_dependency,Name}}};
         false ->
-            {error, {unknown_dependency,Name}}
+            {error, {?MODULE, {unknown_dependency,Name}}}
     end.
 
 top_level_deps(Deps, Locks) ->
@@ -113,3 +125,9 @@ unlock_higher_than(Level, [App = {_,_,AppLevel} | Apps], Locks, Unlocks) ->
        AppLevel =< Level -> unlock_higher_than(Level, Apps, [App | Locks], Unlocks)
     end.
 
+info_useless(Old, New) ->
+    [?INFO("App ~ts is no longer needed and can be deleted.", [Name])
+     || Name <- Old,
+        not lists:member(Name, New)],
+    ok.
+

test/mock_git_resource.erl

@@ -60,7 +60,7 @@ mock_update(Opts) ->
         ?MOD, needs_update,
         fun(_Dir, {git, Url, _Ref}) ->
             App = app(Url),
-            ct:pal("Needed update? ~p (~p) -> ~p", [App, {Url,_Ref}, lists:member(App, ToUpdate)]),
+%            ct:pal("Needed update? ~p (~p) -> ~p", [App, {Url,_Ref}, lists:member(App, ToUpdate)]),
             lists:member(App, ToUpdate)
         end).
 

test/rebar_upgrade_SUITE.erl

@@ -9,7 +9,8 @@ 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_ac, tree_all,
+                delete_d]},
      {git, [], [{group, all}]},
      {pkg, [], [{group, all}]}].
 
@@ -83,7 +84,7 @@ upgrades(top_b) ->
      %% Modified apps, gobally
      ["A","B","D"],
      %% upgrade vs. new tree
-     {"B", {error, {transitive_dependency, <<"B">>}}}};
+     {"B", {error, {rebar_prv_upgrade, {transitive_dependency, <<"B">>}}}}};
 upgrades(top_c) ->
      %% Original tree
     {[{"A", "1", [{"B", [{"D", "1", []}]},
@@ -96,7 +97,7 @@ upgrades(top_c) ->
      %% Modified apps, gobally
      ["A","B","D"],
      %% upgrade vs. new tree
-     {"C", {error, {transitive_dependency, <<"C">>}}}};
+     {"C", {error, {rebar_prv_upgrade, {transitive_dependency, <<"C">>}}}}};
 upgrades(top_d1) ->
      %% Original tree
     {[{"A", "1", [{"B", [{"D", "1", []}]},
@@ -109,7 +110,7 @@ upgrades(top_d1) ->
      %% Modified apps, gobally
      ["A","B","D"],
      %% upgrade vs. new tree
-     {"D", {error, {transitive_dependency, <<"D">>}}}};
+     {"D", {error, {rebar_prv_upgrade, {transitive_dependency, <<"D">>}}}}};
 upgrades(top_d2) ->
      %% Original tree
     {[{"A", "1", [{"B", [{"D", "1", []}]},
@@ -122,7 +123,7 @@ upgrades(top_d2) ->
      %% Modified apps, gobally
      ["A","B","D"],
      %% upgrade vs. new tree
-     {"D", {error, {transitive_dependency, <<"D">>}}}};
+     {"D", {error, {rebar_prv_upgrade, {transitive_dependency, <<"D">>}}}}};
 upgrades(top_e) ->
      %% Original tree
     {[{"A", "1", [{"B", [{"D", "1", []}]},
@@ -135,7 +136,7 @@ upgrades(top_e) ->
      %% Modified apps, gobally
      ["A","B","D"],
      %% upgrade vs. new tree
-     {"E", {error, {unknown_dependency, <<"E">>}}}};
+     {"E", {error, {rebar_prv_upgrade, {unknown_dependency, <<"E">>}}}}};
 upgrades(pair_a) ->
     {[{"A", "1", [{"C", "1", []}]},
       {"B", "1", [{"D", "1", []}]}
@@ -171,7 +172,7 @@ upgrades(pair_c) ->
       {"B", "2", [{"D", "2", []}]}
      ],
      ["A","B","C","D"],
-     {"C", {error, {transitive_dependency, <<"C">>}}}};
+     {"C", {error, {rebar_prv_upgrade, {transitive_dependency, <<"C">>}}}}};
 upgrades(pair_all) ->
     {[{"A", "1", [{"C", "1", []}]},
       {"B", "1", [{"D", "1", []}]}
@@ -340,7 +341,17 @@ upgrades(tree_all) ->
      ["C","I"],
      {"", [{"A","1"}, "D", "J", "E", {"I","1"},
            {"B","1"}, "F", "G",
-           {"C","1"}, "H"]}}.
+           {"C","1"}, "H"]}};
+upgrades(delete_d) ->
+    {[{"A", "1", [{"B", [{"D", "1", []}]},
+                  {"C", [{"D", "2", []}]}]}
+     ],
+     [{"A", "2", [{"B", []},
+                  {"C", []}]}
+     ],
+     ["A","B", "C"],
+     %% upgrade vs. new tree
+     {"", [{"A","2"}, "B", "C"]}}.
 
 %% TODO: add a test that verifies that unlocking files and then
 %% running the upgrade code is enough to properly upgrade things.
@@ -353,7 +364,6 @@ 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(),
@@ -435,6 +445,15 @@ tree_c2(Config) -> run(Config).
 tree_ac(Config) -> run(Config).
 tree_all(Config) -> run(Config).
 
+delete_d(Config) ->
+    meck:new(rebar_log, [no_link, passthrough]),
+    run(Config),
+    Infos = [{Str, Args}
+            || {_, {rebar_log, log, [info, Str, Args]}, _} <- meck:history(rebar_log)],
+    meck:unload(rebar_log),
+    ?assertNotEqual([],
+                    [1 || {"App ~ts is no longer needed and can be deleted.",
+                           [<<"D">>]} <- Infos]).
 run(Config) ->
     apply(?config(mock, Config), []),
     {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),