From bc98ea22aa15406e4cb05e460b72772d40871f4e Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 15 Mar 2015 04:47:13 +0000 Subject: Avoid duplicating deps in discover phase The deps are sorted and merged, but the merge function merges lists, not elements. This yields deps that are duplicated and ran for multiple times. We first add proper sorts so the keymerge is guaranteed to be fine, and then do a dedup run to get rid of duplicates if they happen to be. --- src/rebar_app_discover.erl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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)]. -- cgit v1.1 From f35f26fece0bc3843bd4dba9244ae974fd84d9e1 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 15 Mar 2015 04:49:12 +0000 Subject: Implement deps conflicts as errors The option {deps_error_on_conflict, true} will make it so conflicts in deps being fetched interrupts the operation rather than just display a warning. Defaults to `false'. --- src/rebar_prv_install_deps.erl | 26 +++++++++++++++-------- test/rebar_install_deps_SUITE.erl | 43 +++++++++++++++++++++++++++++++++++---- test/rebar_test_utils.erl | 22 ++++++++++++-------- 3 files changed, 69 insertions(+), 22 deletions(-) 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: -- cgit v1.1