Fix Plugin path handling (again!)

The path reloading of plugins had been fixed properly, but the problem
is that the paths it was using to re-load only considered the current
compile step, rather than the overall state of plugins. As such, the
reloaded paths after plugin compilation only reloaded the *latest*
plugin and not the other ones.

This fix forces the addition of all built plugin paths to the code paths
after a plugin compile job is run. This ensures that the path is clean
for initial plugin deps (only add those that are required), and is
re-made total after the fact (add all the plugins possible).

This commit also includes a system tests suite that can be run
optionally; the problem with this plugin mechanism was impossible to
find through mocked dependencies, and a working counterexample was
provided to us. The systest suite can be run against real projects
without conflict to make sure no regressions are hit.

4 files changed, 107 insertions, 3 deletions

diff --git a/rebar.config b/rebar.config
index 680e6f2..dda579d 100644
--- a/rebar.config
+++ b/rebar.config
@@ -43,6 +43,10 @@
                    {erl_opts, [debug_info, nowarn_export_all]}
                    ]
             },
+            {systest, [
+                {erl_opts, [debug_info, nowarn_export_all]},
+                {ct_opts, [{dir, "systest"}]}
+            ]},
 
             {bootstrap, []},
 
diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl
index 1fc01ff..57c34bc 100644
--- a/src/rebar_plugins.erl
+++ b/src/rebar_plugins.erl
@@ -96,8 +96,8 @@ handle_plugin(Profile, Plugin, State, Upgrade) ->
         ToBuild = rebar_prv_install_deps:cull_compile(Sorted, []),
 
         %% Add already built plugin deps to the code path
-        CodePaths = [rebar_app_info:ebin_dir(A) || A <- Apps -- ToBuild],
-        code:add_pathsa(CodePaths),
+        PreBuiltPaths = [rebar_app_info:ebin_dir(A) || A <- Apps] -- ToBuild,
+        code:add_pathsa(PreBuiltPaths),
 
         %% Build plugin and its deps
         [build_plugin(AppInfo, Apps, State2) || AppInfo <- ToBuild],
@@ -105,10 +105,12 @@ handle_plugin(Profile, Plugin, State, Upgrade) ->
         %% Add newly built deps and plugin to code path
         State3 = rebar_state:update_all_plugin_deps(State2, Apps),
         NewCodePaths = [rebar_app_info:ebin_dir(A) || A <- ToBuild],
+        AllPluginEbins = filelib:wildcard(filename:join([rebar_dir:plugins_dir(State), "*", "ebin"])),
+        CodePaths = PreBuiltPaths++(AllPluginEbins--ToBuild),
         code:add_pathsa(NewCodePaths++CodePaths),
 
         %% Store plugin code paths so we can remove them when compiling project apps
-        State4 = rebar_state:update_code_paths(State3, all_plugin_deps, CodePaths++NewCodePaths),
+        State4 = rebar_state:update_code_paths(State3, all_plugin_deps, PreBuiltPaths++NewCodePaths),
 
         {plugin_providers(Plugin), State4}
     catch
diff --git a/systest/all_SUITE.erl b/systest/all_SUITE.erl
new file mode 100644
index 0000000..523e739
--- /dev/null
+++ b/systest/all_SUITE.erl
@@ -0,0 +1,75 @@
+-module(all_SUITE).
+-include_lib("common_test/include/ct.hrl").
+-include_lib("eunit/include/eunit.hrl").
+-compile([export_all, nowarn_export_all]).
+
+init_per_suite(Config) ->
+    %% TODO: figure out how to use a local rebar3 copy?
+    %% Ensure global rebar3 has the same version as current one!
+    {ok, Vsn} = application:get_key(rebar, vsn),
+    {ok, ExecVsn} = rebar3("version", [{path, "."} | Config]),
+    case rebar_string:lexemes(ExecVsn, " ") of
+        ["rebar", Vsn | _] ->
+            %% Copy all base cases to priv_dir
+            rebar_file_utils:cp_r([?config(data_dir, Config)],
+                                  ?config(priv_dir, Config)),
+            Config;
+        _ ->
+            {skip, "expected current version "++Vsn++" in path "
+                   "and found '"++ ExecVsn ++ "'"}
+    end.
+
+end_per_suite(Config) ->
+    Config.
+
+init_per_testcase(Name, Config) ->
+    set_name_config(Name, Config).
+
+end_per_testcase(_Name, Config) ->
+    Config.
+
+all() ->
+    [noop, resource_plugins].
+
+%groups() ->
+%    [{plugins, [shuffle], []},
+%     {deps, [shuffle], []}].
+
+%%%%%%%%%%%%%%%%%%
+%%% TEST CASES %%%
+%%%%%%%%%%%%%%%%%%
+
+noop() ->
+    [{doc, "just a sanity check on the handling of the test suite init/end"}].
+noop(_Config) ->
+    true.
+
+resource_plugins() ->
+    [{doc, "Issue #1673: "
+           "Ensure that deps using resource plugins with semver compile."}].
+resource_plugins(Config) ->
+    %% When the environment handling is wrong, the run fails violently.
+    {ok, Output} = rebar3("compile", Config),
+    ct:pal("Rebar3 Output:~n~s",[Output]),
+    ok.
+
+%%%%%%%%%%%%%%%
+%%% Helpers %%%
+%%%%%%%%%%%%%%%
+set_name_config(Atom, Config) ->
+    [{path,
+      filename:join([?config(priv_dir, Config),
+                     atom_to_list(?MODULE)++"_data", atom_to_list(Atom)])}
+     | Config].
+
+rebar3(Args, Config) ->
+    Exec = case os:type() of
+        {win32, _} ->
+            "rebar3.cmd";
+        _ ->
+            "rebar3"
+    end,
+    Cmd = Exec ++ " " ++ Args,
+    Opts = [{cd, ?config(path, Config)}, return_on_error, use_stdout],
+    ct:pal("Calling rebar3 ~s with options ~p", [Cmd, Opts]),
+    rebar_utils:sh(Cmd, Opts).
diff --git a/systest/all_SUITE_data/resource_plugins/rebar.config b/systest/all_SUITE_data/resource_plugins/rebar.config
new file mode 100644
index 0000000..7baec1f
--- /dev/null
+++ b/systest/all_SUITE_data/resource_plugins/rebar.config
@@ -0,0 +1,23 @@
+%% Sample provided by @tothlac
+{plugins, [
+    {rebar_tidy_deps, ".*", {git, "https://github.com/ferd/rebar3-tidy-deps-plugin.git"}},
+    {rebar_alias, {git, "https://github.com/tsloughter/rebar_alias.git"}},
+    rebar3_appup_plugin
+]}.
+
+{deps, [
+    {hackney, {git, "https://github.com/benoitc/hackney.git", {tag, "1.10.1"}}}
+ ]}.
+
+
+%% Make work despite compat issues with strings and warnings
+{overrides, [
+  {override, rebar3_appup_plugin, [
+    {erl_opts, [
+        {platform_define, "^19", brutal_purge_fixed},
+        {platform_define, "^2", brutal_purge_fixed},
+        %% warnings_as_errors,
+        debug_info
+    ]}
+  ]}
+]}.