Merge pull request #49 from rebar/skipped-deps-warnings

Print warning when deps are being skipped.

2 files changed, 71 insertions, 13 deletions

diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl
index 1ddfdf5..5878e8c 100644
--- a/src/rebar_prv_install_deps.erl
+++ b/src/rebar_prv_install_deps.erl
@@ -207,6 +207,7 @@ update_src_deps(Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen) ->
                              %% If not seen, add to list of locks to write out
                              case sets:is_element(rebar_app_info:name(AppInfo), SeenAcc) of
                                  true ->
+                                     warn_skip_deps(AppInfo),
                                      {SrcDepsAcc, PkgDepsAcc, SrcAppsAcc, StateAcc, SeenAcc};
                                  false ->
                                      {SeenAcc1, StateAcc1} = maybe_lock(AppInfo, SeenAcc, StateAcc),
@@ -420,3 +421,9 @@ info(Description) ->
                    {app_name, ".*", {fossil, "https://www.example.org/url", "Vsn"}},
                    {app_name, ".*", {p4, "//depot/subdir/app_dir"}}]}
                  ]).
+
+warn_skip_deps(AppInfo) ->
+    ?WARN("Skipping ~s (from ~p) as an app of the same name "
+          "has already been fetched~n",
+          [rebar_app_info:name(AppInfo),
+           rebar_app_info:source(AppInfo)]).
diff --git a/test/rebar_deps_SUITE.erl b/test/rebar_deps_SUITE.erl
index 656c3b5..ccc5fcf 100644
--- a/test/rebar_deps_SUITE.erl
+++ b/test/rebar_deps_SUITE.erl
@@ -5,7 +5,7 @@
 -include_lib("eunit/include/eunit.hrl").
 
 all() -> [flat, pick_highest_left, pick_highest_right, pick_earliest,
-          circular1, circular2].
+          circular1, circular2, circular_skip].
 
 init_per_suite(Config) ->
     application:start(meck),
@@ -15,12 +15,19 @@ end_per_suite(_Config) ->
     application:stop(meck).
 
 init_per_testcase(Case, Config) ->
-    {Deps, Expect} = deps(Case),
+    {Deps, Warnings, Expect} = deps(Case),
     Expected = case Expect of
         {ok, List} -> {ok, format_expected_deps(List)};
         {error, Reason} -> {error, Reason}
     end,
-    [{expect, Expected} | setup_project(Case, Config, expand_deps(Deps))].
+    mock_warnings(),
+    [{expect, Expected},
+     {warnings, Warnings} | setup_project(Case, Config, expand_deps(Deps))].
+
+end_per_testcase(_, Config) ->
+    meck:unload(),
+    Config.
+
 
 format_expected_deps(Deps) ->
     [case Dep of
@@ -28,35 +35,56 @@ format_expected_deps(Deps) ->
         N -> {dep, N}
      end || Dep <- Deps].
 
+%% format:
+%% {Spec,
+%%  [Warning],
+%%  {ok, Result} | {error, Reason}}
+%%
+%% Spec is a list of levelled dependencies of two possible forms:
+%% - {"Name", Spec}
+%% - {"Name", "Vsn", Spec}
+%%
+%% Warnings are going to match on mocked ?WARN(...)
+%% calls to be evaluated. An empty list means we do not care about
+%% warnings, not that no warnings will be printed. This means
+%% the list of warning isn't interpreted to be exhaustive, and more
+%% warnings may be generated than are listed.
 deps(flat) ->
     {[{"B", []},
       {"C", []}],
+     [],
      {ok, ["B", "C"]}};
 deps(pick_highest_left) ->
     {[{"B", [{"C", "2", []}]},
       {"C", "1", []}],
-     {ok, ["B", {"C", "1"}]}}; % Warn C2
+     [{"C","2"}],
+     {ok, ["B", {"C", "1"}]}};
 deps(pick_highest_right) ->
     {[{"B", "1", []},
       {"C", [{"B", "2", []}]}],
-     {ok, [{"B","1"}, "C"]}}; % Warn B2
+     [{"B","2"}],
+     {ok, [{"B","1"}, "C"]}};
 deps(pick_earliest) ->
     {[{"B", [{"D", "1", []}]},
       {"C", [{"D", "2", []}]}],
-     {ok, ["B","C",{"D","1"}]}}; % Warn D2
+     [{"D","2"}],
+     {ok, ["B","C",{"D","1"}]}};
 deps(circular1) ->
     {[{"B", [{"A", []}]}, % A is the top-level app
       {"C", []}],
+     [],
      {error, {rebar_prv_install_deps, {cycles, [[<<"A">>,<<"B">>]]}}}};
 deps(circular2) ->
     {[{"B", [{"C", [{"B", []}]}]},
       {"C", []}],
-     {error, {rebar_prv_install_deps, {cycles, [[<<"B">>,<<"C">>]]}}}}.
-
-end_per_testcase(_, Config) ->
-    mock_git_resource:unmock(),
-    meck:unload(),
-    Config.
+     [],
+     {error, {rebar_prv_install_deps, {cycles, [[<<"B">>,<<"C">>]]}}}};
+deps(circular_skip) ->
+    %% Never spot the circular dep due to being to low in the deps tree.
+    {[{"B", [{"C", "2", [{"D", []}]}]},
+      {"C", "1", [{"B",[]}]}],
+     [{"C","2"}],
+     {ok, ["B", {"C","1"}, "D"]}}.
 
 expand_deps([]) -> [];
 expand_deps([{Name, Deps} | Rest]) ->
@@ -86,6 +114,11 @@ flat_deps([{{Name,_Vsn,_Ref}, Deps} | Rest]) ->
 
 top_level_deps(Deps) -> [{list_to_atom(Name),Vsn,Ref} || {{Name,Vsn,Ref},_} <- Deps].
 
+mock_warnings() ->
+    %% just let it do its thing, we check warnings through
+    %% the call log.
+    meck:new(rebar_log, [no_link, passthrough]).
+
 %%% TESTS %%%
 flat(Config) -> run(Config).
 pick_highest_left(Config) -> run(Config).
@@ -93,10 +126,28 @@ pick_highest_right(Config) -> run(Config).
 pick_earliest(Config) -> run(Config).
 circular1(Config) -> run(Config).
 circular2(Config) -> run(Config).
+circular_skip(Config) -> run(Config).
 
 run(Config) ->
     {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),
     rebar_test_utils:run_and_check(
         Config, RebarConfig, "install_deps", ?config(expect, Config)
-    ).
+    ),
+    check_warnings(warning_calls(), ?config(warnings, Config)).
+
+warning_calls() ->
+    History = meck:history(rebar_log),
+    [{Str, Args} || {_, {rebar_log, log, [warn, Str, Args]}, _} <- History].
+
+check_warnings(_, []) ->
+    ok;
+check_warnings(Warns, [{Name, Vsn} | Rest]) ->
+    ct:pal("Checking for warning ~p in ~p", [{Name,Vsn},Warns]),
+    ?assert(in_warnings(Warns, Name, Vsn)),
+    check_warnings(Warns, Rest).
+
+in_warnings(Warns, NameRaw, VsnRaw) ->
+    Name = iolist_to_binary(NameRaw),
+    1 =< length([1 || {_, [AppName, {git, _, {_, Vsn}}]} <- Warns,
+                      AppName =:= Name, Vsn =:= VsnRaw]).