From 2e34e91dfb54218597f72faac3a39aedd57bcf97 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Wed, 11 Feb 2015 21:00:37 +0000 Subject: 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 | 38 ++++++++++++++++++++++++++++---------- test/mock_git_resource.erl | 2 +- test/rebar_upgrade_SUITE.erl | 37 ++++++++++++++++++++++++++++--------- 3 files changed, 57 insertions(+), 20 deletions(-) diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl index 5ee2be6..1cae5aa 100644 --- a/src/rebar_prv_upgrade.erl +++ b/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. + diff --git a/test/mock_git_resource.erl b/test/mock_git_resource.erl index 2f7a72d..d2f0207 100644 --- a/test/mock_git_resource.erl +++ b/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). diff --git a/test/rebar_upgrade_SUITE.erl b/test/rebar_upgrade_SUITE.erl index e0bb011..39b9687 100644 --- a/test/rebar_upgrade_SUITE.erl +++ b/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)), -- cgit v1.1