From d4cd09e165ebe694a2ac14c2a0c41c2233e864ef Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sat, 9 Apr 2022 21:55:04 +0000 Subject: [PATCH 1/9] Add support for project-local plugins This puts the basic scaffolding in place to allow project-local plugin definitions. This, in turn, can be usable for vendoring mechanisms. A few things that are still tricky with this approach: - This is not tested aside from manual execution with a toy case - There is no great way to only making this work with umbrella projects, since plugins are installed before the discovery phase that decides whether a project is an umbrella project or not, so there may be a need for an imperfect heuristic (eg. using project_src_app directories except the project root) - Whether it clashes with things such as global plugins' paths and the general usage of rebar_paths to switch things in and out of visibility for projects. - If dependencies are well handled. - How overrides should work on this one. I'm assuming they shouldn't. - If profiles are working well with this (and whether they even should, since they don't with other plugins) - If it works with recursive plugins and local definitions Manual executions however seem to show that this works. I'm committing and putting it in a draft PR for awareness. --- src/rebar.hrl | 1 + src/rebar_app_discover.erl | 4 +-- src/rebar_app_info.erl | 3 +- src/rebar_dir.erl | 7 ++++ src/rebar_plugins.erl | 73 ++++++++++++++++++++++++++++++++++---- src/rebar_prv_compile.erl | 1 + 6 files changed, 79 insertions(+), 10 deletions(-) diff --git a/src/rebar.hrl b/src/rebar.hrl index 8a7dd82ff..3931620f6 100644 --- a/src/rebar.hrl +++ b/src/rebar.hrl @@ -15,6 +15,7 @@ -define(DEFAULT_BASE_DIR, "_build"). -define(DEFAULT_ROOT_DIR, "."). -define(DEFAULT_PROJECT_APP_DIRS, ["apps/*", "lib/*", "."]). +-define(DEFAULT_PROJECT_PLUGIN_DIRS, ["plugins/*"]). -define(DEFAULT_CHECKOUTS_DIR, "_checkouts"). -define(DEFAULT_CHECKOUTS_OUT_DIR, "checkouts"). -define(DEFAULT_DEPS_DIR, "lib"). diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl index bdb253859..993925625 100644 --- a/src/rebar_app_discover.erl +++ b/src/rebar_app_discover.erl @@ -64,8 +64,8 @@ do(State, LibDirs) -> OutDir = filename:join(DepsDir, Name), AppInfo2 = rebar_app_info:out_dir(AppInfo1, OutDir), ProjectDeps1 = lists:delete(Name, ProjectDeps), - rebar_state:project_apps(StateAcc1 - ,rebar_app_info:deps(AppInfo2, ProjectDeps1)); + rebar_state:project_apps(StateAcc1, + rebar_app_info:deps(AppInfo2, ProjectDeps1)); false -> ?INFO("Ignoring ~ts", [Name]), StateAcc diff --git a/src/rebar_app_info.erl b/src/rebar_app_info.erl index 4f78f4312..3bc6ab4bb 100644 --- a/src/rebar_app_info.erl +++ b/src/rebar_app_info.erl @@ -638,7 +638,8 @@ valid(#app_info_t{valid=Valid}) -> %% @doc sets whether the app is valid (built) or not. If left unset, %% rebar3 will do the detection of the status itself. --spec valid(t(), boolean()) -> t(). +%% Explicitly setting the value to `undefined' can force a re-evaluation. +-spec valid(t(), boolean() | undefined) -> t(). valid(AppInfo=#app_info_t{}, Valid) -> AppInfo#app_info_t{valid=Valid}. diff --git a/src/rebar_dir.erl b/src/rebar_dir.erl index abd412254..45bfe861a 100644 --- a/src/rebar_dir.erl +++ b/src/rebar_dir.erl @@ -13,6 +13,7 @@ checkouts_out_dir/2, plugins_dir/1, lib_dirs/1, + project_plugin_dirs/1, home_dir/0, global_config_dir/1, global_config/1, @@ -127,6 +128,12 @@ plugins_dir(State) -> lib_dirs(State) -> rebar_state:get(State, project_app_dirs, ?DEFAULT_PROJECT_APP_DIRS). +%% @doc returns the list of relative path where the project plugins can +%% be located. +-spec project_plugin_dirs(rebar_state:t()) -> [file:filename_all()]. +project_plugin_dirs(State) -> + rebar_state:get(State, project_plugin_dirs, ?DEFAULT_PROJECT_PLUGIN_DIRS). + %% @doc returns the user's home directory. -spec home_dir() -> file:filename_all(). home_dir() -> diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl index f2d22233b..9be41ffd3 100644 --- a/src/rebar_plugins.erl +++ b/src/rebar_plugins.erl @@ -94,10 +94,11 @@ handle_plugins(Profile, Plugins, State, Upgrade) -> Locks = rebar_state:lock(State), DepsDir = rebar_state:get(State, deps_dir, ?DEFAULT_DEPS_DIR), State1 = rebar_state:set(State, deps_dir, ?DEFAULT_PLUGINS_DIR), + SrcPlugins = discover_plugins(Plugins, State), %% Install each plugin individually so if one fails to install it doesn't effect the others {_PluginProviders, State2} = lists:foldl(fun(Plugin, {PluginAcc, StateAcc}) -> - {NewPlugins, NewState} = handle_plugin(Profile, Plugin, StateAcc, Upgrade), + {NewPlugins, NewState} = handle_plugin(Profile, Plugin, StateAcc, SrcPlugins, Upgrade), NewState1 = rebar_state:create_logic_providers(NewPlugins, NewState), {PluginAcc++NewPlugins, NewState1} end, {[], State1}, Plugins), @@ -106,24 +107,34 @@ handle_plugins(Profile, Plugins, State, Upgrade) -> State3 = rebar_state:set(State2, deps_dir, DepsDir), rebar_state:lock(State3, Locks). -handle_plugin(Profile, Plugin, State, Upgrade) -> +handle_plugin(Profile, Plugin, State, SrcPlugins, Upgrade) -> try - {Apps, State2} = rebar_prv_install_deps:handle_deps_as_profile(Profile, State, [Plugin], Upgrade), - {no_cycle, Sorted} = rebar_prv_install_deps:find_cycles(Apps), + %% Inject top-level src plugins as project apps, so that they get skipped + %% by the installation as already seen + ProjectApps = rebar_state:project_apps(State), + State0 = rebar_state:project_apps(State, SrcPlugins), + %% We however have to pick the deps of top-level apps and promote them + %% directly to make sure they are installed if they were not also at the top level + TopDeps = top_level_deps(SrcPlugins), + %% Install the plugins + {Apps, State1} = rebar_prv_install_deps:handle_deps_as_profile(Profile, State0, [Plugin|TopDeps], Upgrade), + {no_cycle, Sorted} = rebar_prv_install_deps:find_cycles(SrcPlugins++Apps), ToBuild = rebar_prv_install_deps:cull_compile(Sorted, []), + %% Return things to normal + State2 = rebar_state:project_apps(State1, ProjectApps), %% Add already built plugin deps to the code path ToBuildPaths = [rebar_app_info:ebin_dir(A) || A <- ToBuild], - PreBuiltPaths = [Ebin || A <- Apps, + PreBuiltPaths = [Ebin || A <- Sorted, Ebin <- [rebar_app_info:ebin_dir(A)], not lists:member(Ebin, ToBuildPaths)], code:add_pathsa(PreBuiltPaths), %% Build plugin and its deps - build_plugins(ToBuild, Apps, State2), + build_plugins(ToBuild, Sorted, State2), %% Add newly built deps and plugin to code path - State3 = rebar_state:update_all_plugin_deps(State2, Apps), + State3 = rebar_state:update_all_plugin_deps(State2, Sorted), NewCodePaths = [rebar_app_info:ebin_dir(A) || A <- ToBuild], %% Store plugin code paths so we can remove them when compiling project apps @@ -172,3 +183,51 @@ validate_plugin(Plugin) -> end end. +discover_plugins([], _) -> + %% don't search if nothing is declared + []; +discover_plugins(_, State) -> + %% TODO: only support this mode in an umbrella project to avoid cases where + %% this is used in a project intended to be an installed dependency and accidentally + %% relies on vendoring when not intended. + case lists:member(global, rebar_state:current_profiles(State)) of + true -> + []; + false -> + %% Inject source paths for plugins to allow vendoring and umbrella + %% top-level declarations + BaseDir = rebar_state:dir(State), + LibDirs = rebar_dir:project_plugin_dirs(State), + ?DIAGNOSTIC("Discovering local plugins in {project_plugin_dirs, ~p}", [LibDirs]), + Dirs = [filename:join(BaseDir, LibDir) || LibDir <- LibDirs], + RebarOpts = rebar_state:opts(State), + SrcDirs = rebar_dir:src_dirs(RebarOpts, ["src"]), + Found = rebar_app_discover:find_apps(Dirs, SrcDirs, all, State), + ?DIAGNOSTIC(" Found: ~p", [[rebar_app_info:name(F) || F <- Found]]), + PluginsDir = rebar_dir:plugins_dir(State), + SetUp = lists:map(fun(App) -> + Name = rebar_app_info:name(App), + OutDir = filename:join(PluginsDir, Name), + prepare_plugin(rebar_app_info:out_dir(App, OutDir)) + end, Found), + rebar_utils:sort_deps(SetUp) + end. + +prepare_plugin(AppInfo) -> + %% We need to handle plugins as dependencies to avoid re-building them + %% continuously. So here we copy the app directories to the dep location + %% and then change the AppInfo record to be redirected to the dep location. + AppDir = rebar_app_info:dir(AppInfo), + OutDir = rebar_app_info:out_dir(AppInfo), + rebar_prv_compile:copy_app_dirs(AppInfo, AppDir, OutDir), + Relocated = rebar_app_info:dir(AppInfo, OutDir), + %% Force a revalidation from the new paths + rebar_app_info:valid(Relocated, undefined). + +top_level_deps(Apps) -> + RawDeps = lists:foldl(fun(App, Acc) -> + %% Only support the profiles we would with regular plugins? + rebar_app_info:get(App, {deps, default}, []) ++ + rebar_app_info:get(App, {deps, prod}, []) ++ Acc + end, [], Apps), + rebar_utils:tup_dedup(RawDeps). diff --git a/src/rebar_prv_compile.erl b/src/rebar_prv_compile.erl index bf4b0152b..650de9332 100644 --- a/src/rebar_prv_compile.erl +++ b/src/rebar_prv_compile.erl @@ -7,6 +7,7 @@ format_error/1]). -export([compile/2, compile/3, compile/4]). +-export([copy_app_dirs/3]). -include_lib("providers/include/providers.hrl"). -include("rebar.hrl"). From bbbe56151de985121c981cb560e7f209cafd0da8 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 10 Apr 2022 00:30:21 +0000 Subject: [PATCH 2/9] Fix local plugin external plugin dependency This sounds weird as hell, but essentially, when a local plugin relies on an external plugin, make sure that we fetch and build that one in an orderly manner. This was found by adding initial tests. --- src/rebar_plugins.erl | 5 ++- test/rebar_plugins_SUITE.erl | 62 ++++++++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl index 9be41ffd3..58df32808 100644 --- a/src/rebar_plugins.erl +++ b/src/rebar_plugins.erl @@ -227,7 +227,10 @@ prepare_plugin(AppInfo) -> top_level_deps(Apps) -> RawDeps = lists:foldl(fun(App, Acc) -> %% Only support the profiles we would with regular plugins? + rebar_app_info:get(App, {plugins, default}, []) ++ + rebar_app_info:get(App, {plugins, prod}, []) ++ rebar_app_info:get(App, {deps, default}, []) ++ - rebar_app_info:get(App, {deps, prod}, []) ++ Acc + rebar_app_info:get(App, {deps, prod}, []) ++ + Acc end, [], Apps), rebar_utils:tup_dedup(RawDeps). diff --git a/test/rebar_plugins_SUITE.erl b/test/rebar_plugins_SUITE.erl index a093719ed..0db468e90 100644 --- a/test/rebar_plugins_SUITE.erl +++ b/test/rebar_plugins_SUITE.erl @@ -16,7 +16,10 @@ sub_app_plugins/1, sub_app_plugin_overrides/1, project_plugins/1, - use_checkout_plugins/1]). + use_checkout_plugins/1, + %% project-local plugins + complex_local_plugins/1 + ]). -include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). @@ -39,7 +42,8 @@ end_per_testcase(_, _Config) -> all() -> [compile_plugins, compile_global_plugins, complex_plugins, list, upgrade, upgrade_project_plugin, - sub_app_plugins, sub_app_plugin_overrides, project_plugins, use_checkout_plugins]. + sub_app_plugins, sub_app_plugin_overrides, project_plugins, use_checkout_plugins, + complex_local_plugins]. %% Tests that compiling a project installs and compiles the plugins of deps compile_plugins(Config) -> @@ -412,3 +416,57 @@ use_checkout_plugins(Config) -> Config, RConf, ["checkedout"], {ok, []} )). + +complex_local_plugins(Config) -> + UmbrellaDir = ?config(apps, Config), + AppName = rebar_test_utils:create_random_name("app1_"), + AppDir = filename:join([UmbrellaDir, "apps", AppName]), + LocalPluginName = rebar_test_utils:create_random_name("localplugin1_"), + PluginDir = filename:join([UmbrellaDir, "plugins", LocalPluginName]), + + meck:new(rebar_dir, [passthrough]), + + Vsn = rebar_test_utils:create_random_vsn(), + Vsn2 = rebar_test_utils:create_random_vsn(), + rebar_test_utils:create_app(AppDir, AppName, Vsn, [kernel, stdlib]), + rebar_test_utils:create_app(PluginDir, LocalPluginName, Vsn, [kernel, stdlib]), + + DepName = rebar_test_utils:create_random_name("dep1_"), + DepName2 = rebar_test_utils:create_random_name("dep2_"), + DepName3 = rebar_test_utils:create_random_name("dep3_"), + DepName4 = rebar_test_utils:create_random_name("dep4_"), + PluginName = rebar_test_utils:create_random_name("plugin1_"), + + Deps = rebar_test_utils:expand_deps(git, [{PluginName, Vsn2, [{DepName2, Vsn, + [{DepName3, Vsn, []}]}]} + ,{DepName, Vsn, [{DepName4, Vsn, []}]}]), + {SrcDeps, _} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}]), + + RootConfFile = + rebar_test_utils:create_config(UmbrellaDir, + [{plugins, [list_to_atom(LocalPluginName)]}]), + rebar_test_utils:create_config( + PluginDir, + [{deps, [{list_to_atom(DepName), + {git, "http://site.com/user/"++DepName++".git", {tag, Vsn}}}]}, + {plugins, [{list_to_atom(PluginName), + {git, "http://site.com/user/"++PluginName++".git", {tag, Vsn2}}} + ]}] + ), + {ok, RConf} = file:consult(RootConfFile), + + %% Build with deps. + rebar_test_utils:run_and_check( + Config, RConf, ["compile"], + {ok, [{app, AppName}, + {plugin, LocalPluginName}, + {plugin, PluginName, Vsn2}, + %% deps of plugins also remain plugin apps + {plugin, DepName2}, + {plugin, DepName3}, + {plugin, DepName4}, + {plugin, DepName}]} + ), + + meck:unload(rebar_dir). From 72ab1e9842e68b88ab82e7d6d0279ee1872f5338 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 10 Apr 2022 00:50:14 +0000 Subject: [PATCH 3/9] Local plugins can't declare project plugins This adds a test confirming the implemented behaviour, which states that a local plugin that declares a project_plugin won't see it fetched from the root project. This would generally be a thing ignored by a dependency and it makes no sense to have it declared in the local plugin and also impact the top project. --- src/rebar_plugins.erl | 10 +++--- test/rebar_plugins_SUITE.erl | 61 ++++++++++++++++++++++++++++++++++-- test/rebar_test_utils.erl | 8 +++++ 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl index 58df32808..4242b7865 100644 --- a/src/rebar_plugins.erl +++ b/src/rebar_plugins.erl @@ -227,10 +227,10 @@ prepare_plugin(AppInfo) -> top_level_deps(Apps) -> RawDeps = lists:foldl(fun(App, Acc) -> %% Only support the profiles we would with regular plugins? - rebar_app_info:get(App, {plugins, default}, []) ++ - rebar_app_info:get(App, {plugins, prod}, []) ++ - rebar_app_info:get(App, {deps, default}, []) ++ - rebar_app_info:get(App, {deps, prod}, []) ++ - Acc + lists:append([rebar_app_info:get(App, Key, []) + || Key <- [{plugins, default}, + {plugins, prod}, + {deps, default}, + {deps, prod}]]) ++ Acc end, [], Apps), rebar_utils:tup_dedup(RawDeps). diff --git a/test/rebar_plugins_SUITE.erl b/test/rebar_plugins_SUITE.erl index 0db468e90..8e1528408 100644 --- a/test/rebar_plugins_SUITE.erl +++ b/test/rebar_plugins_SUITE.erl @@ -18,7 +18,8 @@ project_plugins/1, use_checkout_plugins/1, %% project-local plugins - complex_local_plugins/1 + complex_local_plugins/1, + complex_local_project_plugins/1 ]). -include_lib("common_test/include/ct.hrl"). @@ -43,7 +44,7 @@ end_per_testcase(_, _Config) -> all() -> [compile_plugins, compile_global_plugins, complex_plugins, list, upgrade, upgrade_project_plugin, sub_app_plugins, sub_app_plugin_overrides, project_plugins, use_checkout_plugins, - complex_local_plugins]. + complex_local_plugins, complex_local_project_plugins]. %% Tests that compiling a project installs and compiles the plugins of deps compile_plugins(Config) -> @@ -470,3 +471,59 @@ complex_local_plugins(Config) -> ), meck:unload(rebar_dir). + +%% Project plugins aren't supported, they should just be at the project +%% root instead. +complex_local_project_plugins(Config) -> + UmbrellaDir = ?config(apps, Config), + AppName = rebar_test_utils:create_random_name("app1_"), + AppDir = filename:join([UmbrellaDir, "apps", AppName]), + LocalPluginName = rebar_test_utils:create_random_name("localplugin1_"), + PluginDir = filename:join([UmbrellaDir, "plugins", LocalPluginName]), + + meck:new(rebar_dir, [passthrough]), + + Vsn = rebar_test_utils:create_random_vsn(), + Vsn2 = rebar_test_utils:create_random_vsn(), + rebar_test_utils:create_app(AppDir, AppName, Vsn, [kernel, stdlib]), + rebar_test_utils:create_app(PluginDir, LocalPluginName, Vsn, [kernel, stdlib]), + + DepName = rebar_test_utils:create_random_name("dep1_"), + DepName2 = rebar_test_utils:create_random_name("dep2_"), + DepName3 = rebar_test_utils:create_random_name("dep3_"), + DepName4 = rebar_test_utils:create_random_name("dep4_"), + PluginName = rebar_test_utils:create_random_name("plugin1_"), + + Deps = rebar_test_utils:expand_deps(git, [{PluginName, Vsn2, [{DepName2, Vsn, + [{DepName3, Vsn, []}]}]} + ,{DepName, Vsn, [{DepName4, Vsn, []}]}]), + {SrcDeps, _} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}]), + + RootConfFile = + rebar_test_utils:create_config(UmbrellaDir, + [{project_plugins, [list_to_atom(LocalPluginName)]}]), + rebar_test_utils:create_config( + PluginDir, + [{deps, [{list_to_atom(DepName), + {git, "http://site.com/user/"++DepName++".git", {tag, Vsn}}}]}, + {project_plugins, [{list_to_atom(PluginName), + {git, "http://site.com/user/"++PluginName++".git", {tag, Vsn2}}} + ]}] + ), + {ok, RConf} = file:consult(RootConfFile), + + %% Build with deps. + rebar_test_utils:run_and_check( + Config, RConf, ["compile"], + {ok, [{app, AppName}, + {plugin, LocalPluginName}, + {plugin_not_exist, PluginName}, + {plugin_not_exist, DepName2}, + {plugin_not_exist, DepName3}, + {plugin, DepName4}, + {plugin, DepName}]} + ), + + meck:unload(rebar_dir). + diff --git a/test/rebar_test_utils.erl b/test/rebar_test_utils.erl index e115bc054..0a4d2f683 100644 --- a/test/rebar_test_utils.erl +++ b/test/rebar_test_utils.erl @@ -351,6 +351,14 @@ check_results(AppDir, Expected, ProfileRun, State) -> ?assertEqual(iolist_to_binary(Vsn), iolist_to_binary(rebar_app_info:original_vsn(App))) end + ; ({plugin_not_exist, Name}) -> + ct:pal("Plugin Not Exist Name: ~p", [Name]), + case lists:keyfind(Name, 1, PluginsNames) of + false -> + ok; + {Name, _App} -> + error({plugin_found, Name}) + end ; ({global_plugin, Name}) -> ct:pal("Global Plugin Name: ~p", [Name]), ?assertNotEqual(false, lists:keyfind(Name, 1, GlobalPluginsNames)) From c835049c6367de2d7e8b673238e1caef091f7c29 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 10 Apr 2022 01:45:08 +0000 Subject: [PATCH 4/9] Prevent non-umbrella project-local plugins Only umbrella projects can have these. --- src/rebar_plugins.erl | 25 ++++++++++++++-- test/rebar_plugins_SUITE.erl | 57 ++++++++++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl index 4242b7865..7270f17a6 100644 --- a/src/rebar_plugins.erl +++ b/src/rebar_plugins.erl @@ -187,10 +187,11 @@ discover_plugins([], _) -> %% don't search if nothing is declared []; discover_plugins(_, State) -> - %% TODO: only support this mode in an umbrella project to avoid cases where + %% only support this mode in an umbrella project to avoid cases where %% this is used in a project intended to be an installed dependency and accidentally - %% relies on vendoring when not intended. - case lists:member(global, rebar_state:current_profiles(State)) of + %% relies on vendoring when not intended. Also skip for global plugins, this would + %% make no sense. + case lists:member(global, rebar_state:current_profiles(State)) orelse not is_umbrella(State) of true -> []; false -> @@ -213,6 +214,24 @@ discover_plugins(_, State) -> rebar_utils:sort_deps(SetUp) end. +is_umbrella(State) -> + %% We can't know if this is an umbrella project before running app discovery, + %% but plugins are installed before app discovery. So we do a heuristic. + %% The lib dirs we search contain things such as apps/, lib/, etc. + %% which contain sub-applications. Then there's a final search for the + %% local directory ("."), which finds the top-level app in a non-umbrella + %% project. + %% + %% So what we do here is look for the library directories without the ".", + %% and if none of these paths exist but one of the src_dirs exist, then + %% we know this is not an umbrella application. + Root = rebar_dir:root_dir(State), + LibPaths = lists:usort(rebar_dir:lib_dirs(State)) -- ["."], + SrcPaths = rebar_dir:src_dirs(rebar_state:opts(State), ["src"]), + lists:any(fun(Dir) -> [] == filelib:wildcard(filename:join(Root, Dir)) end, LibPaths) + andalso + lists:all(fun(Dir) -> not filelib:is_dir(filename:join(Root, Dir)) end, SrcPaths). + prepare_plugin(AppInfo) -> %% We need to handle plugins as dependencies to avoid re-building them %% continuously. So here we copy the app directories to the dep location diff --git a/test/rebar_plugins_SUITE.erl b/test/rebar_plugins_SUITE.erl index 8e1528408..e0ba3866b 100644 --- a/test/rebar_plugins_SUITE.erl +++ b/test/rebar_plugins_SUITE.erl @@ -19,7 +19,8 @@ use_checkout_plugins/1, %% project-local plugins complex_local_plugins/1, - complex_local_project_plugins/1 + complex_local_project_plugins/1, + local_plugins_umbrella_only/1 ]). -include_lib("common_test/include/ct.hrl"). @@ -44,7 +45,7 @@ end_per_testcase(_, _Config) -> all() -> [compile_plugins, compile_global_plugins, complex_plugins, list, upgrade, upgrade_project_plugin, sub_app_plugins, sub_app_plugin_overrides, project_plugins, use_checkout_plugins, - complex_local_plugins, complex_local_project_plugins]. + complex_local_plugins, complex_local_project_plugins, local_plugins_umbrella_only]. %% Tests that compiling a project installs and compiles the plugins of deps compile_plugins(Config) -> @@ -527,3 +528,55 @@ complex_local_project_plugins(Config) -> meck:unload(rebar_dir). +local_plugins_umbrella_only(Config) -> + BaseDir = ?config(apps, Config), + AppName = rebar_test_utils:create_random_name("app1_"), + AppDir = BaseDir, + LocalPluginName = rebar_test_utils:create_random_name("localplugin1_"), + PluginDir = filename:join([BaseDir, "plugins", LocalPluginName]), + + meck:new(rebar_dir, [passthrough]), + + Vsn = rebar_test_utils:create_random_vsn(), + Vsn2 = rebar_test_utils:create_random_vsn(), + rebar_test_utils:create_app(AppDir, AppName, Vsn, [kernel, stdlib]), + rebar_test_utils:create_app(PluginDir, LocalPluginName, Vsn, [kernel, stdlib]), + + DepName = rebar_test_utils:create_random_name("dep1_"), + DepName2 = rebar_test_utils:create_random_name("dep2_"), + DepName3 = rebar_test_utils:create_random_name("dep3_"), + DepName4 = rebar_test_utils:create_random_name("dep4_"), + PluginName = rebar_test_utils:create_random_name("plugin1_"), + + Deps = rebar_test_utils:expand_deps(git, [{PluginName, Vsn2, [{DepName2, Vsn, + [{DepName3, Vsn, []}]}]} + ,{DepName, Vsn, [{DepName4, Vsn, []}]}]), + {SrcDeps, _} = rebar_test_utils:flat_deps(Deps), + mock_git_resource:mock([{deps, SrcDeps}]), + + RootConfFile = + rebar_test_utils:create_config(BaseDir, + [{plugins, [list_to_atom(LocalPluginName)]}]), + rebar_test_utils:create_config( + PluginDir, + [{deps, [{list_to_atom(DepName), + {git, "http://site.com/user/"++DepName++".git", {tag, Vsn}}}]}, + {plugins, [{list_to_atom(PluginName), + {git, "http://site.com/user/"++PluginName++".git", {tag, Vsn2}}} + ]}] + ), + {ok, RConf} = file:consult(RootConfFile), + + %% Build with deps. + rebar_test_utils:run_and_check( + Config, RConf, ["compile"], + {ok, [{app, AppName}, + {plugin_not_exist, LocalPluginName}, + {plugin_not_exist, PluginName}, + {plugin_not_exist, DepName2}, + {plugin_not_exist, DepName3}, + {plugin_not_exist, DepName4}, + {plugin_not_exist, DepName}]} + ), + + meck:unload(rebar_dir). From 206c7a9860c5389d181c615c9b2939b1acdf8690 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 10 Apr 2022 04:31:05 +0000 Subject: [PATCH 5/9] Skip upgrades on locally-defined plugins Otherwise you can run into funny situations where externally-defined plugins sharing the same name as the local one get downloaded and clobbers the definition in _build (even if it gets copied over later). Locally defined plugins are just not upgradable, the same way top level dependencies are not. --- src/rebar_plugins.erl | 6 +++++- src/rebar_prv_plugins_upgrade.erl | 36 ++++++++++++++++++++----------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl index 7270f17a6..412b4c685 100644 --- a/src/rebar_plugins.erl +++ b/src/rebar_plugins.erl @@ -8,7 +8,8 @@ ,project_apps_install/1 ,install/2 ,handle_plugins/3 - ,handle_plugins/4]). + ,handle_plugins/4 + ,discover_plugins/1]). -include("rebar.hrl"). @@ -187,6 +188,9 @@ discover_plugins([], _) -> %% don't search if nothing is declared []; discover_plugins(_, State) -> + discover_plugins(State). + +discover_plugins(State) -> %% only support this mode in an umbrella project to avoid cases where %% this is used in a project intended to be an installed dependency and accidentally %% relies on vendoring when not intended. Also skip for global plugins, this would diff --git a/src/rebar_prv_plugins_upgrade.erl b/src/rebar_prv_plugins_upgrade.erl index d2e542698..e525e3822 100644 --- a/src/rebar_prv_plugins_upgrade.erl +++ b/src/rebar_prv_plugins_upgrade.erl @@ -69,27 +69,37 @@ upgrade(Plugin, State) -> Dep -> Dep end, - + LocalPlugins = [rebar_utils:to_atom(rebar_app_info:name(App)) + || App <- rebar_plugins:discover_plugins(State)], case Dep of not_found -> ?PRV_ERROR({not_found, Plugin}); {ok, P, Profile} -> - State1 = rebar_state:set(State, deps_dir, ?DEFAULT_PLUGINS_DIR), - maybe_update_pkg(P, State1), - {Apps, State2} = rebar_prv_install_deps:handle_deps_as_profile(Profile, State1, [P], true), + case lists:member(P, LocalPlugins) of + true -> + ?INFO("Plugin ~p is defined locally and does not need upgrading", [P]), + {ok, State}; + false -> + do_upgrade(State, P, Profile) + end + end. - {no_cycle, Sorted} = rebar_prv_install_deps:find_cycles(Apps), - ToBuild = rebar_prv_install_deps:cull_compile(Sorted, []), +do_upgrade(State, P, Profile) -> + State1 = rebar_state:set(State, deps_dir, ?DEFAULT_PLUGINS_DIR), + maybe_update_pkg(P, State1), + {Apps, State2} = rebar_prv_install_deps:handle_deps_as_profile(Profile, State1, [P], true), - %% Add already built plugin deps to the code path - CodePaths = [rebar_app_info:ebin_dir(A) || A <- Apps -- ToBuild], - code:add_pathsa(CodePaths), + {no_cycle, Sorted} = rebar_prv_install_deps:find_cycles(Apps), + ToBuild = rebar_prv_install_deps:cull_compile(Sorted, []), - %% Build plugin and its deps - _ = build_plugin(ToBuild, State2), + %% Add already built plugin deps to the code path + CodePaths = [rebar_app_info:ebin_dir(A) || A <- Apps -- ToBuild], + code:add_pathsa(CodePaths), - {ok, State} - end. + %% Build plugin and its deps + _ = build_plugin(ToBuild, State2), + + {ok, State}. find_plugin(Plugin, Profiles, State) -> ec_lists:search(fun(Profile) -> From 17abf0208fec32eefc21dcc2f948d02f90b7fbf0 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 10 Apr 2022 04:40:37 +0000 Subject: [PATCH 6/9] normalize debug output --- src/rebar_plugins.erl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl index 412b4c685..426018352 100644 --- a/src/rebar_plugins.erl +++ b/src/rebar_plugins.erl @@ -203,12 +203,14 @@ discover_plugins(State) -> %% top-level declarations BaseDir = rebar_state:dir(State), LibDirs = rebar_dir:project_plugin_dirs(State), - ?DIAGNOSTIC("Discovering local plugins in {project_plugin_dirs, ~p}", [LibDirs]), Dirs = [filename:join(BaseDir, LibDir) || LibDir <- LibDirs], RebarOpts = rebar_state:opts(State), SrcDirs = rebar_dir:src_dirs(RebarOpts, ["src"]), Found = rebar_app_discover:find_apps(Dirs, SrcDirs, all, State), - ?DIAGNOSTIC(" Found: ~p", [[rebar_app_info:name(F) || F <- Found]]), + ?DEBUG("Found local plugins: ~p~n" + "\tusing config: {project_plugin_dirs, ~p}", + [[rebar_utils:to_atom(rebar_app_info:name(F)) || F <- Found], + LibDirs]), PluginsDir = rebar_dir:plugins_dir(State), SetUp = lists:map(fun(App) -> Name = rebar_app_info:name(App), From 42f00ea6021cc954f36345e84ad283199b3256cc Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 10 Apr 2022 05:47:18 +0000 Subject: [PATCH 7/9] Rebuild modified local plugins Turns out that using local plugins as deps means we don't do recompilation checks automatically, so instead I reuse some of the compiler util functions to do a quick timestamp comparison in order to retrigger a build, which will then go through the usual DAG mechanism once deps are built. --- src/rebar_plugins.erl | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl index 426018352..52b049ed1 100644 --- a/src/rebar_plugins.erl +++ b/src/rebar_plugins.erl @@ -246,8 +246,10 @@ prepare_plugin(AppInfo) -> OutDir = rebar_app_info:out_dir(AppInfo), rebar_prv_compile:copy_app_dirs(AppInfo, AppDir, OutDir), Relocated = rebar_app_info:dir(AppInfo, OutDir), - %% Force a revalidation from the new paths - rebar_app_info:valid(Relocated, undefined). + case needs_rebuild(AppInfo) of + true -> rebar_app_info:valid(Relocated, false); % force recompilation + false -> rebar_app_info:valid(Relocated, undefined) % force revalidation + end. top_level_deps(Apps) -> RawDeps = lists:foldl(fun(App, Acc) -> @@ -259,3 +261,21 @@ top_level_deps(Apps) -> {deps, prod}]]) ++ Acc end, [], Apps), rebar_utils:tup_dedup(RawDeps). + +needs_rebuild(AppInfo) -> + %% if source files are newer than built files then the code was edited + %% and can't be considered valid -- force a rebuild. + %% + %% we do this by reusing the compiler code for Erlang as a heuristic for + %% files to check. The actual compiler provider will do an in-depth + %% validation of each module that may or may not need recompiling. + #{src_dirs := SrcD, include_dirs := InclD, + out_mappings := List} = rebar_compiler_erl:context(AppInfo), + SrcDirs = SrcD++InclD, + OutDirs = [Dir || {_Ext, Dir} <- List], + newest_stamp(OutDirs) < newest_stamp(SrcDirs). + +newest_stamp(DirList) -> + lists:max([0] ++ + [filelib:last_modified(F) + || F <- rebar_utils:find_files_in_dirs(DirList, ".+", true)]). From 7667b7b76fd100fd8ef77a7f5df295a5810190a9 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 10 Apr 2022 16:57:40 +0000 Subject: [PATCH 8/9] give deps for shelltests --- .github/workflows/shelltests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/shelltests.yml b/.github/workflows/shelltests.yml index fd9436f73..c09a58bae 100644 --- a/.github/workflows/shelltests.yml +++ b/.github/workflows/shelltests.yml @@ -29,7 +29,7 @@ jobs: - name: Install and run shelltestrunner run: | sudo apt-get update - sudo apt-get install -y shelltestrunner build-essential + sudo apt-get install -y shelltestrunner build-essential cmake liblz4-dev cd rebar3_tests mix local.hex --force ./run_tests.sh From 2af0af1090ab4ac25d62b53bb48e47b80696560a Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Wed, 27 Apr 2022 16:49:45 +0000 Subject: [PATCH 9/9] Going for more profiles than default and prod --- src/rebar_plugins.erl | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl index 52b049ed1..cd5f377cc 100644 --- a/src/rebar_plugins.erl +++ b/src/rebar_plugins.erl @@ -116,7 +116,7 @@ handle_plugin(Profile, Plugin, State, SrcPlugins, Upgrade) -> State0 = rebar_state:project_apps(State, SrcPlugins), %% We however have to pick the deps of top-level apps and promote them %% directly to make sure they are installed if they were not also at the top level - TopDeps = top_level_deps(SrcPlugins), + TopDeps = top_level_deps(State, SrcPlugins), %% Install the plugins {Apps, State1} = rebar_prv_install_deps:handle_deps_as_profile(Profile, State0, [Plugin|TopDeps], Upgrade), {no_cycle, Sorted} = rebar_prv_install_deps:find_cycles(SrcPlugins++Apps), @@ -251,14 +251,12 @@ prepare_plugin(AppInfo) -> false -> rebar_app_info:valid(Relocated, undefined) % force revalidation end. -top_level_deps(Apps) -> +top_level_deps(State, Apps) -> + CurrentProfiles = rebar_state:current_profiles(State), + Keys = lists:append([[{plugins, P}, {deps, P}] || P <- CurrentProfiles]), RawDeps = lists:foldl(fun(App, Acc) -> %% Only support the profiles we would with regular plugins? - lists:append([rebar_app_info:get(App, Key, []) - || Key <- [{plugins, default}, - {plugins, prod}, - {deps, default}, - {deps, prod}]]) ++ Acc + lists:append([rebar_app_info:get(App, Key, []) || Key <- Keys]) ++ Acc end, [], Apps), rebar_utils:tup_dedup(RawDeps).