From a5fc48bc52049767fd74e7652eb6cf827c9e0e3f Mon Sep 17 00:00:00 2001 From: Pablo Costas Date: Sun, 13 Sep 2020 23:05:48 +0200 Subject: [PATCH 1/3] Change xref paths to only those of runtime deps This commit changes mainly the `rebar_paths` module by adding a new `runtime` target. When using that target, `rebar_paths:get_apps/2` will, for each project app, find all their `applications` and `included_applicactions` and filter those that could be found in rebar3's state. --- src/rebar_paths.erl | 33 ++++++++++++++++++++++++++++++--- src/rebar_prv_xref.erl | 2 +- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/rebar_paths.erl b/src/rebar_paths.erl index 160f9fa2..a0f5cedf 100644 --- a/src/rebar_paths.erl +++ b/src/rebar_paths.erl @@ -51,12 +51,14 @@ normalize_targets(List) -> %% Plan for the eventuality of getting values piped in %% from future versions of rebar3, possibly from plugins and so on, %% which means we'd risk failing kind of violently. We only support - %% deps and plugins + %% deps, plugins and runtime deps. TmpList = lists:foldl( fun(deps, [deps | _] = Acc) -> Acc; (plugins, [plugins | _] = Acc) -> Acc; + (runtime, [runtime | _] = Acc) -> Acc; (deps, Acc) -> [deps | Acc -- [deps]]; (plugins, Acc) -> [plugins | Acc -- [plugins]]; + (runtime, Acc) -> [runtime | Acc -- [runtime]]; (_, Acc) -> Acc end, [], @@ -196,7 +198,9 @@ app_groups(Targets, State) -> get_paths(deps, State) -> rebar_state:code_paths(State, all_deps); get_paths(plugins, State) -> - rebar_state:code_paths(State, all_plugin_deps). + rebar_state:code_paths(State, all_plugin_deps); +get_paths(runtime, State) -> + rebar_state:code_paths(State, all_deps). get_apps(deps, State) -> %% The code paths for deps also include the top level apps @@ -208,4 +212,27 @@ get_apps(deps, State) -> end ++ rebar_state:all_deps(State); get_apps(plugins, State) -> - rebar_state:all_plugin_deps(State). + rebar_state:all_plugin_deps(State); +get_apps(runtime, State) -> + %% We get all project apps and for each of them we find + %% their runtime deps (i.e., `applications' and `included_applications'). + ProjectApps = rebar_state:project_apps(State), + RuntimeApps = + [begin + Apps = rebar_app_info:applications(App), + IncludedApps = rebar_app_info:included_applications(App), + lists:foldl( + fun(AppName0, Acc) -> + %% If the App is an atom convert it to a binary, otherwise leave it as it is. + AppName1 = if is_atom(AppName0) -> atom_to_binary(AppName0, utf8); + true -> AppName0 + end, + %% We only care about those apps we could find in the `State'. + case rebar_app_utils:find(AppName1, ProjectApps) of + {ok, AppInfo} -> [AppInfo|Acc]; + error -> Acc + end + end, [App], Apps ++ IncludedApps) + end || App <- ProjectApps], + %% We get rid of duplicated apps from the flattened list. + lists:usort(lists:flatten(RuntimeApps)). \ No newline at end of file diff --git a/src/rebar_prv_xref.erl b/src/rebar_prv_xref.erl index b7c4fec2..a720d846 100644 --- a/src/rebar_prv_xref.erl +++ b/src/rebar_prv_xref.erl @@ -36,7 +36,7 @@ init(State) -> -spec do(rebar_state:t()) -> {ok, rebar_state:t()} | {error, string()}. do(State) -> - rebar_paths:set_paths([deps], State), + rebar_paths:set_paths([runtime], State), XrefChecks = prepare(State), XrefIgnores = rebar_state:get(State, xref_ignores, []), %% Run xref checks From 7cb579349242adf42e8513367b92132bb8ff1f10 Mon Sep 17 00:00:00 2001 From: Pablo Costas Date: Mon, 14 Sep 2020 20:35:53 +0200 Subject: [PATCH 2/3] Fix runtime apps paths --- src/rebar_paths.erl | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/rebar_paths.erl b/src/rebar_paths.erl index a0f5cedf..6b8d6a30 100644 --- a/src/rebar_paths.erl +++ b/src/rebar_paths.erl @@ -200,7 +200,8 @@ get_paths(deps, State) -> get_paths(plugins, State) -> rebar_state:code_paths(State, all_plugin_deps); get_paths(runtime, State) -> - rebar_state:code_paths(State, all_deps). + RuntimeApps = get_apps(runtime, State), + [rebar_app_info:ebin_dir(App) || App <- RuntimeApps]. get_apps(deps, State) -> %% The code paths for deps also include the top level apps @@ -217,22 +218,26 @@ get_apps(runtime, State) -> %% We get all project apps and for each of them we find %% their runtime deps (i.e., `applications' and `included_applications'). ProjectApps = rebar_state:project_apps(State), - RuntimeApps = - [begin - Apps = rebar_app_info:applications(App), - IncludedApps = rebar_app_info:included_applications(App), - lists:foldl( - fun(AppName0, Acc) -> - %% If the App is an atom convert it to a binary, otherwise leave it as it is. - AppName1 = if is_atom(AppName0) -> atom_to_binary(AppName0, utf8); - true -> AppName0 - end, - %% We only care about those apps we could find in the `State'. - case rebar_app_utils:find(AppName1, ProjectApps) of - {ok, AppInfo} -> [AppInfo|Acc]; - error -> Acc - end - end, [App], Apps ++ IncludedApps) - end || App <- ProjectApps], - %% We get rid of duplicated apps from the flattened list. - lists:usort(lists:flatten(RuntimeApps)). \ No newline at end of file + AppsList = rebar_state:project_apps(State) ++ rebar_state:all_deps(State), + get_runtime_apps(ProjectApps, sets:new(), AppsList). + +get_runtime_apps([], RuntimeApps, _AppsList) -> sets:to_list(RuntimeApps); +%% We skip those apps that are not AppInfos. +get_runtime_apps([App|Rest], AppsAcc0, AppsList) when is_atom(App) orelse is_binary(App) -> + get_runtime_apps(Rest, AppsAcc0, AppsList); +get_runtime_apps([App|Rest0], AppsAcc0, AppsList) -> + Apps = rebar_app_info:applications(App), + IncludedApps = rebar_app_info:included_applications(App), + TotalApps0 = [atom_to_binary(A, utf8) || A <- (Apps ++ IncludedApps)], + TotalApps = TotalApps0 -- [rebar_app_info:name(A) || A <- sets:to_list(AppsAcc0)], + + {Rest1, AppsAcc1} = + lists:foldl( + fun(AppName, {Rest, Acc}) -> + %% We only care about those apps we ccould find in the state. + case rebar_app_utils:find(AppName, AppsList) of + {ok, AppInfo} -> {[AppInfo|Rest], sets:add_element(AppInfo, Acc)}; + error -> {Rest, Acc} + end + end, {Rest0, sets:add_element(App, AppsAcc0)}, TotalApps), + get_runtime_apps(Rest1 ++ TotalApps, AppsAcc1, AppsList). \ No newline at end of file From 24c4d78b5faf0385617defd00dbfe9b22806beb9 Mon Sep 17 00:00:00 2001 From: Pablo Costas Date: Thu, 17 Sep 2020 20:24:12 +0200 Subject: [PATCH 3/3] Update rebar_paths_SUITE with checks for runtime applications This commit adds new checks in the `set_paths` test case related to runtime apps and their path handling with the `runtime` target in `rebar_paths`. These new checks also hope to increase the coverage of the changes done to the xref provider in this same PR (#2354). This commit also adds some new helper functions to help deal with future changes related to these new checks (i.e., changes related to the runtime apps), besides fixing a problem when looking for the first instace of an app/dep/plugin in the path while testing. --- test/rebar_paths_SUITE.erl | 106 ++++++++++++++++++++++++++++--------- 1 file changed, 81 insertions(+), 25 deletions(-) diff --git a/test/rebar_paths_SUITE.erl b/test/rebar_paths_SUITE.erl index 96cda454..67d92739 100644 --- a/test/rebar_paths_SUITE.erl +++ b/test/rebar_paths_SUITE.erl @@ -39,21 +39,25 @@ init_per_testcase(Case, Config) -> EPlug = fake_app(<<"rp_e">>, <<"1.0.0">>, InDir("_build/default/plugins/lib/rp_e/")), + TopApp0 = fake_app(<<"top_app">>, <<"1.0.0">>, InDir("_build/default/lib/top_app/"), [<<"rp_a">>, <<"rp_b">>]), + TopApp1 = rebar_app_info:applications(TopApp0, ['rp_a', 'rp_b']), + 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, + S3 = rebar_state:project_apps(S2, [TopApp1]), + S4 = rebar_state:code_paths(S3, default, code:get_path()), + S5 = rebar_state:code_paths( + S4, all_deps, [rebar_app_info:ebin_dir(A) || A <- [ADep, BDep, CDep, DDep, RelxDep]] ), - S5 = rebar_state:code_paths( - S4, + S6 = rebar_state:code_paths( + S5, all_plugin_deps, [rebar_app_info:ebin_dir(A) || A <- [APlug, RelxPlug, EPlug]] ), - [{base_paths, BasePaths}, {root_dir, Dir}, {state, S5} | Config]. + [{base_paths, BasePaths}, {root_dir, Dir}, {state, S6} | Config]. end_per_testcase(_, Config) -> %% this is deeply annoying because we interfere with rebar3's own @@ -63,13 +67,19 @@ end_per_testcase(_, Config) -> fake_app(Name, Vsn, OutDir) -> {ok, App} = rebar_app_info:new(Name, Vsn, OutDir), - compile_fake_appmod(App), + compile_fake_appmod(App, []), App. -compile_fake_appmod(App) -> +fake_app(Name, Vsn, OutDir, Apps) -> + {ok, App} = rebar_app_info:new(Name, Vsn, OutDir), + compile_fake_appmod(App, Apps), + App. + +compile_fake_appmod(App, Apps) -> OutDir = rebar_app_info:ebin_dir(App), Vsn = rebar_app_info:original_vsn(App), Name = rebar_app_info:name(App), + AppsStr = apps_to_str(Apps), ok = filelib:ensure_dir(filename:join([OutDir, ".touch"])), @@ -79,7 +89,7 @@ compile_fake_appmod(App) -> " {vsn, \"", Vsn, "\"}, " " {modules, [",Name,"]}, " " {registered, []}, " - " {applications, [stdlib, kernel]} " + " {applications, [" ++ AppsStr ++ "]} " " ]}. "], ok = file:write_file(filename:join([OutDir, <>]), AppFile), @@ -111,60 +121,94 @@ clashing_apps(Config) -> set_paths(Config) -> State = ?config(state, Config), RootDir = filename:split(?config(root_dir, Config)), + %% Take a snapshot of runtime deps being set; to see if your test is valid, this should fail + %% when you set the [deps] paths here + rebar_paths:set_paths([runtime], State), + RuntimePaths = code:get_path(), + %% Revert back to regular dep paths rebar_paths:set_paths([plugins, deps], State), PluginPaths = code:get_path(), rebar_paths:set_paths([deps, plugins], State), DepPaths = code:get_path(), + %% Plugin related paths checks ?assertEqual( RootDir ++ ["_build", "default", "plugins", "lib", "rp_a", "ebin"], - find_first_instance("rp_a", PluginPaths) + find_first_instance(RootDir, "rp_a", PluginPaths) ), ?assertEqual( RootDir ++ ["_build", "default", "lib", "rp_b", "ebin"], - find_first_instance("rp_b", PluginPaths) + find_first_instance(RootDir, "rp_b", PluginPaths) ), ?assertEqual( RootDir ++ ["_build", "default", "lib", "rp_c", "ebin"], - find_first_instance("rp_c", PluginPaths) + find_first_instance(RootDir, "rp_c", PluginPaths) ), ?assertEqual( RootDir ++ ["_build", "default", "lib", "rp_d", "ebin"], - find_first_instance("rp_d", PluginPaths) + find_first_instance(RootDir, "rp_d", PluginPaths) ), ?assertEqual( RootDir ++ ["_build", "default", "plugins", "lib", "rp_e", "ebin"], - find_first_instance("rp_e", PluginPaths) + find_first_instance(RootDir, "rp_e", PluginPaths) ), ?assertEqual( RootDir ++ ["_build", "default", "plugins", "lib", "relx", "ebin"], - find_first_instance("relx", PluginPaths) + find_first_instance(RootDir, "relx", PluginPaths) ), + %% Dependency related paths checks ?assertEqual( RootDir ++ ["_build", "default", "lib", "rp_a", "ebin"], - find_first_instance("rp_a", DepPaths) + find_first_instance(RootDir, "rp_a", DepPaths) ), ?assertEqual( RootDir ++ ["_build", "default", "lib", "rp_b", "ebin"], - find_first_instance("rp_b", DepPaths) + find_first_instance(RootDir, "rp_b", DepPaths) ), ?assertEqual( RootDir ++ ["_build", "default", "lib", "rp_c", "ebin"], - find_first_instance("rp_c", DepPaths) + find_first_instance(RootDir, "rp_c", DepPaths) ), ?assertEqual( RootDir ++ ["_build", "default", "lib", "rp_d", "ebin"], - find_first_instance("rp_d", DepPaths) + find_first_instance(RootDir, "rp_d", DepPaths) ), ?assertEqual( RootDir ++ ["_build", "default", "plugins", "lib", "rp_e", "ebin"], - find_first_instance("rp_e", DepPaths) + find_first_instance(RootDir, "rp_e", DepPaths) ), ?assertEqual( RootDir ++ ["_build", "default", "lib", "relx", "ebin"], - find_first_instance("relx", DepPaths) + find_first_instance(RootDir, "relx", DepPaths) + ), + + + %% Runtime related paths checks + ?assertEqual( + RootDir ++ ["_build", "default", "lib", "rp_a", "ebin"], + find_first_instance(RootDir, "rp_a", RuntimePaths) + ), + ?assertEqual( + RootDir ++ ["_build", "default", "lib", "rp_b", "ebin"], + find_first_instance(RootDir, "rp_b", RuntimePaths) + ), + ?assertMatch( + {not_found, _}, + find_first_instance(RootDir, "rp_c", RuntimePaths) + ), + ?assertMatch( + {not_found, _}, + find_first_instance(RootDir, "rp_d", RuntimePaths) + ), + ?assertMatch( + {not_found, _}, + find_first_instance(RootDir, "rp_e", RuntimePaths) + ), + ?assertMatch( + {not_found, _}, + find_first_instance(RootDir, "relx", RuntimePaths) ), ok. @@ -230,11 +274,23 @@ misloaded_mods(_Config) -> %%% HELPERS %%% %%%%%%%%%%%%%%% -find_first_instance(Frag, []) -> +find_first_instance(_RootDir, Frag, []) -> {not_found, Frag}; -find_first_instance(Frag, [Path|Rest]) -> +find_first_instance(RootDir, Frag, [Path|Rest]) -> Frags = filename:split(Path), case lists:member(Frag, Frags) of - true -> Frags; - false -> find_first_instance(Frag, Rest) + true -> + %% When testing the runtime deps the paths would have + %% apps such as `relx' that were not from within the root dir. + case re:run(Frags, RootDir) of + nomatch -> find_first_instance(RootDir, Frag, Rest); + {match, _} -> Frags + end; + false -> find_first_instance(RootDir, Frag, Rest) end. + +apps_to_str([]) -> + "stdlib, kernel"; +apps_to_str(Apps) -> + AppsStr = unicode:characters_to_list(lists:join(", ", Apps)), + "stdlib, kernel, " ++ AppsStr. \ No newline at end of file