Merge pull request #269 from ferd/dep-conflict-as-error

Dep conflict as error

4 files changed, 77 insertions, 23 deletions

diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl
index 63cf703..0799313 100644
--- a/src/rebar_app_discover.erl
+++ b/src/rebar_app_discover.erl
@@ -48,7 +48,9 @@ merge_deps(AppInfo, State) ->
     State1 = lists:foldl(fun(Profile, StateAcc) ->
                                  AppProfDeps = rebar_state:get(AppState, {deps, Profile}, []),
                                  TopLevelProfDeps = rebar_state:get(StateAcc, {deps, Profile}, []),
-                                 ProfDeps2 = lists:keymerge(1, TopLevelProfDeps, AppProfDeps),
+                                 ProfDeps2 = dedup(lists:keymerge(1,
+                                                                  lists:keysort(1, TopLevelProfDeps),
+                                                                  lists:keysort(1, AppProfDeps))),
                                  rebar_state:set(StateAcc, {deps, Profile}, ProfDeps2)
                          end, State, lists:reverse(CurrentProfiles)),
 
@@ -166,3 +168,8 @@ create_app_info(AppDir, AppFile) ->
         _ ->
             error
     end.
+
+dedup([]) -> [];
+dedup([A]) -> [A];
+dedup([H,H|T]) -> dedup([H|T]);
+dedup([H|T]) -> [H|dedup(T)].
diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl
index 1c269fb..48402da 100644
--- a/src/rebar_prv_install_deps.erl
+++ b/src/rebar_prv_install_deps.erl
@@ -189,7 +189,7 @@ update_pkg_deps(Profile, Packages, PkgDeps, Graph, Upgrade, Seen, State) ->
                 {ok, Solution, []} ->
                     Solution;
                 {ok, Solution, Discarded} ->
-                    [warn_skip_pkg(Pkg) || Pkg <- Discarded],
+                    [warn_skip_pkg(Pkg, State) || Pkg <- Discarded],
                     Solution
             end,
             update_pkg_deps(Profile, S, Packages, Upgrade, Seen, State)
@@ -287,7 +287,7 @@ update_seen_src_dep(AppInfo, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrade, S
     %% If seen from lock file don't print warning about skipping
     case lists:keymember(Name, 1, BaseLocks) of
         false ->
-            warn_skip_deps(AppInfo);
+            warn_skip_deps(AppInfo, State);
         true ->
             ok
     end,
@@ -513,13 +513,21 @@ parse_goal(Name, Constraint) ->
             fail
     end.
 
-warn_skip_deps(AppInfo) ->
-    ?WARN("Skipping ~s (from ~p) as an app of the same name "
+warn_skip_deps(AppInfo, State) ->
+    Msg = "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)]).
+    Args = [rebar_app_info:name(AppInfo),
+            rebar_app_info:source(AppInfo)],
+    case rebar_state:get(State, deps_error_on_conflict, false) of
+        false -> ?WARN(Msg, Args);
+        true -> ?ERROR(Msg, Args), ?FAIL
+    end.
 
-warn_skip_pkg({Name, Source}) ->
-    ?WARN("Skipping ~s (version ~s from package index) as an app of the same "
+warn_skip_pkg({Name, Source}, State) ->
+    Msg = "Skipping ~s (version ~s from package index) as an app of the same "
           "name has already been fetched~n",
-          [Name, Source]).
+    Args = [Name, Source],
+    case rebar_state:get(State, deps_error_on_conflict, false) of
+        false -> ?WARN(Msg, Args);
+        true -> ?ERROR(Msg, Args), ?FAIL
+    end.
diff --git a/test/rebar_install_deps_SUITE.erl b/test/rebar_install_deps_SUITE.erl
index 28cc277..aebd3e3 100644
--- a/test/rebar_install_deps_SUITE.erl
+++ b/test/rebar_install_deps_SUITE.erl
@@ -8,7 +8,8 @@ all() -> [{group, git}, {group, pkg}].
 groups() ->
     [{all, [], [flat, pick_highest_left, pick_highest_right,
                 pick_smallest1, pick_smallest2,
-                circular1, circular2, circular_skip]},
+                circular1, circular2, circular_skip,
+                fail_conflict]},
      {git, [], [{group, all}]},
      {pkg, [], [{group, all}]}].
 
@@ -33,7 +34,7 @@ init_per_testcase(Case, Config) ->
     {Deps, Warnings, Expect} = deps(Case),
     Expected = case Expect of
         {ok, List} -> {ok, format_expected_deps(List)};
-        {error, Reason} -> {error, Reason}
+        Other -> Other
     end,
     DepsType = ?config(deps_type, Config),
     mock_warnings(),
@@ -108,13 +109,36 @@ deps(circular_skip) ->
     {[{"B", [{"C", "2", [{"B", []}]}]},
       {"C", "1", [{"D",[]}]}],
      [{"C","2"}],
-     {ok, ["B", {"C","1"}, "D"]}}.
+     {ok, ["B", {"C","1"}, "D"]}};
+deps(fail_conflict) ->
+    {[{"B", [{"C", "2", []}]},
+      {"C", "1", []}],
+     [{"C","2"}],
+     rebar_abort}.
 
+setup_project(fail_conflict, Config0, Deps) ->
+    DepsType = ?config(deps_type, Config0),
+    Config = rebar_test_utils:init_rebar_state(
+            Config0,
+            "fail_conflict_"++atom_to_list(DepsType)++"_"
+    ),
+    AppDir = ?config(apps, Config),
+    rebar_test_utils:create_app(AppDir, "A", "0.0.0", [kernel, stdlib]),
+    TopDeps = rebar_test_utils:top_level_deps(Deps),
+    RebarConf = rebar_test_utils:create_config(AppDir, [{deps, TopDeps},
+                                                        {deps_error_on_conflict, true}]),
+    case DepsType of
+        git ->
+            mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(Deps)}]);
+        pkg ->
+            mock_pkg_resource:mock([{pkgdeps, rebar_test_utils:flat_pkgdeps(Deps)}])
+    end,
+    [{rebarconfig, RebarConf} | Config];
 setup_project(Case, Config0, Deps) ->
     DepsType = ?config(deps_type, Config0),
     Config = rebar_test_utils:init_rebar_state(
             Config0,
-            atom_to_list(Case)++"_"++atom_to_list(DepsType)++"_"
+            atom_to_list(Case)++"_installdeps_"++atom_to_list(DepsType)++"_"
     ),
     AppDir = ?config(apps, Config),
     rebar_test_utils:create_app(AppDir, "A", "0.0.0", [kernel, stdlib]),
@@ -143,6 +167,13 @@ circular1(Config) -> run(Config).
 circular2(Config) -> run(Config).
 circular_skip(Config) -> run(Config).
 
+fail_conflict(Config) ->
+    {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),
+    rebar_test_utils:run_and_check(
+        Config, RebarConfig, ["install_deps"], ?config(expect, Config)
+    ),
+    check_warnings(error_calls(), ?config(warnings, Config), ?config(deps_type, Config)).
+
 run(Config) ->
     {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),
     rebar_test_utils:run_and_check(
@@ -154,6 +185,10 @@ warning_calls() ->
     History = meck:history(rebar_log),
     [{Str, Args} || {_, {rebar_log, log, [warn, Str, Args]}, _} <- History].
 
+error_calls() ->
+    History = meck:history(rebar_log),
+    [{Str, Args} || {_, {rebar_log, log, [error, Str, Args]}, _} <- History].
+
 check_warnings(_, [], _) ->
     ok;
 check_warnings(Warns, [{Name, Vsn} | Rest], Type) ->
diff --git a/test/rebar_test_utils.erl b/test/rebar_test_utils.erl
index 8d999b4..7c52a18 100644
--- a/test/rebar_test_utils.erl
+++ b/test/rebar_test_utils.erl
@@ -48,15 +48,19 @@ run_and_check(Config, RebarConfig, Command, Expect) ->
     %% Assumes init_rebar_state has run first
     AppDir = ?config(apps, Config),
     State = ?config(state, Config),
-    Res = rebar3:run(rebar_state:new(State, RebarConfig, AppDir), Command),
-    case Expect of
-        {error, Reason} ->
-            ?assertEqual({error, Reason}, Res);
-        {ok, Expected} ->
-            {ok, _} = Res,
-            check_results(AppDir, Expected);
-        return ->
-            Res
+    try
+        Res = rebar3:run(rebar_state:new(State, RebarConfig, AppDir), Command),
+        case Expect of
+            {error, Reason} ->
+                ?assertEqual({error, Reason}, Res);
+            {ok, Expected} ->
+                {ok, _} = Res,
+                check_results(AppDir, Expected);
+            return ->
+                Res
+        end
+    catch
+        rebar_abort when Expect =:= rebar_abort -> rebar_abort
     end.
 
 %% @doc Creates a dummy application including: