From 9d788d893620f868b9c0ee00ddec8ae4d5d8fea7 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Fri, 5 Oct 2018 07:57:20 -0400 Subject: Abstracted path management Move path management out of rebar_utils manual code path function handling (which we leave there for backwards compat), and centralize them to allow easier coordination of paths between plugins and deps. On top of path handling, do a check of loaded modules to only purge and reload those that actually need it done in order to prevent all kinds of weird interaction and accidental purge kills. It also allows the possible cohabitation of both at once, with a "in case of conflict pick X" as a policy Changing path handling in providers also highlighted a bunch of bugs in some tests and appears to fix some in other providers, specifically around plugins. --- test/rebar_compile_SUITE.erl | 5 +- test/rebar_paths_SUITE.erl | 211 +++++++++++++++++++++++++++++++++++++++++++ test/rebar_plugins_SUITE.erl | 2 +- 3 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 test/rebar_paths_SUITE.erl (limited to 'test') diff --git a/test/rebar_compile_SUITE.erl b/test/rebar_compile_SUITE.erl index 867460c..6b1d791 100644 --- a/test/rebar_compile_SUITE.erl +++ b/test/rebar_compile_SUITE.erl @@ -1856,7 +1856,7 @@ include_file_in_src_test_multiapp(Config) -> AppDir1 = filename:join([?config(apps, Config), "lib", Name1]), AppDir2 = filename:join([?config(apps, Config), "lib", Name2]), Vsn = rebar_test_utils:create_random_vsn(), - rebar_test_utils:create_app(AppDir1, Name1, Vsn, [kernel, stdlib]), + rebar_test_utils:create_app(AppDir1, Name1, Vsn, [kernel, stdlib, list_to_atom(Name2)]), rebar_test_utils:create_app(AppDir2, Name2, Vsn, [kernel, stdlib]), Src = "-module(test).\n" @@ -1878,7 +1878,8 @@ include_file_in_src_test_multiapp(Config) -> RebarConfig = [], rebar_test_utils:run_and_check(Config, RebarConfig, ["as", "test", "compile"], - {ok, [{app, Name1}]}). + {ok, [{app, Name1}]}), + ok. %% this test sets the env var, compiles, records the file last modified timestamp, %% recompiles and compares the file last modified timestamp to ensure it hasn't diff --git a/test/rebar_paths_SUITE.erl b/test/rebar_paths_SUITE.erl new file mode 100644 index 0000000..48a7000 --- /dev/null +++ b/test/rebar_paths_SUITE.erl @@ -0,0 +1,211 @@ +-module(rebar_paths_SUITE). +-include_lib("eunit/include/eunit.hrl"). +-include_lib("common_test/include/ct.hrl"). +-compile(export_all). + +all() -> + [%clashing_apps, + check_modules, + set_paths + ]. + +init_per_testcase(Case, Config) -> + BasePaths = code:get_path(), + %% This test checks that the right module sets get loaded; however, we must + %% ensure that we do not have clashes with other test suites' loaded modules, + %% which we cannot track. As such, we have to ensure all module names here are + %% unique. + %% + %% This is done by hand; if you see this test suite failing on its own, you + %% probably wrote a test suite that clashes! + Dir = filename:join([?config(priv_dir, Config), atom_to_list(?MODULE), + atom_to_list(Case)]), + InDir = fun(Path) -> filename:join([Dir, Path]) end, + ADep = fake_app(<<"rp_a">>, <<"1.0.0">>, InDir("_build/default/lib/rp_a/")), + BDep = fake_app(<<"rp_b">>, <<"1.0.0">>, InDir("_build/default/lib/rp_b/")), + CDep = fake_app(<<"rp_c">>, <<"1.0.0">>, InDir("_build/default/lib/rp_c/")), + DDep = fake_app(<<"rp_d">>, <<"1.0.0">>, InDir("_build/default/lib/rp_d/")), + RelxDep = fake_app(<<"relx">>, <<"1.0.0">>, InDir("_build/default/lib/relx/")), + + APlug = fake_app(<<"rp_a">>, <<"1.0.0">>, + InDir("_build/default/plugins/lib/rp_a/")), + RelxPlug = fake_app(<<"relx">>, <<"1.1.1">>, + InDir("_build/default/plugins/lib/relx")), + EPlug = fake_app(<<"rp_e">>, <<"1.0.0">>, + InDir("_build/default/plugins/lib/rp_e/")), + + S0 = rebar_state:new(), + S1 = rebar_state:all_deps(S0, [ADep, BDep, CDep, DDep, RelxDep]), + S2 = rebar_state:all_plugin_deps(S1, [APlug, RelxPlug]), + S3 = rebar_state:code_paths(S2, default, code:get_path()), + S4 = rebar_state:code_paths( + S3, + all_deps, + [rebar_app_info:ebin_dir(A) || A <- [ADep, BDep, CDep, DDep, RelxDep]] + ), + S5 = rebar_state:code_paths( + S4, + all_plugin_deps, + [rebar_app_info:ebin_dir(A) || A <- [APlug, RelxPlug, EPlug]] + ), + [{base_paths, BasePaths}, {root_dir, Dir}, {state, S5} | Config]. + +end_per_testcase(_, Config) -> + %% this is deeply annoying because we interfere with rebar3's own + %% path handling! + rebar_paths:unset_paths([plugins, deps], ?config(state, Config)), + Config. + +fake_app(Name, Vsn, OutDir) -> + {ok, App} = rebar_app_info:new(Name, Vsn, OutDir), + compile_fake_appmod(App), + App. + +compile_fake_appmod(App) -> + OutDir = rebar_app_info:ebin_dir(App), + Vsn = rebar_app_info:original_vsn(App), + Name = rebar_app_info:name(App), + + ok = filelib:ensure_dir(filename:join([OutDir, ".touch"])), + + AppFile = [ + "{application,", Name, ", " + " [{description, \"some app\"}, " + " {vsn, \"", Vsn, "\"}, " + " {modules, [",Name,"]}, " + " {registered, []}, " + " {applications, [stdlib, kernel]} " + " ]}. "], + + ok = file:write_file(filename:join([OutDir, <>]), AppFile), + + Mod = [{attribute, 1, module, binary_to_atom(Name, utf8)}, + {attribute, 2, export, [{f,0}]}, + {function,3,f,0, + [{clause,3, [], [], + [{string,3,OutDir}] + }]} + ], + + {ok, _, Bin} = compile:forms(Mod), + ok = file:write_file(filename:join([OutDir, <>]), Bin). + +clashing_apps(Config) -> + Clashes = rebar_paths:clashing_apps([deps, plugins], + ?config(state, Config)), + ct:pal("Clashes: ~p", [Clashes]), + + ?assertEqual([<<"relx">>, <<"rp_a">>], lists:sort(proplists:get_value(deps, Clashes))), + ?assertEqual(undefined, proplists:get_value(plugins, Clashes)), + ok. + +set_paths(Config) -> + State = ?config(state, Config), + RootDir = filename:split(?config(root_dir, Config)), + rebar_paths:set_paths([plugins, deps], State), + PluginPaths = code:get_path(), + rebar_paths:set_paths([deps, plugins], State), + DepPaths = code:get_path(), + + ?assertEqual( + RootDir ++ ["_build", "default", "plugins", "lib", "rp_a", "ebin"], + find_first_instance("rp_a", PluginPaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "lib", "rp_b", "ebin"], + find_first_instance("rp_b", PluginPaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "lib", "rp_c", "ebin"], + find_first_instance("rp_c", PluginPaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "lib", "rp_d", "ebin"], + find_first_instance("rp_d", PluginPaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "plugins", "lib", "rp_e", "ebin"], + find_first_instance("rp_e", PluginPaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "plugins", "lib", "relx", "ebin"], + find_first_instance("relx", PluginPaths) + ), + + + ?assertEqual( + RootDir ++ ["_build", "default", "lib", "rp_a", "ebin"], + find_first_instance("rp_a", DepPaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "lib", "rp_b", "ebin"], + find_first_instance("rp_b", DepPaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "lib", "rp_c", "ebin"], + find_first_instance("rp_c", DepPaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "lib", "rp_d", "ebin"], + find_first_instance("rp_d", DepPaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "plugins", "lib", "rp_e", "ebin"], + find_first_instance("rp_e", DepPaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "lib", "relx", "ebin"], + find_first_instance("relx", DepPaths) + ), + ok. + +check_modules(Config) -> + State = ?config(state, Config), + RootDir = ?config(root_dir, Config)++"/", + rebar_paths:set_paths([plugins, deps], State), + ct:pal("code:get_path() -> ~p", [code:get_path()]), + + ?assertEqual(RootDir ++ "_build/default/plugins/lib/rp_a/ebin", rp_a:f()), + ct:pal("~p", [catch file:list_dir(RootDir ++ "_build/default/lib/")]), + ct:pal("~p", [catch file:list_dir(RootDir ++ "_build/default/lib/rp_b/")]), + ct:pal("~p", [catch file:list_dir(RootDir ++ "_build/default/lib/rp_b/ebin")]), + ct:pal("~p", [catch b:module_info()]), + ?assertEqual(RootDir ++ "_build/default/lib/rp_b/ebin", rp_b:f()), + ?assertEqual(RootDir ++ "_build/default/lib/rp_c/ebin", rp_c:f()), + ?assertEqual(RootDir ++ "_build/default/lib/rp_d/ebin", rp_d:f()), + ?assertEqual(RootDir ++ "_build/default/plugins/lib/rp_e/ebin", rp_e:f()), + ?assertEqual(RootDir ++ "_build/default/plugins/lib/relx/ebin", relx:f()), + ?assertEqual(3, length(relx:module_info(exports))), % can't replace bundled + + rebar_paths:set_paths([deps, plugins], State), + ct:pal("code:get_path() -> ~p", [code:get_path()]), + + ?assertEqual(RootDir ++ "_build/default/lib/rp_a/ebin", rp_a:f()), + ?assertEqual(RootDir ++ "_build/default/lib/rp_b/ebin", rp_b:f()), + ?assertEqual(RootDir ++ "_build/default/lib/rp_c/ebin", rp_c:f()), + ?assertEqual(RootDir ++ "_build/default/lib/rp_d/ebin", rp_d:f()), + ?assertEqual(RootDir ++ "_build/default/plugins/lib/rp_e/ebin", rp_e:f()), + ?assertEqual(RootDir ++ "_build/default/lib/relx/ebin", relx:f()), + ?assertEqual(3, length(relx:module_info(exports))), % can't replace bundled + + %% once again + rebar_paths:set_paths([plugins, deps], State), + ct:pal("code:get_path() -> ~p", [code:get_path()]), + + ?assertEqual(RootDir ++ "_build/default/plugins/lib/rp_a/ebin", rp_a:f()), + ?assertEqual(RootDir ++ "_build/default/lib/rp_b/ebin", rp_b:f()), + ?assertEqual(RootDir ++ "_build/default/lib/rp_c/ebin", rp_c:f()), + ?assertEqual(RootDir ++ "_build/default/lib/rp_d/ebin", rp_d:f()), + ?assertEqual(RootDir ++ "_build/default/plugins/lib/rp_e/ebin", rp_e:f()), + ?assertEqual(RootDir ++ "_build/default/plugins/lib/relx/ebin", relx:f()), + ?assertEqual(3, length(relx:module_info(exports))), % can't replace bundled + ok. + +find_first_instance(Frag, []) -> + {not_found, Frag}; +find_first_instance(Frag, [Path|Rest]) -> + Frags = filename:split(Path), + case lists:member(Frag, Frags) of + true -> Frags; + false -> find_first_instance(Frag, Rest) + end. diff --git a/test/rebar_plugins_SUITE.erl b/test/rebar_plugins_SUITE.erl index 2d74539..c7a5d51 100644 --- a/test/rebar_plugins_SUITE.erl +++ b/test/rebar_plugins_SUITE.erl @@ -335,7 +335,7 @@ project_plugins(Config) -> rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]), DepName = rebar_test_utils:create_random_name("dep1_"), - PluginName = "compile", + PluginName = "compile_plugin", PluginName2 = "release", Plugins = rebar_test_utils:expand_deps(git, [{PluginName, Vsn, []}, {PluginName2, Vsn, []}]), -- cgit v1.1 From af5cecd8eec9692f43d04ad53c8f28734012b873 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Mon, 8 Oct 2018 12:41:30 -0400 Subject: Clean path code, add tests, add clash detection Some finishing touch to that code --- test/rebar_paths_SUITE.erl | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) (limited to 'test') diff --git a/test/rebar_paths_SUITE.erl b/test/rebar_paths_SUITE.erl index 48a7000..f2832c0 100644 --- a/test/rebar_paths_SUITE.erl +++ b/test/rebar_paths_SUITE.erl @@ -4,11 +4,16 @@ -compile(export_all). all() -> - [%clashing_apps, + [clashing_apps, check_modules, - set_paths + set_paths, + misloaded_mods ]. +%%%%%%%%%%%%%%%%%% +%%% TEST SETUP %%% +%%%%%%%%%%%%%%%%%% + init_per_testcase(Case, Config) -> BasePaths = code:get_path(), %% This test checks that the right module sets get loaded; however, we must @@ -90,13 +95,17 @@ compile_fake_appmod(App) -> {ok, _, Bin} = compile:forms(Mod), ok = file:write_file(filename:join([OutDir, <>]), Bin). +%%%%%%%%%%%%% +%%% TESTS %%% +%%%%%%%%%%%%% + clashing_apps(Config) -> Clashes = rebar_paths:clashing_apps([deps, plugins], ?config(state, Config)), ct:pal("Clashes: ~p", [Clashes]), ?assertEqual([<<"relx">>, <<"rp_a">>], lists:sort(proplists:get_value(deps, Clashes))), - ?assertEqual(undefined, proplists:get_value(plugins, Clashes)), + ?assertEqual([], proplists:get_value(plugins, Clashes)), ok. set_paths(Config) -> @@ -201,6 +210,28 @@ check_modules(Config) -> ?assertEqual(3, length(relx:module_info(exports))), % can't replace bundled ok. +misloaded_mods(_Config) -> + Res = rebar_paths:misloaded_modules( + [a,b,c,d,e,f], + ["/1/2/3/4", + "/1/2/4", + "/2/1/1", + "/3/4/5"], + [{a, "/0/1/2/file.beam"}, + {aa, "/1/2/3/4/file.beam"}, + {b, "/1/2/3/4/file.beam"}, + {c, "/2/1/file.beam"}, + {f, preloaded}, + {d, "/3/5/7/file.beam"}, + {e, "/3/4/5/file.beam"}] + ), + ?assertEqual([a,c,d], Res), + ok. + +%%%%%%%%%%%%%%% +%%% HELPERS %%% +%%%%%%%%%%%%%%% + find_first_instance(Frag, []) -> {not_found, Frag}; find_first_instance(Frag, [Path|Rest]) -> -- cgit v1.1 From dada4e36e6d9a5c4b41bbe1f68389520e7c59ace Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Thu, 11 Oct 2018 08:38:37 -0400 Subject: Optimize path handling - Only set paths that need to be put as a priority - Clean up paths before leaving API mode The first point accounted for some performance cost, but the latter one explains the 40% overhead in test runs: since rebar3 calls rebar3 a lot with a bunch of fake apps, and that the new mechanism for path handling by default does not _remove_ paths, it just _orders_ them, we would end up in a situation where as the tests ran, more and more fake paths would get added to the VM. By the time the run was over, all path handling would take longer since more paths needed filtering every time. By resetting paths at the end of an API run, we prevent a given 'project' from polluting another one's runtime and performance once the API successfully returns. --- test/rebar_paths_SUITE.erl | 2 -- 1 file changed, 2 deletions(-) (limited to 'test') diff --git a/test/rebar_paths_SUITE.erl b/test/rebar_paths_SUITE.erl index f2832c0..96cda45 100644 --- a/test/rebar_paths_SUITE.erl +++ b/test/rebar_paths_SUITE.erl @@ -212,13 +212,11 @@ check_modules(Config) -> misloaded_mods(_Config) -> Res = rebar_paths:misloaded_modules( - [a,b,c,d,e,f], ["/1/2/3/4", "/1/2/4", "/2/1/1", "/3/4/5"], [{a, "/0/1/2/file.beam"}, - {aa, "/1/2/3/4/file.beam"}, {b, "/1/2/3/4/file.beam"}, {c, "/2/1/file.beam"}, {f, preloaded}, -- cgit v1.1