From 553a579b36fe0fb4a8bf464cd282d43c07d4e192 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Tue, 5 Dec 2017 07:47:07 -0500 Subject: Alias plugin promoted to built-in command - Uses the code at https://github.com/tsloughter/rebar_alias and brings it within rebar3 - adds safety checks to prevent redefining built-in commands or obvious circular dependencies between commands (indirect circular deps are still possible) - adds tests - adds a systest to ensure no clash with the existing plugin --- src/rebar.app.src | 3 +- src/rebar_prv_alias.erl | 102 +++++++++++++++++++ src/rebar_state.erl | 2 +- systest/all_SUITE.erl | 24 ++++- systest/all_SUITE_data/alias_clash/rebar.config | 4 + .../alias_clash/src/alias_clash.app.src | 15 +++ .../all_SUITE_data/alias_clash/src/alias_clash.erl | 13 +++ test/rebar_alias_SUITE.erl | 113 +++++++++++++++++++++ 8 files changed, 271 insertions(+), 5 deletions(-) create mode 100644 src/rebar_prv_alias.erl create mode 100644 systest/all_SUITE_data/alias_clash/rebar.config create mode 100644 systest/all_SUITE_data/alias_clash/src/alias_clash.app.src create mode 100644 systest/all_SUITE_data/alias_clash/src/alias_clash.erl create mode 100644 test/rebar_alias_SUITE.erl diff --git a/src/rebar.app.src b/src/rebar.app.src index 74efe97..43ad620 100644 --- a/src/rebar.app.src +++ b/src/rebar.app.src @@ -73,6 +73,7 @@ rebar_prv_update, rebar_prv_upgrade, rebar_prv_version, - rebar_prv_xref]} + rebar_prv_xref, + rebar_prv_alias]} % must run last to prevent overloads ]} ]}. diff --git a/src/rebar_prv_alias.erl b/src/rebar_prv_alias.erl new file mode 100644 index 0000000..2a03668 --- /dev/null +++ b/src/rebar_prv_alias.erl @@ -0,0 +1,102 @@ +%%% @doc Meta-provider that dynamically compiles providers +%%% to run aliased commands. +%%% +%%% This is hackish and out-there, but this module has graduated +%%% from a plugin at https://github.com/tsloughter/rebar_alias after +%%% years of stability. Only some error checks were added +-module(rebar_prv_alias). + +-export([init/1]). +-include("rebar.hrl"). + +%% =================================================================== +%% Public API +%% =================================================================== +-spec init(rebar_state:t()) -> {ok, rebar_state:t()}. +init(State) -> + Aliases = rebar_state:get(State, alias, []), + lists:foldl(fun({Alias, Cmds}, {ok, StateAcc}) -> + case validate_provider(Alias, Cmds, State) of + true -> init_alias(Alias, Cmds, StateAcc); + false -> {ok, State} + end + end, {ok, State}, Aliases). + +init_alias(Alias, Cmds, State) -> + Module = list_to_atom("rebar_prv_alias_" ++ atom_to_list(Alias)), + + MF = module(Module), + EF = exports(), + FF = do_func(Cmds), + + {ok, _, Bin} = compile:forms([MF, EF, FF]), + code:load_binary(Module, "none", Bin), + + Provider = providers:create([ + {name, Alias}, + {module, Module}, + {bare, true}, + {deps, []}, + {example, example(Alias)}, + {opts, []}, + {short_desc, desc(Cmds)}, + {desc, desc(Cmds)} + ]), + {ok, rebar_state:add_provider(State, Provider)}. + +validate_provider(Alias, Cmds, State) -> + %% This would be caught and prevented anyway, but the warning + %% is friendlier + case providers:get_provider(Alias, rebar_state:providers(State)) of + not_found -> + %% check for circular deps in the alias. + case not proplists:is_defined(Alias, Cmds) of + true -> true; + false -> + ?WARN("Alias ~p contains itself and would never " + "terminate. It will be ignored.", + [Alias]), + false + end; + _ -> + ?WARN("Alias ~p is already the name of a command in " + "the default namespace and will be ignored.", + [Alias]), + false + end. + + +example(Alias) -> + "rebar3 " ++ atom_to_list(Alias). + +desc(Cmds) -> + "Equivalent to running: rebar3 do " ++ + rebar_string:join(lists:map(fun({Cmd, Args}) -> + atom_to_list(Cmd) ++ " " ++ Args; + (Cmd) -> + atom_to_list(Cmd) + end, Cmds), ","). + +module(Name) -> + {attribute,1,module,Name}. + +exports() -> + {attribute,1,export,[{do,1}]}. + +do_func(Cmds) -> + {function,1,do,1, + [{clause,1, + [{var,1,'State'}], + [], + [{call,1, + {remote,1,{atom,1,rebar_prv_do},{atom,1,do_tasks}}, + [to_args(Cmds),{var,1,'State'}]}]}]}. + + +to_args([]) -> + {nil,1}; +to_args([{Cmd, Args} | Rest]) -> + {cons,1,{tuple,1,[{string,1,atom_to_list(Cmd)},{string,1,Args}]}, to_args(Rest)}; +to_args([Cmd | Rest]) -> + {cons,1,{tuple,1,[{string,1,atom_to_list(Cmd)},{nil,1}]}, to_args(Rest)}. + diff --git a/src/rebar_state.erl b/src/rebar_state.erl index 3314a11..577ed23 100644 --- a/src/rebar_state.erl +++ b/src/rebar_state.erl @@ -391,7 +391,7 @@ add_provider(State=#state_t{providers=Providers, allow_provider_overrides=false} case {providers:impl(P), providers:namespace(P)} of {Name, Namespace} -> ?DEBUG("Not adding provider ~p ~p from module ~p because it already exists from module ~p", - [Namespace, Name, providers:module(P), Module]), + [Namespace, Name, Module, providers:module(P)]), true; _ -> false diff --git a/systest/all_SUITE.erl b/systest/all_SUITE.erl index 523e739..a0cfd3f 100644 --- a/systest/all_SUITE.erl +++ b/systest/all_SUITE.erl @@ -29,7 +29,7 @@ end_per_testcase(_Name, Config) -> Config. all() -> - [noop, resource_plugins]. + [noop, resource_plugins, alias_clash]. %groups() -> % [{plugins, [shuffle], []}, @@ -53,6 +53,21 @@ resource_plugins(Config) -> ct:pal("Rebar3 Output:~n~s",[Output]), ok. +alias_clash() -> + [{doc, "checking that the provider won't get plugin interference."}, + {timetrap, 10000}]. +alias_clash(Config) -> + {ok, Help} = rebar3("help", Config), % should be redefined, but by the plugin + ?assertNotEqual(nomatch, + re:run(Help, "Alias help is already the name of a command[a-z ]+and will be ignored") + ), + {ok, Output} = rebar3("test", Config, [{env, [{"DEBUG", "1"}]}]), + ?assertNotEqual(nomatch, re:run(Output, "cover summary written to:")), + ?assertNotEqual(nomatch, + re:run(Output, "Not adding provider default test from module rebar_prv_alias_test " + "because it already exists from module rebar_prv_alias_test")), + ok. + %%%%%%%%%%%%%%% %%% Helpers %%% %%%%%%%%%%%%%%% @@ -62,7 +77,9 @@ set_name_config(Atom, Config) -> atom_to_list(?MODULE)++"_data", atom_to_list(Atom)])} | Config]. -rebar3(Args, Config) -> +rebar3(Args, Config) -> rebar3(Args, Config, []). + +rebar3(Args, Config, UserOpts) -> Exec = case os:type() of {win32, _} -> "rebar3.cmd"; @@ -70,6 +87,7 @@ rebar3(Args, Config) -> "rebar3" end, Cmd = Exec ++ " " ++ Args, - Opts = [{cd, ?config(path, Config)}, return_on_error, use_stdout], + Opts = [{cd, ?config(path, Config)}, return_on_error, use_stdout + | UserOpts], ct:pal("Calling rebar3 ~s with options ~p", [Cmd, Opts]), rebar_utils:sh(Cmd, Opts). diff --git a/systest/all_SUITE_data/alias_clash/rebar.config b/systest/all_SUITE_data/alias_clash/rebar.config new file mode 100644 index 0000000..baf20a9 --- /dev/null +++ b/systest/all_SUITE_data/alias_clash/rebar.config @@ -0,0 +1,4 @@ +{alias, [{help, [version]}, % should be skipped, but be overriden by plugin + {test, [compile, {eunit, "-c"}, cover]}]}. + +{plugins, [rebar_alias]}. % should be overridden diff --git a/systest/all_SUITE_data/alias_clash/src/alias_clash.app.src b/systest/all_SUITE_data/alias_clash/src/alias_clash.app.src new file mode 100644 index 0000000..b4cdda2 --- /dev/null +++ b/systest/all_SUITE_data/alias_clash/src/alias_clash.app.src @@ -0,0 +1,15 @@ +{application, alias_clash, + [{description, "An OTP library"}, + {vsn, "0.1.0"}, + {registered, []}, + {applications, + [kernel, + stdlib + ]}, + {env,[]}, + {modules, []}, + + {maintainers, []}, + {licenses, ["Apache 2.0"]}, + {links, []} + ]}. diff --git a/systest/all_SUITE_data/alias_clash/src/alias_clash.erl b/systest/all_SUITE_data/alias_clash/src/alias_clash.erl new file mode 100644 index 0000000..9249cdb --- /dev/null +++ b/systest/all_SUITE_data/alias_clash/src/alias_clash.erl @@ -0,0 +1,13 @@ +-module(alias_clash). + +%% API exports +-export([]). + +%%==================================================================== +%% API functions +%%==================================================================== + + +%%==================================================================== +%% Internal functions +%%==================================================================== diff --git a/test/rebar_alias_SUITE.erl b/test/rebar_alias_SUITE.erl new file mode 100644 index 0000000..f889ae4 --- /dev/null +++ b/test/rebar_alias_SUITE.erl @@ -0,0 +1,113 @@ +-module(rebar_alias_SUITE). +-compile([export_all]). + +-include_lib("common_test/include/ct.hrl"). +-include_lib("eunit/include/eunit.hrl"). + +init_per_suite(Config) -> Config. +end_per_suite(_Config) -> ok. + +init_per_testcase(_, Config) -> + rebar_test_utils:init_rebar_state(Config, "alias_"). + +end_per_testcase(_, _Config) -> + ok. + +all() -> [command, args, many, override_default, no_circular]. + %% namespaces: unsupported, untested. + +command() -> + [{doc, "Runs multiple regular commands as one alias"}]. +command(Config) -> + AppDir = ?config(apps, Config), + + Name = rebar_test_utils:create_random_name("alias_command_"), + Vsn = rebar_test_utils:create_random_vsn(), + rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]), + RebarConfig = [{alias, [{test, [compile, unlock]}]}], + + %% compile job ran + rebar_test_utils:run_and_check(Config, RebarConfig, + ["test"], {ok, [{app, Name}]}), + %% unlock job also ran + Lockfile = filename:join(?config(apps, Config), "rebar.lock"), + ?assertNot(filelib:is_file(Lockfile)), + ok. + +args() -> + [{doc, "Runs multiple regular commands as one alias, some of " + "which have default arguments"}]. +args(Config) -> + AppDir = ?config(apps, Config), + + Name = rebar_test_utils:create_random_name("alias_args_"), + Vsn = rebar_test_utils:create_random_vsn(), + rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]), + RebarConfig = [{alias, [{test, [{eunit,"-c"}, cover]}]}], + + %% test job ran (compiled and succeeded) + rebar_test_utils:run_and_check(Config, RebarConfig, + ["test"], {ok, [{app, Name}]}), + %% cover job also ran, meaning eunit had coverage on, otherwise + %% the index file is not generated. + CoverFile = filename:join([?config(apps, Config), + "_build", "test", "cover", "index.html"]), + ?assert(filelib:is_file(CoverFile)), + ok. + +many() -> + [{doc, "Multiple aliases may be registered"}]. +many(Config) -> + AppDir = ?config(apps, Config), + + Name = rebar_test_utils:create_random_name("alias_args_"), + Vsn = rebar_test_utils:create_random_vsn(), + rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]), + RebarConfig = [{alias, [{test, [{eunit,"-c"}, cover]}, + {nolock, [compile, unlock]}]}], + + %% test job ran (compiled and succeeded) + rebar_test_utils:run_and_check(Config, RebarConfig, + ["test"], {ok, [{app, Name}]}), + rebar_test_utils:run_and_check(Config, RebarConfig, + ["nolock"], {ok, [{app, Name}]}), + %% both jobs ran (see args/1 and command/1) + CoverFile = filename:join([?config(apps, Config), + "_build", "test", "cover", "index.html"]), + ?assert(filelib:is_file(CoverFile)), + Lockfile = filename:join(?config(apps, Config), "rebar.lock"), + ?assertNot(filelib:is_file(Lockfile)), + ok. + +override_default() -> + [{doc, "An alias cannot take over a default provider"}]. +override_default(Config) -> + AppDir = ?config(apps, Config), + + Name = rebar_test_utils:create_random_name("alias_override_"), + Vsn = rebar_test_utils:create_random_vsn(), + rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]), + RebarConfig = [{alias, [{compile, [help]}]}], + + %% App compiles anyway + rebar_test_utils:run_and_check(Config, RebarConfig, ["compile"], + {ok, [{app, Name}]}), + ok. + +no_circular() -> + [{doc, "An alias cannot define itself as itself"}, + {timetrap, 2000}]. +no_circular(Config) -> + AppDir = ?config(apps, Config), + + Name = rebar_test_utils:create_random_name("alias_circular_"), + Vsn = rebar_test_utils:create_random_vsn(), + rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]), + RebarConfig = [{alias, [{test, [help, {test,"-a"}, compile]}]}], + + %% Code does not deadlock forever and errors by not knowing + %% the command + rebar_test_utils:run_and_check(Config, RebarConfig, ["test"], + {error, [$C,$o,$m,$m,$a,$n,$d,$ ,"test",$ ,$n,$o,$t,$ , + $f,$o,$u,$n,$d]}), + ok. -- cgit v1.1