From 59579c070838eced4af05113364a87bcc6ad3ebf Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 16 Aug 2015 04:31:18 +0000 Subject: Add tests for mixed deps installs Requires a rework of other test suites using the same dep-handling mechanism. --- test/mock_git_resource.erl | 7 +- test/rebar_compile_SUITE.erl | 3 +- test/rebar_deps_SUITE.erl | 31 ++---- test/rebar_install_deps_SUITE.erl | 225 ++++++++++++++++++++++++++++++++------ test/rebar_plugins_SUITE.erl | 6 +- test/rebar_profiles_SUITE.erl | 15 ++- test/rebar_test_utils.erl | 79 +++++++++---- test/rebar_upgrade_SUITE.erl | 12 +- 8 files changed, 286 insertions(+), 92 deletions(-) diff --git a/test/mock_git_resource.erl b/test/mock_git_resource.erl index d8f747b..0f4aff6 100644 --- a/test/mock_git_resource.erl +++ b/test/mock_git_resource.erl @@ -20,7 +20,8 @@ mock() -> mock([]). | {override_vsn, [{App, Vsn}]} | {deps, [{App, [Dep]}]}, App :: string(), - Dep :: {App, string(), {git, string()} | {git, string(), term()}}, + Dep :: {App, string(), {git, string()} | {git, string(), term()}} + | {pkg, App, term()}, Vsn :: string(). mock(Opts) -> meck:new(?MOD, [no_link]), @@ -46,8 +47,8 @@ mock_lock(_) -> case Git of {git, Url, {tag, Ref}} -> {git, Url, {ref, Ref}}; {git, Url, {ref, Ref}} -> {git, Url, {ref, Ref}}; - {git, Url} -> {git, Url, {ref, "fake-ref"}}; - {git, Url, _} -> {git, Url, {ref, "fake-ref"}} + {git, Url} -> {git, Url, {ref, "0.0.0"}}; + {git, Url, _} -> {git, Url, {ref, "0.0.0"}} end end). diff --git a/test/rebar_compile_SUITE.erl b/test/rebar_compile_SUITE.erl index 0aaa899..1d5aab8 100644 --- a/test/rebar_compile_SUITE.erl +++ b/test/rebar_compile_SUITE.erl @@ -544,7 +544,8 @@ only_default_transitive_deps(Config) -> GitDeps = rebar_test_utils:expand_deps(git, [{"a", "1.0.0", []}]), PkgName = rebar_test_utils:create_random_name("pkg1_"), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(GitDeps)}, + {SrcDeps, _} = rebar_test_utils:flat_deps(GitDeps), + mock_git_resource:mock([{deps, SrcDeps}, {config, [{profiles, [{test, [{deps, [list_to_atom(PkgName)]}]}]}]}]), mock_pkg_resource:mock([{pkgdeps, [{{iolist_to_binary(PkgName), <<"0.1.0">>}, []}]}]), diff --git a/test/rebar_deps_SUITE.erl b/test/rebar_deps_SUITE.erl index bfb6a1f..04018fc 100644 --- a/test/rebar_deps_SUITE.erl +++ b/test/rebar_deps_SUITE.erl @@ -171,30 +171,11 @@ setup_project(Case, Config0, Deps) -> 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}]), - case DepsType of - git -> - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(Deps)}]); - pkg -> - mock_pkg_resource:mock([{pkgdeps, flat_pkgdeps(Deps)}]) - end, + {SrcDeps, PkgDeps} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}]), + mock_pkg_resource:mock([{pkgdeps, PkgDeps}]), [{rebarconfig, RebarConf} | Config]. -flat_pkgdeps([]) -> []; -flat_pkgdeps([{{pkg, Name, Vsn}, Deps} | Rest]) -> - [{{iolist_to_binary(Name),iolist_to_binary(Vsn)}, rebar_test_utils:top_level_deps(Deps)}] - ++ - flat_pkgdeps(Deps) - ++ - flat_pkgdeps(Rest). - -app_vsn([]) -> []; -app_vsn([{Source, Deps} | Rest]) -> - {Name, Vsn} = case Source of - {pkg, N, V} -> {N,V}; - {N,V,_Ref} -> {N,V} - end, - [{Name, Vsn}] ++ app_vsn(Deps) ++ app_vsn(Rest). - mock_warnings() -> %% just let it do its thing, we check warnings through %% the call log. @@ -217,7 +198,8 @@ sub_app_deps(Config) -> Deps = rebar_test_utils:expand_deps(git, [{"a", "1.0.0", []} ,{"b", "1.0.0", []} ,{"b", "2.0.0", []}]), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(Deps)}]), + {SrcDeps, _} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}]), Name = rebar_test_utils:create_random_name("sub_app1_"), Vsn = rebar_test_utils:create_random_vsn(), @@ -241,7 +223,8 @@ newly_added_dep(Config) -> Deps = rebar_test_utils:expand_deps(git, [{"a", "1.0.0", []} ,{"b", "1.0.0", [{"c", "1.0.0", []}]} ,{"c", "2.0.0", []}]), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(Deps)}]), + {SrcDeps, _} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}]), Name = rebar_test_utils:create_random_name("app_"), Vsn = rebar_test_utils:create_random_vsn(), diff --git a/test/rebar_install_deps_SUITE.erl b/test/rebar_install_deps_SUITE.erl index 4e6c3d9..cf3b260 100644 --- a/test/rebar_install_deps_SUITE.erl +++ b/test/rebar_install_deps_SUITE.erl @@ -4,16 +4,24 @@ -include_lib("eunit/include/eunit.hrl"). -include_lib("kernel/include/file.hrl"). -all() -> [{group, git}, {group, pkg}]. +all() -> [{group, git}, {group, pkg}, {group, mixed}]. groups() -> - [{all, [], [flat, pick_highest_left, pick_highest_right, - pick_smallest1, pick_smallest2, - circular1, circular2, circular_skip, - fail_conflict, default_profile, nondefault_profile, - nondefault_pick_highest]}, - {git, [], [{group, all}]}, - {pkg, [], [{group, all}]}]. + [{unique, [], [flat, pick_highest_left, pick_highest_right, + pick_smallest1, pick_smallest2, + circular1, circular2, circular_skip, + fail_conflict, default_profile, nondefault_profile, + nondefault_pick_highest]}, + {git, [], [{group, unique}]}, + {pkg, [], [{group, unique}]}, + {mixed, [], [ + m_flat1, m_flat2, m_circular1, m_circular2, m_circular3, + m_pick_source1, m_pick_source2, m_pick_source3, + m_pick_source4, m_pick_source5, m_source_to_pkg, + m_pkg_level1, m_pkg_level2, + m_pkg_src_override + ]} + ]. init_per_suite(Config) -> application:start(meck), @@ -26,19 +34,33 @@ init_per_group(git, Config) -> [{deps_type, git} | Config]; init_per_group(pkg, Config) -> [{deps_type, pkg} | Config]; +init_per_group(mixed, Config) -> + [{deps_type, mixed} | Config]; init_per_group(_, Config) -> Config. end_per_group(_, Config) -> Config. -init_per_testcase(Case, Config) -> +init_per_testcase(Case, Config) when is_atom(Case) -> + DepsType = ?config(deps_type, Config), + init_per_testcase({DepsType, Case}, Config); +init_per_testcase({mixed, Case}, Config) -> + {Deps, Warnings, Expect} = mdeps(Case), + Expected = case Expect of + {ok, List} -> {ok, format_expected_mdeps(List)}; + Other -> Other + end, + mock_warnings(), + [{expect, Expected}, + {warnings, format_expected_mixed_warnings(Warnings)} + | setup_project(Case, Config, rebar_test_utils:expand_deps(mixed, Deps))]; +init_per_testcase({DepsType, Case}, Config) -> {Deps, Warnings, Expect} = deps(Case), Expected = case Expect of {ok, List} -> {ok, format_expected_deps(List)}; Other -> Other end, - DepsType = ?config(deps_type, Config), mock_warnings(), [{expect, Expected}, {warnings, Warnings} @@ -54,6 +76,32 @@ format_expected_deps(Deps) -> N -> [{dep, N}, {lock, N}] end || Dep <- Deps]). +format_expected_mdeps(Deps) -> + %% for mixed deps, lowercase is a package, uppercase is source. + %% We can't check which was used from the dep, but the lock contains + %% the type and we can use that information. + lists:append([ + case Dep of + {N,V} when hd(N) >= $a, hd(N) =< $z -> + UN = string:to_upper(N), + [{dep, UN, V}, {lock, pkg, UN, V}]; + {N,V} when hd(N) >= $A, hd(N) =< $Z -> + [{dep, N, V}, {lock, src, N, V}]; + N when hd(N) >= $a, hd(N) =< $z -> + UN = string:to_upper(N), + [{dep, UN}, {lock, pkg, UN, "0.0.0"}]; + N when hd(N) >= $A, hd(N) =< $Z -> + [{dep, N}, {lock, src, N, "0.0.0"}] + end || Dep <- Deps]). + +format_expected_mixed_warnings(Warnings) -> + [case W of + {N, Vsn} when hd(N) >= $a, hd(N) =< $z -> {pkg, string:to_upper(N), Vsn}; + {N, Vsn} when hd(N) >= $A, hd(N) =< $Z -> {src, N, Vsn}; + N when hd(N) >= $a, hd(N) =< $z -> {pkg, string:to_upper(N), "0.0.0"}; + N when hd(N) >= $A, hd(N) =< $Z -> {src, N, "0.0.0"} + end || W <- Warnings]. + %% format: %% {Spec, %% [Warning], @@ -131,6 +179,78 @@ deps(nondefault_pick_highest) -> %% This is all handled in setup_project {[],[],{ok,[]}}. +%% format: +%% Same as `deps/1' except "A" is a source dep +%% and "a" is a package dep. +mdeps(m_flat1) -> + {[{"c", []}, + {"B", []}], + [], + {ok, ["B","c"]}}; +mdeps(m_flat2) -> + {[{"B", []}, + {"c", []}], + [], + {ok, ["B","c"]}}; +mdeps(m_circular1) -> + {[{"b", [{"a",[]}]}], % "A" is the top app + [], + {error, {rebar_prv_install_deps, {cycles, [[<<"A">>,<<"B">>]]}}}}; +mdeps(m_circular2) -> + {[{"B", [{"c", [{"b", []}]}]}], + [], + {error, {rebar_prv_install_deps, {cycles, [[<<"B">>,<<"C">>]]}}}}; +mdeps(m_circular3) -> + %% Spot the circular dep due to being to low in the deps tree + %% but as a source dep, taking precedence over packages + {[{"B", [{"C", "2", [{"B", []}]}]}, + {"c", "1", [{"d",[]}]}], + [], + {error, {rebar_prv_install_deps, {cycles, [[<<"B">>,<<"C">>]]}}}}; +mdeps(m_pick_source1) -> + {[{"B", [{"D", []}]}, + {"c", [{"d", []}]}], + ["d"], + {ok, ["B", "c", "D"]}}; +mdeps(m_pick_source2) -> + {[{"b", [{"d", []}]}, + {"C", [{"D", []}]}], + ["d"], + {ok, ["b", "C", "D"]}}; +mdeps(m_pick_source3) -> + {[{"b", []}, + {"B", []}], + ["b"], + {ok, ["B"]}}; +mdeps(m_pick_source4) -> + {[{"B", []}, + {"b", []}], + ["b"], + {ok, ["B"]}}; +mdeps(m_pick_source5) -> + {[{"B", [{"d", []}]}, + {"C", [{"D", []}]}], + ["d"], + {ok, ["B", "C", "D"]}}; +mdeps(m_source_to_pkg) -> + {[{"B", [{"c",[{"d", []}]}]}], + [], + {ok, ["B", "c", "d"]}}; +mdeps(m_pkg_level1) -> + {[{"B", [{"D", [{"e", "2", []}]}]}, + {"C", [{"e", "1", []}]}], + [{"e","2"}], + {ok, ["B","C","D",{"e","1"}]}}; +mdeps(m_pkg_level2) -> + {[{"B", [{"e", "1", []}]}, + {"C", [{"D", [{"e", "2", []}]}]}], + [{"e","2"}], + {ok, ["B","C","D",{"e","1"}]}}; +mdeps(m_pkg_src_override) -> + %% This is all handled in setup_project + {[],[],{ok,[]}}. + + setup_project(fail_conflict, Config0, Deps) -> DepsType = ?config(deps_type, Config0), Config = rebar_test_utils:init_rebar_state( @@ -142,12 +262,9 @@ setup_project(fail_conflict, Config0, Deps) -> 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, + {SrcDeps, PkgDeps} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}]), + mock_pkg_resource:mock([{pkgdeps, PkgDeps}]), [{rebarconfig, RebarConf} | Config]; setup_project(nondefault_profile, Config0, Deps) -> DepsType = ?config(deps_type, Config0), @@ -161,12 +278,9 @@ setup_project(nondefault_profile, Config0, Deps) -> RebarConf = rebar_test_utils:create_config(AppDir, [{profiles, [ {nondef, [{deps, TopDeps}]} ]}]), - 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, + {SrcDeps, PkgDeps} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}]), + mock_pkg_resource:mock([{pkgdeps, PkgDeps}]), [{rebarconfig, RebarConf} | Config]; setup_project(nondefault_pick_highest, Config0, _) -> DepsType = ?config(deps_type, Config0), @@ -187,15 +301,30 @@ setup_project(nondefault_pick_highest, Config0, _) -> ), case DepsType of git -> - mock_git_resource:mock( - [{deps, rebar_test_utils:flat_deps(DefaultDeps ++ ProfileDeps)}] - ); + {SrcDeps, _} = rebar_test_utils:flat_deps(DefaultDeps++ProfileDeps), + mock_git_resource:mock([{deps, SrcDeps}]); pkg -> - mock_pkg_resource:mock( - [{pkgdeps, rebar_test_utils:flat_pkgdeps(DefaultDeps ++ ProfileDeps)}] - ) + {_, PkgDeps} = rebar_test_utils:flat_deps(DefaultDeps++ProfileDeps), + mock_pkg_resource:mock([{pkgdeps, PkgDeps}]) end, [{rebarconfig, RebarConf} | Config]; +setup_project(m_pkg_src_override, Config0, _) -> + Config = rebar_test_utils:init_rebar_state(Config0, "m_pkg_src_override_mixed_"), + AppDir = ?config(apps, Config), + rebar_test_utils:create_app(AppDir, "A", "0.0.0", [kernel, stdlib]), + DefaultDeps = rebar_test_utils:expand_deps(mixed, [{"b", [{"c", []}]}]), + OverrideDeps = rebar_test_utils:expand_deps(mixed, [{"C", []}]), + DefaultTop = rebar_test_utils:top_level_deps(DefaultDeps), + OverrideTop = rebar_test_utils:top_level_deps(OverrideDeps), + RebarConf = rebar_test_utils:create_config( + AppDir, + [{deps, DefaultTop}, + {overrides, [{override, b, [{deps, OverrideTop}]}]}] + ), + {SrcDeps,PkgDeps} = rebar_test_utils:flat_deps(DefaultDeps++OverrideDeps), + mock_git_resource:mock([{deps, SrcDeps}]), + mock_pkg_resource:mock([{pkgdeps, PkgDeps}]), + [{rebarconfig, RebarConf} | Config]; setup_project(Case, Config0, Deps) -> DepsType = ?config(deps_type, Config0), Config = rebar_test_utils:init_rebar_state( @@ -206,12 +335,9 @@ setup_project(Case, Config0, Deps) -> 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}]), - 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, + {SrcDeps, PkgDeps} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}]), + mock_pkg_resource:mock([{pkgdeps, PkgDeps}]), [{rebarconfig, RebarConf} | Config]. mock_warnings() -> @@ -319,6 +445,33 @@ nondefault_pick_highest(Config) -> {ok, [{dep, "B"}, {lock, "B"}, {lock, "C", "1"}, {dep, "C", "2"}], "nondef"} ). +m_flat1(Config) -> run(Config). +m_flat2(Config) -> run(Config). +m_circular1(Config) -> run(Config). +m_circular2(Config) -> run(Config). +m_circular3(Config) -> run(Config). +m_pick_source1(Config) -> run(Config). +m_pick_source2(Config) -> run(Config). +m_pick_source3(Config) -> run(Config). +m_pick_source4(Config) -> run(Config). +m_pick_source5(Config) -> run(Config). +m_source_to_pkg(Config) -> run(Config). +m_pkg_level1(Config) -> run(Config). +m_pkg_level2(Config) -> run(Config). + + +m_pkg_src_override(Config) -> + %% Detect the invalid override where a package dep's are overriden + %% with source dependencies. We only test with overrides because + %% we trust the package index to be correct there and not introduce + %% that kind of error. + {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)), + rebar_test_utils:run_and_check( + Config, RebarConfig, ["lock"], + {error, {rebar_prv_install_deps, + {source_dep_in_pkg, [<<"C">>,<<"b">>]}}} + ). + run(Config) -> {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)), rebar_test_utils:run_and_check( @@ -336,6 +489,10 @@ error_calls() -> check_warnings(_, [], _) -> ok; +check_warnings(Warns, [{Type, Name, Vsn} | Rest], mixed) -> + ct:pal("Checking for warning ~p in ~p", [{Name,Vsn},Warns]), + ?assert(in_warnings(Type, Warns, Name, Vsn)), + check_warnings(Warns, Rest, mixed); check_warnings(Warns, [{Name, Vsn} | Rest], Type) -> ct:pal("Checking for warning ~p in ~p", [{Name,Vsn},Warns]), ?assert(in_warnings(Type, Warns, Name, Vsn)), diff --git a/test/rebar_plugins_SUITE.erl b/test/rebar_plugins_SUITE.erl index adfeafe..5e2c782 100644 --- a/test/rebar_plugins_SUITE.erl +++ b/test/rebar_plugins_SUITE.erl @@ -45,7 +45,8 @@ compile_plugins(Config) -> PluginName = rebar_test_utils:create_random_name("plugin1_"), Plugins = rebar_test_utils:expand_deps(git, [{PluginName, Vsn, []}]), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(Plugins)}]), + {SrcDeps, _} = rebar_test_utils:flat_deps(Plugins), + mock_git_resource:mock([{deps, SrcDeps}]), mock_pkg_resource:mock([{pkgdeps, [{{list_to_binary(DepName), list_to_binary(Vsn)}, []}]}, {config, [{plugins, [ @@ -137,7 +138,8 @@ complex_plugins(Config) -> Deps = rebar_test_utils:expand_deps(git, [{PluginName, Vsn2, [{DepName2, Vsn, [{DepName3, Vsn, []}]}]} ,{DepName, Vsn, []}]), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(Deps)}]), + {SrcDeps, _} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}]), RConfFile = rebar_test_utils:create_config(AppDir, diff --git a/test/rebar_profiles_SUITE.erl b/test/rebar_profiles_SUITE.erl index b42df39..41bb535 100644 --- a/test/rebar_profiles_SUITE.erl +++ b/test/rebar_profiles_SUITE.erl @@ -57,7 +57,8 @@ profile_new_key(Config) -> AllDeps = rebar_test_utils:expand_deps(git, [{"a", "1.0.0", []} ,{"b", "1.0.0", []}]), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(AllDeps)}]), + {SrcDeps, []} = rebar_test_utils:flat_deps(AllDeps), + mock_git_resource:mock([{deps, SrcDeps}]), Name = rebar_test_utils:create_random_name("profile_new_key_"), Vsn = rebar_test_utils:create_random_vsn(), @@ -82,7 +83,8 @@ profile_merge_keys(Config) -> AllDeps = rebar_test_utils:expand_deps(git, [{"a", "1.0.0", []} ,{"b", "1.0.0", []} ,{"b", "2.0.0", []}]), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(AllDeps)}]), + {SrcDeps, []} = rebar_test_utils:flat_deps(AllDeps), + mock_git_resource:mock([{deps, SrcDeps}]), Name = rebar_test_utils:create_random_name("profile_new_key_"), Vsn = rebar_test_utils:create_random_vsn(), @@ -111,7 +113,8 @@ explicit_profile_deduplicate_deps(Config) -> ,{"a", "2.0.0", []} ,{"b", "1.0.0", []} ,{"b", "2.0.0", []}]), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(AllDeps)}]), + {SrcDeps, []} = rebar_test_utils:flat_deps(AllDeps), + mock_git_resource:mock([{deps, SrcDeps}]), Name = rebar_test_utils:create_random_name("explicit_profile_deduplicate_deps_"), Vsn = rebar_test_utils:create_random_vsn(), @@ -141,7 +144,8 @@ implicit_profile_deduplicate_deps(Config) -> ,{"a", "2.0.0", []} ,{"b", "1.0.0", []} ,{"b", "2.0.0", []}]), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(AllDeps)}]), + {SrcDeps, []} = rebar_test_utils:flat_deps(AllDeps), + mock_git_resource:mock([{deps, SrcDeps}]), Name = rebar_test_utils:create_random_name("implicit_profile_deduplicate_deps_"), Vsn = rebar_test_utils:create_random_vsn(), @@ -169,7 +173,8 @@ all_deps_code_paths(Config) -> AllDeps = rebar_test_utils:expand_deps(git, [{"a", "1.0.0", []} ,{"b", "2.0.0", []}]), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(AllDeps)}]), + {SrcDeps, []} = rebar_test_utils:flat_deps(AllDeps), + mock_git_resource:mock([{deps, SrcDeps}]), Name = rebar_test_utils:create_random_name("all_deps_code_paths"), Vsn = rebar_test_utils:create_random_vsn(), diff --git a/test/rebar_test_utils.erl b/test/rebar_test_utils.erl index be52e81..8e00483 100644 --- a/test/rebar_test_utils.erl +++ b/test/rebar_test_utils.erl @@ -2,7 +2,7 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). -export([init_rebar_state/1, init_rebar_state/2, run_and_check/4]). --export([expand_deps/2, flat_deps/1, flat_pkgdeps/1, top_level_deps/1]). +-export([expand_deps/2, flat_deps/1, top_level_deps/1]). -export([create_app/4, create_eunit_app/4, create_empty_app/4, create_config/2]). -export([create_random_name/1, create_random_vsn/0, write_src_file/2]). @@ -137,24 +137,43 @@ expand_deps(pkg, [{Name, Deps} | Rest]) -> [{Dep, expand_deps(pkg, Deps)} | expand_deps(pkg, Rest)]; expand_deps(pkg, [{Name, Vsn, Deps} | Rest]) -> Dep = {pkg, Name, Vsn}, - [{Dep, expand_deps(pkg, Deps)} | expand_deps(pkg, Rest)]. - -flat_deps([]) -> []; -flat_deps([{{Name,_Vsn,Ref}, Deps} | Rest]) -> - [{{Name,vsn_from_ref(Ref)}, top_level_deps(Deps)}] - ++ - flat_deps(Deps) - ++ - flat_deps(Rest). - -flat_pkgdeps([]) -> []; -flat_pkgdeps([{{pkg, Name, Vsn}, Deps} | Rest]) -> - [{{iolist_to_binary(Name),iolist_to_binary(Vsn)}, top_level_deps(Deps)}] - ++ - flat_pkgdeps(Deps) - ++ - flat_pkgdeps(Rest). - + [{Dep, expand_deps(pkg, Deps)} | expand_deps(pkg, Rest)]; +expand_deps(mixed, [{Name, Deps} | Rest]) -> + Dep = if hd(Name) >= $a, hd(Name) =< $z -> + {pkg, string:to_upper(Name), "0.0.0"} + ; hd(Name) >= $A, hd(Name) =< $Z -> + {Name, ".*", {git, "https://example.org/user/"++Name++".git", "master"}} + end, + [{Dep, expand_deps(mixed, Deps)} | expand_deps(mixed, Rest)]; +expand_deps(mixed, [{Name, Vsn, Deps} | Rest]) -> + Dep = if hd(Name) >= $a, hd(Name) =< $z -> + {pkg, string:to_upper(Name), Vsn} + ; hd(Name) >= $A, hd(Name) =< $Z -> + {Name, Vsn, {git, "https://example.org/user/"++Name++".git", {tag, Vsn}}} + end, + [{Dep, expand_deps(mixed, Deps)} | expand_deps(mixed, Rest)]. + +%% Source deps can depend on both source and package dependencies; +%% package deps can only depend on package deps. +%% For things to work we have to go down the dep tree and find all +%% lineages of pkg deps and return them, whereas the source deps +%% can be left as is. +flat_deps(Deps) -> flat_deps(Deps, [], []). + +flat_deps([], Src, Pkg) -> {Src, Pkg}; +flat_deps([{{pkg, Name, Vsn}, PkgDeps} | Rest], Src, Pkg) -> + Current = {{iolist_to_binary(Name), iolist_to_binary(Vsn)}, + top_level_deps(PkgDeps)}, + {[], FlatPkgDeps} = flat_deps(PkgDeps), + flat_deps(Rest, + Src, + Pkg ++ [Current | FlatPkgDeps]); +flat_deps([{{Name,_Vsn,Ref}, Deps} | Rest], Src, Pkg) -> + Current = {{Name,vsn_from_ref(Ref)}, top_level_deps(Deps)}, + {FlatDeps, FlatPkgDeps} = flat_deps(Deps), + flat_deps(Rest, + Src ++ [Current | FlatDeps], + Pkg ++ FlatPkgDeps). vsn_from_ref({git, _, {_, Vsn}}) -> Vsn; vsn_from_ref({git, _, Vsn}) -> Vsn. @@ -278,6 +297,28 @@ check_results(AppDir, Expected, ProfileRun) -> ?assertEqual(iolist_to_binary(Vsn), iolist_to_binary(LockVsn)) end + ; ({lock, pkg, Name, Vsn}) -> + ct:pal("Pkg Lock Name: ~p, Vsn: ~p", [Name, Vsn]), + case lists:keyfind(iolist_to_binary(Name), 1, Locks) of + false -> + error({lock_not_found, Name}); + {_LockName, {pkg, _, LockVsn}, _} -> + ?assertEqual(iolist_to_binary(Vsn), + iolist_to_binary(LockVsn)); + {_LockName, {_, _, {ref, LockVsn}}, _} -> + error({source_lock, {Name, LockVsn}}) + end + ; ({lock, src, Name, Vsn}) -> + ct:pal("Src Lock Name: ~p, Vsn: ~p", [Name, Vsn]), + case lists:keyfind(iolist_to_binary(Name), 1, Locks) of + false -> + error({lock_not_found, Name}); + {_LockName, {pkg, _, LockVsn}, _} -> + error({pkg_lock, {Name, LockVsn}}); + {_LockName, {_, _, {ref, LockVsn}}, _} -> + ?assertEqual(iolist_to_binary(Vsn), + iolist_to_binary(LockVsn)) + end ; ({release, Name, Vsn, ExpectedDevMode}) -> ct:pal("Release: ~p-~s", [Name, Vsn]), {ok, Cwd} = file:get_cwd(), diff --git a/test/rebar_upgrade_SUITE.erl b/test/rebar_upgrade_SUITE.erl index 4ab99c7..54f16da 100644 --- a/test/rebar_upgrade_SUITE.erl +++ b/test/rebar_upgrade_SUITE.erl @@ -425,17 +425,21 @@ upgrades(compile_upgrade_parity) -> mock_deps(git, Deps, Upgrades) -> catch mock_git_resource:unmock(), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(Deps)}, {upgrade, Upgrades}]); + {SrcDeps, _} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}, {upgrade, Upgrades}]); mock_deps(pkg, Deps, Upgrades) -> catch mock_pkg_resource:unmock(), - mock_pkg_resource:mock([{pkgdeps, rebar_test_utils:flat_pkgdeps(Deps)}, {upgrade, Upgrades}]). + {_, PkgDeps} = rebar_test_utils:flat_deps(Deps), + mock_pkg_resource:mock([{pkgdeps, PkgDeps}, {upgrade, Upgrades}]). mock_deps(git, OldDeps, Deps, Upgrades) -> catch mock_git_resource:unmock(), - mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(Deps++OldDeps)}, {upgrade, Upgrades}]); + {SrcDeps, _} = rebar_test_utils:flat_deps(Deps++OldDeps), + mock_git_resource:mock([{deps, SrcDeps}, {upgrade, Upgrades}]); mock_deps(pkg, OldDeps, Deps, Upgrades) -> catch mock_pkg_resource:unmock(), - mock_pkg_resource:mock([{pkgdeps, rebar_test_utils:flat_pkgdeps(Deps++OldDeps)}, {upgrade, Upgrades}]). + {_, PkgDeps} = rebar_test_utils:flat_deps(Deps++OldDeps), + mock_pkg_resource:mock([{pkgdeps, PkgDeps}, {upgrade, Upgrades}]). normalize_unlocks({App, Locks}) -> {iolist_to_binary(App), -- cgit v1.1 From e880e16063066d2582a8e73c006d37bd02bc8268 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 16 Aug 2015 15:03:55 +0000 Subject: Review rule about dep priorities On a single app's dep list, the first noted wins if there's a duplicate between packages and sources, rather than favoring source there anyway. --- test/rebar_install_deps_SUITE.erl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/rebar_install_deps_SUITE.erl b/test/rebar_install_deps_SUITE.erl index cf3b260..ff1d851 100644 --- a/test/rebar_install_deps_SUITE.erl +++ b/test/rebar_install_deps_SUITE.erl @@ -218,10 +218,11 @@ mdeps(m_pick_source2) -> ["d"], {ok, ["b", "C", "D"]}}; mdeps(m_pick_source3) -> + %% The order of declaration is important. {[{"b", []}, {"B", []}], - ["b"], - {ok, ["B"]}}; + ["B"], + {ok, ["b"]}}; mdeps(m_pick_source4) -> {[{"B", []}, {"b", []}], -- cgit v1.1 From e25ba5d8eaff9a3a70b9cecfad48b0717e0b49b2 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 16 Aug 2015 16:25:18 +0000 Subject: All dep overrides in packages are blocked Just rework the error message. --- test/rebar_install_deps_SUITE.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rebar_install_deps_SUITE.erl b/test/rebar_install_deps_SUITE.erl index ff1d851..3fabc56 100644 --- a/test/rebar_install_deps_SUITE.erl +++ b/test/rebar_install_deps_SUITE.erl @@ -470,7 +470,7 @@ m_pkg_src_override(Config) -> rebar_test_utils:run_and_check( Config, RebarConfig, ["lock"], {error, {rebar_prv_install_deps, - {source_dep_in_pkg, [<<"C">>,<<"b">>]}}} + {package_dep_override, <<"b">>}}} ). run(Config) -> -- cgit v1.1 From cf22c7e941bceae8c53ec8160c4311a47676c913 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Sat, 15 Aug 2015 14:57:18 -0500 Subject: track the parent of umbrella app dependencies --- src/rebar_app_discover.erl | 67 +++++++++++++++++++++++++++++++++++------- src/rebar_prv_deps_tree.erl | 13 +++++--- src/rebar_prv_install_deps.erl | 22 ++++++++++---- src/rebar_prv_upgrade.erl | 17 ++++++----- 4 files changed, 92 insertions(+), 27 deletions(-) diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl index 3b34539..63429b5 100644 --- a/src/rebar_app_discover.erl +++ b/src/rebar_app_discover.erl @@ -16,7 +16,31 @@ do(State, LibDirs) -> Apps = find_apps(Dirs, all), ProjectDeps = rebar_state:deps_names(State), DepsDir = rebar_dir:deps_dir(State), + CurrentProfiles = rebar_state:current_profiles(State), + + %% There may be a top level src which is an app and there may not + %% Find it here if there is, otherwise define the deps parent as root + TopLevelApp = case ec_lists:find(fun(X) -> + ec_file:real_dir_path(rebar_app_info:dir(X)) =:= + ec_file:real_dir_path(rebar_dir:root_dir(State)) + end, Apps) of + {ok, App} -> + rebar_app_info:name(App); + error -> + root + end, + + %% Handle top level deps + State1 = lists:foldl(fun(Profile, StateAcc) -> + ProfileDeps = rebar_state:get(StateAcc, {deps, Profile}, []), + ParsedDeps = parse_profile_deps(Profile + ,TopLevelApp + ,ProfileDeps + ,StateAcc), + rebar_state:set(StateAcc, {parsed_deps, Profile}, ParsedDeps) + end, State, lists:reverse(CurrentProfiles)), + %% Handle sub project apps deps %% Sort apps so we get the same merged deps config everytime SortedApps = rebar_utils:sort_deps(Apps), lists:foldl(fun(AppInfo, StateAcc) -> @@ -33,7 +57,7 @@ do(State, LibDirs) -> ?INFO("Ignoring ~s", [Name]), StateAcc end - end, State, SortedApps). + end, State1, SortedApps). format_error({module_list, File}) -> io_lib:format("Error reading module list from ~p~n", [File]); @@ -51,24 +75,45 @@ merge_deps(AppInfo, State) -> rebar_state:apply_profiles( rebar_state:new(reset_hooks(rebar_state:opts(State, Default)), C, rebar_app_info:dir(AppInfo)), CurrentProfiles), Name), + AppState1 = rebar_state:overrides(AppState, rebar_state:get(AppState, overrides, [])), - rebar_utils:check_min_otp_version(rebar_state:get(AppState, minimum_otp_vsn, undefined)), - rebar_utils:check_blacklisted_otp_versions(rebar_state:get(AppState, blacklisted_otp_vsns, [])), + rebar_utils:check_min_otp_version(rebar_state:get(AppState1, minimum_otp_vsn, undefined)), + rebar_utils:check_blacklisted_otp_versions(rebar_state:get(AppState1, blacklisted_otp_vsns, [])), - AppState1 = rebar_state:set(AppState, artifacts, []), - AppInfo1 = rebar_app_info:state(AppInfo, AppState1), + AppState2 = rebar_state:set(AppState1, artifacts, []), + AppInfo1 = rebar_app_info:state(AppInfo, AppState2), State1 = lists:foldl(fun(Profile, StateAcc) -> - AppProfDeps = rebar_state:get(AppState, {deps, Profile}, []), - TopLevelProfDeps = rebar_state:get(StateAcc, {deps, Profile}, []), - ProfDeps2 = rebar_utils:tup_dedup(rebar_utils:tup_umerge( - rebar_utils:tup_sort(TopLevelProfDeps) - ,rebar_utils:tup_sort(AppProfDeps))), - rebar_state:set(StateAcc, {deps, Profile}, ProfDeps2) + handle_profile(Profile, Name, AppState1, StateAcc) end, State, lists:reverse(CurrentProfiles)), {AppInfo1, State1}. +handle_profile(Profile, Name, AppState, State) -> + {TopSrc, TopPkg} = rebar_state:get(State, {parsed_deps, Profile}, {[], []}), + TopLevelProfileDeps = rebar_state:get(State, {deps, Profile}, []), + AppProfileDeps = rebar_state:get(AppState, {deps, Profile}, []), + ProfileDeps2 = rebar_utils:tup_dedup(rebar_utils:tup_umerge( + rebar_utils:tup_sort(TopLevelProfileDeps) + ,rebar_utils:tup_sort(AppProfileDeps))), + State1 = rebar_state:set(State, {deps, Profile}, ProfileDeps2), + + %% Only deps not also specified in the top level config need + %% to be included in the parsed deps + NewDeps = ProfileDeps2 -- TopLevelProfileDeps, + {ParsedSrc, ParsedPkg} = parse_profile_deps(Profile, Name, NewDeps, State1), + rebar_state:set(State1, {parsed_deps, Profile}, {TopSrc++ParsedSrc, TopPkg++ParsedPkg}). + +parse_profile_deps(Profile, Name, Deps, State) -> + DepsDir = rebar_prv_install_deps:profile_dep_dir(State, Profile), + Locks = rebar_state:get(State, {locks, Profile}, []), + rebar_prv_install_deps:parse_deps(Name + ,DepsDir + ,Deps + ,State + ,Locks + ,1). + project_app_config(AppInfo, State) -> C = rebar_config:consult(rebar_app_info:dir(AppInfo)), Dir = rebar_app_info:dir(AppInfo), diff --git a/src/rebar_prv_deps_tree.erl b/src/rebar_prv_deps_tree.erl index 6c69aba..d429c52 100644 --- a/src/rebar_prv_deps_tree.erl +++ b/src/rebar_prv_deps_tree.erl @@ -29,7 +29,7 @@ init(State) -> do(State) -> {Args, _} = rebar_state:command_parsed_args(State), Verbose = proplists:get_value(verbose, Args, false), - print_deps_tree(rebar_state:all_deps(State), Verbose), + print_deps_tree(rebar_state:all_deps(State), Verbose, State), {ok, State}. -spec format_error(any()) -> iolist(). @@ -38,7 +38,7 @@ format_error(Reason) -> %% Internal functions -print_deps_tree(SrcDeps, Verbose) -> +print_deps_tree(SrcDeps, Verbose, State) -> D = lists:foldl(fun(App, Dict) -> Name = rebar_app_info:name(App), Vsn = rebar_app_info:original_vsn(App), @@ -46,11 +46,14 @@ print_deps_tree(SrcDeps, Verbose) -> Parent = rebar_app_info:parent(App), dict:append_list(Parent, [{Name, Vsn, Source}], Dict) end, dict:new(), SrcDeps), + ProjectAppNames = [{rebar_app_info:name(App) + ,rebar_app_info:original_vsn(App) + ,project} || App <- rebar_state:project_apps(State)], case dict:find(root, D) of {ok, Children} -> - print_children(-1, lists:keysort(1, Children), D, Verbose); + print_children(-1, lists:keysort(1, Children++ProjectAppNames), D, Verbose); error -> - none + print_children(-1, lists:keysort(1, ProjectAppNames), D, Verbose) end. print_children(_, [], _, _) -> @@ -68,6 +71,8 @@ print_children(Indent, [{Name, Vsn, Source} | Rest], Dict, Verbose) -> print_children(Indent, Rest, Dict, Verbose) end. +type(project, _) -> + "project app"; type(Source, Verbose) when is_tuple(Source) -> case {element(1, Source), Verbose} of {pkg, _} -> diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index e521b25..74de109 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -37,6 +37,8 @@ -export([handle_deps_as_profile/4, parse_deps/5, + parse_deps/6, + profile_dep_dir/2, find_cycles/1, cull_compile/2]). @@ -155,10 +157,11 @@ deps_per_profile(Profiles, Upgrade, State) -> handle_profile_pkg_level(PkgDeps1, AllApps, Seen, Upgrade, Locks, State1). parse_profile_deps(State, Profile, Level) -> - DepsDir = profile_dep_dir(State, Profile), + %DepsDir = profile_dep_dir(State, Profile), Locks = rebar_state:get(State, {locks, Profile}, []), - Deps = rebar_state:get(State, {deps, Profile}, []), - {SrcDeps, PkgDeps} = parse_deps(DepsDir, Deps, State, Locks, Level), + %Deps = rebar_state:get(State, {deps, Profile}, []), + %{SrcDeps, PkgDeps} = parse_deps(DepsDir, Deps, State, Locks, Level), + {SrcDeps, PkgDeps} = rebar_state:get(State, {parsed_deps, Profile}, {[], []}), {{Profile, SrcDeps, Locks, Level}, {Profile, Level, PkgDeps}}. %% Level-order traversal of all dependencies, across profiles. @@ -373,12 +376,21 @@ handle_dep(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> -spec handle_dep(rebar_state:t(), atom(), file:filename_all(), rebar_app_info:t(), list(), integer()) -> {rebar_app_info:t(), [rebar_app_info:t()], [pkg_dep()], [integer()], rebar_state:t()}. handle_dep(State, Profile, DepsDir, AppInfo, Locks, Level) -> + Parent = rebar_app_info:parent(AppInfo), Profiles = rebar_state:current_profiles(State), Name = rebar_app_info:name(AppInfo), - C = rebar_config:consult(rebar_app_info:dir(AppInfo)), + %% Deps may be under a sub project app, find it and use its state if so + S = case ec_lists:find(fun(X) -> + Parent =:= rebar_app_info:name(X) + end, rebar_state:project_apps(State)) of + {ok, ParentApp} -> + rebar_app_info:state(ParentApp); + error -> + rebar_app_info:state(AppInfo) + end, - S = rebar_app_info:state(AppInfo), + C = rebar_config:consult(rebar_app_info:dir(AppInfo)), S1 = rebar_state:new(S, C, rebar_app_info:dir(AppInfo)), S2 = rebar_state:apply_overrides(S1, Name), diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl index 3a371ca..f49eafe 100644 --- a/src/rebar_prv_upgrade.erl +++ b/src/rebar_prv_upgrade.erl @@ -53,18 +53,21 @@ do(State) -> {Locks0, _Unlocks0} -> Deps0 = top_level_deps(Deps, Locks), State1 = rebar_state:set(State, {deps, default}, Deps0), - State2 = rebar_state:set(State1, {locks, default}, Locks0), - State3 = rebar_state:set(State2, upgrade, true), - UpdatedLocks = [L || L <- rebar_state:lock(State3), + DepsDir = rebar_prv_install_deps:profile_dep_dir(State, default), + D = rebar_prv_install_deps:parse_deps(root, DepsDir, Deps0, State1, Locks0, 0), + State2 = rebar_state:set(State1, {parsed_deps, default}, D), + State3 = rebar_state:set(State2, {locks, default}, Locks0), + State4 = rebar_state:set(State3, upgrade, true), + UpdatedLocks = [L || L <- rebar_state:lock(State4), lists:keymember(rebar_app_info:name(L), 1, Locks0)], - Res = rebar_prv_install_deps:do(rebar_state:lock(State3, UpdatedLocks)), + Res = rebar_prv_install_deps:do(rebar_state:lock(State4, UpdatedLocks)), case Res of - {ok, State4} -> + {ok, State5} -> rebar_utils:info_useless( [element(1,Lock) || Lock <- Locks], - [rebar_app_info:name(App) || App <- rebar_state:lock(State4)] + [rebar_app_info:name(App) || App <- rebar_state:lock(State5)] ), - rebar_prv_lock:do(State4); + rebar_prv_lock:do(State5); _ -> Res end -- cgit v1.1 From 073dbb71ec98cec6c0aeced82cc3797ffa38e68e Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Sun, 16 Aug 2015 19:09:42 -0500 Subject: handle new tests for mix package types (git/pkg) --- src/rebar_app_discover.erl | 6 +- src/rebar_digraph.erl | 139 ++++++++++++++++++++++++-------------- src/rebar_prv_install_deps.erl | 7 +- test/rebar_install_deps_SUITE.erl | 53 ++++----------- 4 files changed, 111 insertions(+), 94 deletions(-) diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl index 63429b5..ae0fd73 100644 --- a/src/rebar_app_discover.erl +++ b/src/rebar_app_discover.erl @@ -33,9 +33,10 @@ do(State, LibDirs) -> %% Handle top level deps State1 = lists:foldl(fun(Profile, StateAcc) -> ProfileDeps = rebar_state:get(StateAcc, {deps, Profile}, []), + ProfileDeps2 = rebar_utils:tup_dedup(rebar_utils:tup_sort(ProfileDeps)), ParsedDeps = parse_profile_deps(Profile ,TopLevelApp - ,ProfileDeps + ,ProfileDeps2 ,StateAcc), rebar_state:set(StateAcc, {parsed_deps, Profile}, ParsedDeps) end, State, lists:reverse(CurrentProfiles)), @@ -93,9 +94,10 @@ handle_profile(Profile, Name, AppState, State) -> {TopSrc, TopPkg} = rebar_state:get(State, {parsed_deps, Profile}, {[], []}), TopLevelProfileDeps = rebar_state:get(State, {deps, Profile}, []), AppProfileDeps = rebar_state:get(AppState, {deps, Profile}, []), + AppProfileDeps2 = rebar_utils:tup_dedup(rebar_utils:tup_sort(AppProfileDeps)), ProfileDeps2 = rebar_utils:tup_dedup(rebar_utils:tup_umerge( rebar_utils:tup_sort(TopLevelProfileDeps) - ,rebar_utils:tup_sort(AppProfileDeps))), + ,rebar_utils:tup_sort(AppProfileDeps2))), State1 = rebar_state:set(State, {deps, Profile}, ProfileDeps2), %% Only deps not also specified in the top level config need diff --git a/src/rebar_digraph.erl b/src/rebar_digraph.erl index 1132a6c..f2bb540 100644 --- a/src/rebar_digraph.erl +++ b/src/rebar_digraph.erl @@ -3,6 +3,7 @@ -export([compile_order/1 ,restore_graph/1 ,cull_deps/2 + ,cull_deps/3 ,subgraph/2 ,format_error/1]). @@ -69,8 +70,12 @@ restore_graph({Vs, Es}) -> %% The first dep while traversing the graph is chosen and any conflicting %% dep encountered later on is ignored. cull_deps(Graph, Vertices) -> - {Solution, Levels} = build_initial_dicts(Vertices), - cull_deps(Graph, Vertices, Levels, Solution, []). + cull_deps(Graph, Vertices, sets:new()). + +cull_deps(Graph, Vertices, Seen) -> + Vertices1 = lists:keysort(2, Vertices), + {Solution, Levels, Discarded} = {dict:new(), dict:new(), []}, + cull_deps(Graph, Vertices1, Levels, Solution, Seen, Discarded). format_error(no_solution) -> io_lib:format("No solution for packages found.", []). @@ -79,79 +84,109 @@ format_error(no_solution) -> %% Internal Functions %%==================================================================== -cull_deps(_Graph, [], Levels, Solution, Discarded) -> +cull_deps(_Graph, [], Levels, Solution, _, Discarded) -> {_, Vertices} = lists:unzip(dict:to_list(Solution)), - LvlVertices = [{Profile, {Parent,App,Vsn,dict:fetch(App,Levels)}} || {Profile, {Parent,App,Vsn}} <- Vertices], + LvlVertices = [{Profile, {Parent, App, Vsn, dict:fetch(App, Levels)}} + || {Profile, {Parent,App,Vsn}} <- Vertices], {ok, LvlVertices, Discarded}; -cull_deps(Graph, [{Profile, Level, Vs} | Vertices], Levels, Solution, Discarded) -> +cull_deps(Graph, [{Profile, Level, Vs} | Vertices], Levels, Solution, Seen, Discarded) -> {NV, NS, LS, DS} = - lists:foldl(fun({_, Name, Vsn}, {Acc, SolutionAcc, LevelsAcc, DiscardedAcc}) -> - OutNeighbors = lists:keysort(1, digraph:out_neighbours(Graph, {Name,Vsn})), - handle_neighbors(Profile, Level, Name - ,OutNeighbors, Acc, SolutionAcc - ,LevelsAcc, DiscardedAcc) - - end, {[], Solution, Levels, Discarded}, lists:keysort(2, Vs)), - - cull_deps(Graph, Vertices++NV, LS, NS, DS). + lists:foldl(fun({Parent, Name, Vsn}, {Acc, SolutionAcc, LevelsAcc, DiscardedAcc}) -> + {SolutionAcc1, LevelsAcc1, DiscardedAcc1} = + maybe_add_to_solution(Profile, Level, Name, {Name, Vsn}, Parent + ,SolutionAcc + ,LevelsAcc, Seen, DiscardedAcc), + OutNeighbors = digraph:out_neighbours(Graph, {Name,Vsn}), + {NewVertices, DiscardedAcc2} = handle_neighbors(Profile, Level, Name + ,OutNeighbors, Acc, SolutionAcc1 + ,Seen, DiscardedAcc1), + {NewVertices, SolutionAcc1, LevelsAcc1, DiscardedAcc2} + end, {[], Solution, Levels, Discarded}, Vs), + NewVertices = combine_profile_levels(Vertices, NV), + cull_deps(Graph, NewVertices, LS, NS, Seen, DS). + +%% Combine lists of deps that have the same profile and level +combine_profile_levels(Vertices, NewVertices) -> + V = lists:foldl(fun({Profile, Level, Vs}, Acc) -> + case ec_lists:find(fun({P, L, _}) when P =:= Profile + , L =:= Level -> + true; + (_) -> + false + end, Acc) of + {ok, {_, _, OldVs}=Old} -> + lists:delete(Old, Acc)++[{Profile, Level, lists:keysort(1, OldVs++Vs)}]; + error -> + Acc++[{Profile, Level, Vs}] + end + end, Vertices, NewVertices), + lists:keysort(2, V). %% For each outgoing edge of a dep check if it should be added to the solution %% and add it to the list of vertices to do the same for handle_neighbors(Profile, Level, Parent, OutNeighbors, Vertices - ,Solution, Levels, Discarded) -> - case lists:foldl(fun({Name, _}=N, {NewVertices, Solution1, Levels1, Discarded1}) -> - maybe_add_to_solution(Profile, Level, Name, N, Parent - ,NewVertices, Solution1 - ,Levels1, Discarded1) - end, {[], Solution, Levels, Discarded}, OutNeighbors) of - {[], SolutionAcc2, LevelsAcc2, DiscardedAcc2} -> - {Vertices, SolutionAcc2, LevelsAcc2, DiscardedAcc2}; - {NewVertices1, SolutionAcc2, LevelsAcc2, DiscardedAcc2} -> - {Vertices++[{Profile, Level+1, NewVertices1}] - ,SolutionAcc2, LevelsAcc2, DiscardedAcc2} + ,Solution, Seen, Discarded) -> + case lists:foldl(fun({Name, Vsn}=Value, {NewVertices, Discarded1}) -> + case dict:find(Name, Solution) of + {ok, {Profile, {Parent, Name, Vsn}}} -> % already seen + {NewVertices, + Discarded1}; + {ok, _} -> % conflict resolution! + %% Warn on different version + {NewVertices, + [Value|Discarded1]}; + error -> + %% We check Seen separately because we don't care + %% to warn if the exact same version of a package + %% was already part of the solution but we do + %% if it was simply seen in source deps + case sets:is_element(Name, Seen) of + true -> + {NewVertices, + [Value|Discarded1]}; + false -> + {[{Parent, Name, Vsn} | NewVertices], + Discarded1} + end + end + end, {[], Discarded}, OutNeighbors) of + {[], DiscardedAcc2} -> + {Vertices, DiscardedAcc2}; + {NewVertices1, DiscardedAcc2} -> + {Vertices++[{Profile, Level+1, NewVertices1}] ,DiscardedAcc2} end. maybe_add_to_solution(Profile, Level, Key, {Name, Vsn}=Value, Parent - ,Vertices ,Solution, Levels, Discarded) -> + ,Solution, Levels, Seen, Discarded) -> case dict:find(Key, Solution) of {ok, {Profile, {Parent, Name, Vsn}}} -> % already seen - {Vertices, - Solution, + {Solution, Levels, Discarded}; {ok, _} -> % conflict resolution! - {Vertices, - Solution, + %% Warn on different version + {Solution, Levels, [Value|Discarded]}; error -> - {[{Parent, Name, Vsn} | Vertices], - dict:store(Key, {Profile, {Parent, Name, Vsn}}, Solution), - dict:store(Key, Level+1, Levels), - Discarded} + %% We check Seen separately because we don't care to warn if the exact + %% same version of a package was already part of the solution but we do + %% if it was simply seen in source deps + case sets:is_element(Name, Seen) of + true -> + {Solution, + Levels, + [Value|Discarded]}; + false -> + {dict:store(Key, {Profile, {Parent, Name, Vsn}}, Solution), + dict:store(Key, Level, Levels), + Discarded} + end end. subgraph(Graph, Vertices) -> digraph_utils:subgraph(Graph, Vertices). -maybe_add_to_dict(Key, Value, Dict) -> - case dict:is_key(Key, Dict) of - true -> - Dict; - false -> - dict:store(Key, Value, Dict) - end. - -%% Track the profile (so we know where to install it), name/vsn of each dep -%% and the level it is from (for the lock file) -build_initial_dicts(Vertices) -> - lists:foldl(fun({Profile, Level, Vs}, {Solution, Levels}) -> - lists:foldl(fun({Parent, Key, Vsn}, {SAcc, LAcc}) -> - {maybe_add_to_dict(Key, {Profile, {Parent,Key,Vsn}}, SAcc), - maybe_add_to_dict(Key, Level, LAcc)} - end, {Solution, Levels}, Vs) - end, {dict:new(), dict:new()}, Vertices). - -spec names_to_apps([atom()], [rebar_app_info:t()]) -> [rebar_app_info:t()]. names_to_apps(Names, Apps) -> [element(2, App) || App <- [find_app_by_name(Name, Apps) || Name <- Names], App =/= error]. diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index 74de109..ad469c3 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -193,9 +193,12 @@ handle_profile_pkg_level(PkgDeps, AllApps, Seen, Upgrade, Locks, State) -> State1 = rebar_state:packages(rebar_state:registry(State, Registry) ,{Packages, Graph}), - S = case rebar_digraph:cull_deps(Graph, lists:keysort(2, PkgDeps)) of - {ok, [], _} -> + S = case rebar_digraph:cull_deps(Graph, lists:keysort(2, PkgDeps), Seen) of + {ok, [], []} -> throw({rebar_digraph, no_solution}); + {ok, [], Discarded} -> + [warn_skip_pkg(Pkg, State) || Pkg <- Discarded, not(pkg_locked(Pkg, Locks))], + []; {ok, Solution, []} -> Solution; {ok, Solution, Discarded} -> diff --git a/test/rebar_install_deps_SUITE.erl b/test/rebar_install_deps_SUITE.erl index 3fabc56..66c762e 100644 --- a/test/rebar_install_deps_SUITE.erl +++ b/test/rebar_install_deps_SUITE.erl @@ -18,8 +18,7 @@ groups() -> m_flat1, m_flat2, m_circular1, m_circular2, m_circular3, m_pick_source1, m_pick_source2, m_pick_source3, m_pick_source4, m_pick_source5, m_source_to_pkg, - m_pkg_level1, m_pkg_level2, - m_pkg_src_override + m_pkg_level1, m_pkg_level2, m_pkg_level3, m_pkg_level3_alpha_order ]} ]. @@ -221,12 +220,12 @@ mdeps(m_pick_source3) -> %% The order of declaration is important. {[{"b", []}, {"B", []}], - ["B"], + [], {ok, ["b"]}}; mdeps(m_pick_source4) -> {[{"B", []}, {"b", []}], - ["b"], + [], {ok, ["B"]}}; mdeps(m_pick_source5) -> {[{"B", [{"d", []}]}, @@ -247,10 +246,16 @@ mdeps(m_pkg_level2) -> {"C", [{"D", [{"e", "2", []}]}]}], [{"e","2"}], {ok, ["B","C","D",{"e","1"}]}}; -mdeps(m_pkg_src_override) -> - %% This is all handled in setup_project - {[],[],{ok,[]}}. - +mdeps(m_pkg_level3_alpha_order) -> + {[{"B", [{"d", [{"f", "1", []}]}]}, + {"C", [{"E", [{"f", "2", []}]}]}], + [{"f","2"}], + {ok, ["B","C","d","E",{"f","1"}]}}; +mdeps(m_pkg_level3) -> + {[{"B", [{"d", [{"f", "1", []}]}]}, + {"C", [{"E", [{"G", [{"f", "2", []}]}]}]}], + [{"f","2"}], + {ok, ["B","C","d","E","G",{"f","1"}]}}. setup_project(fail_conflict, Config0, Deps) -> DepsType = ?config(deps_type, Config0), @@ -309,23 +314,6 @@ setup_project(nondefault_pick_highest, Config0, _) -> mock_pkg_resource:mock([{pkgdeps, PkgDeps}]) end, [{rebarconfig, RebarConf} | Config]; -setup_project(m_pkg_src_override, Config0, _) -> - Config = rebar_test_utils:init_rebar_state(Config0, "m_pkg_src_override_mixed_"), - AppDir = ?config(apps, Config), - rebar_test_utils:create_app(AppDir, "A", "0.0.0", [kernel, stdlib]), - DefaultDeps = rebar_test_utils:expand_deps(mixed, [{"b", [{"c", []}]}]), - OverrideDeps = rebar_test_utils:expand_deps(mixed, [{"C", []}]), - DefaultTop = rebar_test_utils:top_level_deps(DefaultDeps), - OverrideTop = rebar_test_utils:top_level_deps(OverrideDeps), - RebarConf = rebar_test_utils:create_config( - AppDir, - [{deps, DefaultTop}, - {overrides, [{override, b, [{deps, OverrideTop}]}]}] - ), - {SrcDeps,PkgDeps} = rebar_test_utils:flat_deps(DefaultDeps++OverrideDeps), - mock_git_resource:mock([{deps, SrcDeps}]), - mock_pkg_resource:mock([{pkgdeps, PkgDeps}]), - [{rebarconfig, RebarConf} | Config]; setup_project(Case, Config0, Deps) -> DepsType = ?config(deps_type, Config0), Config = rebar_test_utils:init_rebar_state( @@ -459,19 +447,8 @@ m_pick_source5(Config) -> run(Config). m_source_to_pkg(Config) -> run(Config). m_pkg_level1(Config) -> run(Config). m_pkg_level2(Config) -> run(Config). - - -m_pkg_src_override(Config) -> - %% Detect the invalid override where a package dep's are overriden - %% with source dependencies. We only test with overrides because - %% we trust the package index to be correct there and not introduce - %% that kind of error. - {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)), - rebar_test_utils:run_and_check( - Config, RebarConfig, ["lock"], - {error, {rebar_prv_install_deps, - {package_dep_override, <<"b">>}}} - ). +m_pkg_level3(Config) -> run(Config). +m_pkg_level3_alpha_order(Config) -> run(Config). run(Config) -> {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)), -- cgit v1.1 From ad463398dcc4b0d652218d20f79cc010d2767b37 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Sun, 16 Aug 2015 20:48:59 -0500 Subject: use correct rebar_state for a dep, not the top level state --- src/rebar_app_discover.erl | 7 ++++--- src/rebar_prv_common_test.erl | 8 ++++---- src/rebar_prv_install_deps.erl | 11 +---------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl index ae0fd73..6eeadf0 100644 --- a/src/rebar_app_discover.erl +++ b/src/rebar_app_discover.erl @@ -37,6 +37,7 @@ do(State, LibDirs) -> ParsedDeps = parse_profile_deps(Profile ,TopLevelApp ,ProfileDeps2 + ,StateAcc ,StateAcc), rebar_state:set(StateAcc, {parsed_deps, Profile}, ParsedDeps) end, State, lists:reverse(CurrentProfiles)), @@ -103,16 +104,16 @@ handle_profile(Profile, Name, AppState, State) -> %% Only deps not also specified in the top level config need %% to be included in the parsed deps NewDeps = ProfileDeps2 -- TopLevelProfileDeps, - {ParsedSrc, ParsedPkg} = parse_profile_deps(Profile, Name, NewDeps, State1), + {ParsedSrc, ParsedPkg} = parse_profile_deps(Profile, Name, NewDeps, AppState, State1), rebar_state:set(State1, {parsed_deps, Profile}, {TopSrc++ParsedSrc, TopPkg++ParsedPkg}). -parse_profile_deps(Profile, Name, Deps, State) -> +parse_profile_deps(Profile, Name, Deps, AppState, State) -> DepsDir = rebar_prv_install_deps:profile_dep_dir(State, Profile), Locks = rebar_state:get(State, {locks, Profile}, []), rebar_prv_install_deps:parse_deps(Name ,DepsDir ,Deps - ,State + ,AppState ,Locks ,1). diff --git a/src/rebar_prv_common_test.erl b/src/rebar_prv_common_test.erl index 2b024cf..1165631 100644 --- a/src/rebar_prv_common_test.erl +++ b/src/rebar_prv_common_test.erl @@ -361,10 +361,10 @@ remove_links(Path) -> end. delete_dir_link(Path) -> - case os:type() of - {unix, _} -> file:delete(Path); - {win32, _} -> file:del_dir(Path) - end. + case os:type() of + {unix, _} -> file:delete(Path); + {win32, _} -> file:del_dir(Path) + end. dir_entries(Path) -> {ok, SubDirs} = file:list_dir(Path), diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index ad469c3..e790dc9 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -379,20 +379,11 @@ handle_dep(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> -spec handle_dep(rebar_state:t(), atom(), file:filename_all(), rebar_app_info:t(), list(), integer()) -> {rebar_app_info:t(), [rebar_app_info:t()], [pkg_dep()], [integer()], rebar_state:t()}. handle_dep(State, Profile, DepsDir, AppInfo, Locks, Level) -> - Parent = rebar_app_info:parent(AppInfo), Profiles = rebar_state:current_profiles(State), Name = rebar_app_info:name(AppInfo), %% Deps may be under a sub project app, find it and use its state if so - S = case ec_lists:find(fun(X) -> - Parent =:= rebar_app_info:name(X) - end, rebar_state:project_apps(State)) of - {ok, ParentApp} -> - rebar_app_info:state(ParentApp); - error -> - rebar_app_info:state(AppInfo) - end, - + S = rebar_app_info:state(AppInfo), C = rebar_config:consult(rebar_app_info:dir(AppInfo)), S1 = rebar_state:new(S, C, rebar_app_info:dir(AppInfo)), S2 = rebar_state:apply_overrides(S1, Name), -- cgit v1.1 From 1f72ddf04277322a162ff3235f029f28541a2c14 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Mon, 17 Aug 2015 17:11:10 -0500 Subject: remove commented out code --- src/rebar_prv_install_deps.erl | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index e790dc9..11a4250 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -157,10 +157,7 @@ deps_per_profile(Profiles, Upgrade, State) -> handle_profile_pkg_level(PkgDeps1, AllApps, Seen, Upgrade, Locks, State1). parse_profile_deps(State, Profile, Level) -> - %DepsDir = profile_dep_dir(State, Profile), Locks = rebar_state:get(State, {locks, Profile}, []), - %Deps = rebar_state:get(State, {deps, Profile}, []), - %{SrcDeps, PkgDeps} = parse_deps(DepsDir, Deps, State, Locks, Level), {SrcDeps, PkgDeps} = rebar_state:get(State, {parsed_deps, Profile}, {[], []}), {{Profile, SrcDeps, Locks, Level}, {Profile, Level, PkgDeps}}. -- cgit v1.1 From e941e170e4572d6f5252232b4fd75f1fa4cad2c2 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Mon, 17 Aug 2015 17:39:31 -0500 Subject: small refactorings per Fred comments --- src/rebar_app_discover.erl | 22 +++++++++++++--------- src/rebar_digraph.erl | 9 +++------ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl index 6eeadf0..e81a323 100644 --- a/src/rebar_app_discover.erl +++ b/src/rebar_app_discover.erl @@ -20,15 +20,7 @@ do(State, LibDirs) -> %% There may be a top level src which is an app and there may not %% Find it here if there is, otherwise define the deps parent as root - TopLevelApp = case ec_lists:find(fun(X) -> - ec_file:real_dir_path(rebar_app_info:dir(X)) =:= - ec_file:real_dir_path(rebar_dir:root_dir(State)) - end, Apps) of - {ok, App} -> - rebar_app_info:name(App); - error -> - root - end, + TopLevelApp = define_root_app(Apps, State), %% Handle top level deps State1 = lists:foldl(fun(Profile, StateAcc) -> @@ -61,6 +53,18 @@ do(State, LibDirs) -> end end, State1, SortedApps). +define_root_app(Apps, State) -> + RootDir = rebar_dir:root_dir(State), + case ec_lists:find(fun(X) -> + ec_file:real_dir_path(rebar_app_info:dir(X)) =:= + ec_file:real_dir_path(RootDir) + end, Apps) of + {ok, App} -> + rebar_app_info:name(App); + error -> + root + end. + format_error({module_list, File}) -> io_lib:format("Error reading module list from ~p~n", [File]); format_error({missing_module, Module}) -> diff --git a/src/rebar_digraph.erl b/src/rebar_digraph.erl index f2bb540..472a543 100644 --- a/src/rebar_digraph.erl +++ b/src/rebar_digraph.erl @@ -108,12 +108,9 @@ cull_deps(Graph, [{Profile, Level, Vs} | Vertices], Levels, Solution, Seen, Disc %% Combine lists of deps that have the same profile and level combine_profile_levels(Vertices, NewVertices) -> V = lists:foldl(fun({Profile, Level, Vs}, Acc) -> - case ec_lists:find(fun({P, L, _}) when P =:= Profile - , L =:= Level -> - true; - (_) -> - false - end, Acc) of + case ec_lists:find(fun({P, L, _}) -> + P =:= Profile andalso L =:= Level + end, Acc) of {ok, {_, _, OldVs}=Old} -> lists:delete(Old, Acc)++[{Profile, Level, lists:keysort(1, OldVs++Vs)}]; error -> -- cgit v1.1