From 31a24ad4ffb6a121819087da2cb9b58db5bd287d Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Wed, 19 Aug 2015 22:41:33 -0500 Subject: this patch treats pkg and src deps as equals, so level decides winner Instead fetching and resolving src deps (which could depend on pkg deps) and then pkg deps this patch combines the two into a single set of iterations by level. The only difference between src and pkg deps in this new install_deps is how their deps list is found -- from the config or lock file for src deps and from the neighbors of the vertex for pkg. --- test/mock_pkg_resource.erl | 4 +++- test/rebar_deps_SUITE.erl | 2 +- test/rebar_install_deps_SUITE.erl | 28 ++++------------------------ 3 files changed, 8 insertions(+), 26 deletions(-) (limited to 'test') diff --git a/test/mock_pkg_resource.erl b/test/mock_pkg_resource.erl index e5c5645..9258f72 100644 --- a/test/mock_pkg_resource.erl +++ b/test/mock_pkg_resource.erl @@ -119,7 +119,9 @@ mock_pkg_index(Opts) -> meck:expect(rebar_state, registry, fun(_State) -> {ok, to_registry(Deps)} end), meck:expect(rebar_state, packages, - fun(_State) -> {Dict, Digraph} end). + fun(_State) -> Dict end), + meck:expect(rebar_state, packages_graph, + fun(_State) -> Digraph end). %%%%%%%%%%%%%%% %%% Helpers %%% diff --git a/test/rebar_deps_SUITE.erl b/test/rebar_deps_SUITE.erl index 04018fc..fd86226 100644 --- a/test/rebar_deps_SUITE.erl +++ b/test/rebar_deps_SUITE.erl @@ -297,5 +297,5 @@ in_warnings(git, Warns, NameRaw, VsnRaw) -> in_warnings(pkg, Warns, NameRaw, VsnRaw) -> Name = iolist_to_binary(NameRaw), Vsn = iolist_to_binary(VsnRaw), - 1 =< length([1 || {_, [AppName, AppVsn]} <- Warns, + 1 =< length([1 || {_, [AppName, {pkg, _, AppVsn}]} <- Warns, AppName =:= Name, AppVsn =:= Vsn]). diff --git a/test/rebar_install_deps_SUITE.erl b/test/rebar_install_deps_SUITE.erl index 66c762e..9d536d1 100644 --- a/test/rebar_install_deps_SUITE.erl +++ b/test/rebar_install_deps_SUITE.erl @@ -15,9 +15,9 @@ groups() -> {git, [], [{group, unique}]}, {pkg, [], [{group, unique}]}, {mixed, [], [ - m_flat1, m_flat2, m_circular1, m_circular2, m_circular3, + m_flat1, m_flat2, m_circular1, m_circular2, m_pick_source1, m_pick_source2, m_pick_source3, - m_pick_source4, m_pick_source5, m_source_to_pkg, + m_source_to_pkg, m_pkg_level1, m_pkg_level2, m_pkg_level3, m_pkg_level3_alpha_order ]} ]. @@ -199,39 +199,22 @@ 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) -> %% The order of declaration is important. {[{"b", []}, {"B", []}], [], {ok, ["b"]}}; -mdeps(m_pick_source4) -> +mdeps(m_pick_source3) -> {[{"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", []}]}]}], [], @@ -438,12 +421,9 @@ 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). @@ -483,5 +463,5 @@ in_warnings(git, Warns, NameRaw, VsnRaw) -> in_warnings(pkg, Warns, NameRaw, VsnRaw) -> Name = iolist_to_binary(NameRaw), Vsn = iolist_to_binary(VsnRaw), - 1 =< length([1 || {_, [AppName, AppVsn]} <- Warns, + 1 =< length([1 || {_, [AppName, {pkg, _, AppVsn}]} <- Warns, AppName =:= Name, AppVsn =:= Vsn]). -- cgit v1.1 From cf5390f01876ff8d9e70cff521740ab0dd805929 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 21 Aug 2015 12:18:30 -0500 Subject: replace use of dict of packages and registry with single ets table --- test/mock_pkg_resource.erl | 55 +++++++++++++++++++++------------------------- test/rebar_pkg_SUITE.erl | 26 ++++++++++++++-------- 2 files changed, 42 insertions(+), 39 deletions(-) (limited to 'test') diff --git a/test/mock_pkg_resource.erl b/test/mock_pkg_resource.erl index 9258f72..4ed8d8e 100644 --- a/test/mock_pkg_resource.erl +++ b/test/mock_pkg_resource.erl @@ -35,7 +35,7 @@ mock(Opts) -> unmock() -> meck:unload(?MOD), - meck:unload(rebar_state). + meck:unload(rebar_packages). %%%%%%%%%%%%%%% %%% Private %%% @@ -111,34 +111,19 @@ mock_pkg_index(Opts) -> Deps = proplists:get_value(pkgdeps, Opts, []), Skip = proplists:get_value(not_in_index, Opts, []), %% Dict: {App, Vsn}: [{<<"link">>, <<>>}, {<<"deps">>, []}] - %% Digraph: all apps and deps in the index + %% Index: all apps and deps in the index + Dict = find_parts(Deps, Skip), - GraphParts = to_graph_parts(Dict), - Digraph = rebar_digraph:restore_graph(GraphParts), - meck:new(rebar_state, [passthrough, no_link]), - meck:expect(rebar_state, registry, - fun(_State) -> {ok, to_registry(Deps)} end), - meck:expect(rebar_state, packages, - fun(_State) -> Dict end), - meck:expect(rebar_state, packages_graph, - fun(_State) -> Digraph end). + meck:new(rebar_packages, [passthrough, no_link]), + meck:expect(rebar_packages, packages, + fun(_State) -> to_index(Deps, Dict) end), + meck:expect(rebar_packages, load_and_verify_version, + fun(_State) -> to_index(Deps, Dict), true end). %%%%%%%%%%%%%%% %%% Helpers %%% %%%%%%%%%%%%%%% -to_registry(Deps) -> - Tid = ets:new(registry, []), - lists:foreach(fun({{Name, Vsn}, _}) -> - case ets:lookup(Tid, Name) of - [{_, [Vsns]}] -> - ets:insert(Tid, {Name, [[Vsn | Vsns]]}); - _ -> - ets:insert(Tid, {Name, [[Vsn]]}) - end - end, Deps), - Tid. - all_files(Dir) -> filelib:wildcard(filename:join([Dir, "**"])). @@ -158,10 +143,20 @@ find_parts([{AppName, Deps}|Rest], Skip, Acc) -> find_parts(Rest, Skip, AccNew) end. -to_graph_parts(Dict) -> - LastUpdated = os:timestamp(), - dict:fold(fun(K,Deps,{Ks,Vs}) -> - {[{K,LastUpdated}|Ks], - [{K,{list_to_binary(atom_to_list(DK)), list_to_binary(DV)}} - || {DK,DV} <- Deps] ++ Vs} - end, {[],[]}, Dict). +to_index(AllDeps, Dict) -> + catch ets:delete(package_index), + ets:new(package_index, [named_table, public]), + dict:fold( + fun(K, Deps, _) -> + DepsList = [{ec_cnv:to_binary(DK), ec_cnv:to_binary(DV)} || {DK, DV} <- Deps], + ets:insert(package_index, {K, DepsList, <<"checksum">>}) + end, ok, Dict), + ets:insert(package_index, {package_index_version, 3}), + lists:foreach(fun({{Name, Vsn}, _}) -> + case ets:lookup(package_index, ec_cnv:to_binary(Name)) of + [{_, Vsns}] -> + ets:insert(package_index, {ec_cnv:to_binary(Name), [ec_cnv:to_binary(Vsn) | Vsns]}); + _ -> + ets:insert(package_index, {ec_cnv:to_binary(Name), [ec_cnv:to_binary(Vsn)]}) + end + end, AllDeps). diff --git a/test/rebar_pkg_SUITE.erl b/test/rebar_pkg_SUITE.erl index e99e787..3cd3a67 100644 --- a/test/rebar_pkg_SUITE.erl +++ b/test/rebar_pkg_SUITE.erl @@ -159,25 +159,33 @@ mock_config(Name, Config) -> Priv = ?config(priv_dir, Config), CacheRoot = filename:join([Priv, "cache", atom_to_list(Name)]), TmpDir = filename:join([Priv, "tmp", atom_to_list(Name)]), - T = ets:new(fake_registry, [public]), - ets:insert_new(T, [ - {{<<"badindexchk">>,<<"1.0.0">>}, [[], ?bad_checksum]}, - {{<<"goodpkg">>,<<"1.0.0">>}, [[], ?good_checksum]}, - {{<<"badpkg">>,<<"1.0.0">>}, [[], ?good_checksum]} + Tid = ets:new(registry_table, [public]), + ets:insert_new(Tid, [ + {<<"badindexchk">>,[[<<"1.0.0">>]]}, + {<<"goodpkg">>,[[<<"1.0.0">>]]}, + {<<"badpkg">>,[[<<"1.0.0">>]]}, + {{<<"badindexchk">>,<<"1.0.0">>}, [[], ?bad_checksum, [<<"rebar3">>]]}, + {{<<"goodpkg">>,<<"1.0.0">>}, [[], ?good_checksum, [<<"rebar3">>]]}, + {{<<"badpkg">>,<<"1.0.0">>}, [[], ?good_checksum, [<<"rebar3">>]]} ]), CacheDir = filename:join([CacheRoot, "hex", "com", "test", "packages"]), filelib:ensure_dir(filename:join([CacheDir, "registry"])), - ok = ets:tab2file(T, filename:join([CacheDir, "registry"])), + ok = ets:tab2file(Tid, filename:join([CacheDir, "registry"])), + %% The state returns us a fake registry meck:new(rebar_state, [passthrough]), - meck:expect(rebar_state, registry, - fun(_State) -> {ok, T} end), meck:expect(rebar_state, get, fun(_State, rebar_packages_cdn, _Default) -> "http://test.com/" end), + meck:new(rebar_dir, [passthrough]), meck:expect(rebar_dir, global_cache_dir, fun(_) -> CacheRoot end), + + meck:new(rebar_packages, [passthrough]), + meck:expect(rebar_packages, package_dir, fun(_) -> CacheDir end), + rebar_prv_update:hex_to_index(rebar_state:new()), + %% Cache fetches are mocked -- we assume the server and clients are %% correctly used. GoodCache = ?config(good_cache, Config), @@ -194,7 +202,7 @@ mock_config(Name, Config) -> [{cache_root, CacheRoot}, {cache_dir, CacheDir}, {tmp_dir, TmpDir}, - {mock_table, T} | Config]. + {mock_table, Tid} | Config]. unmock_config(Config) -> meck:unload(), -- cgit v1.1 From e853e12d7c5b3f0f29670d4ae89d17d004edada1 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 21 Aug 2015 21:15:25 -0500 Subject: add back removed tests --- test/rebar_install_deps_SUITE.erl | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'test') diff --git a/test/rebar_install_deps_SUITE.erl b/test/rebar_install_deps_SUITE.erl index 9d536d1..b8b70b3 100644 --- a/test/rebar_install_deps_SUITE.erl +++ b/test/rebar_install_deps_SUITE.erl @@ -17,7 +17,7 @@ groups() -> {mixed, [], [ m_flat1, m_flat2, m_circular1, m_circular2, m_pick_source1, m_pick_source2, m_pick_source3, - m_source_to_pkg, + m_pick_source4, m_pick_source5, m_source_to_pkg, m_pkg_level1, m_pkg_level2, m_pkg_level3, m_pkg_level3_alpha_order ]} ]. @@ -96,9 +96,9 @@ format_expected_mdeps(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, Vsn} when hd(N) >= $A, hd(N) =< $Z -> {git, 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"} + N when hd(N) >= $A, hd(N) =< $Z -> {git, N, "0.0.0"} end || W <- Warnings]. %% format: @@ -215,6 +215,16 @@ mdeps(m_pick_source3) -> {"b", []}], [], {ok, ["B"]}}; +mdeps(m_pick_source4) -> + {[{"b", [{"d", "1", []}]}, + {"C", [{"D", "1", []}]}], + [{"D", "1"}], + {ok, ["b", "C", {"d", "1"}]}}; +mdeps(m_pick_source5) -> + {[{"B", [{"d", "1", []}]}, + {"C", [{"D", "1", []}]}], + [{"D", "1"}], + {ok, ["B", "C", {"d", "1"}]}}; mdeps(m_source_to_pkg) -> {[{"B", [{"c",[{"d", []}]}]}], [], @@ -424,6 +434,8 @@ m_circular2(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). -- cgit v1.1