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.

14 files changed, 448 insertions, 48 deletions

diff --git a/src/rebar_api.erl b/src/rebar_api.erl
index 4dabe8a..00eb054 100644
--- a/src/rebar_api.erl
+++ b/src/rebar_api.erl
@@ -9,6 +9,8 @@
          expand_env_variable/3,
          get_arch/0,
          wordsize/0,
+         set_paths/2,
+         unset_paths/2,
          add_deps_to_path/1,
          restore_code_path/1,
          processing_base_dir/1,
@@ -67,6 +69,21 @@ get_arch() ->
 wordsize() ->
     rebar_utils:wordsize().
 
+%% @doc Set code paths. Takes arguments of the form
+%% `[plugins, deps]' or `[deps, plugins]' and ensures the
+%% project's app and dependencies are set in the right order
+%% for the next bit of execution
+-spec set_paths(rebar_paths:targets(), rebar_state:t()) -> ok.
+set_paths(List, State) ->
+    rebar_paths:set_paths(List, State).
+
+%% @doc Unsets code paths. Takes arguments of the form
+%% `[plugins, deps]' or `[deps, plugins]' and ensures the
+%% paths are no longer active.
+-spec unset_paths(rebar_paths:targets(), rebar_state:t()) -> ok.
+unset_paths(List, State) ->
+    rebar_paths:unset_paths(List, State).
+
 %% @doc Add deps to the code path
 -spec add_deps_to_path(rebar_state:t()) -> ok.
 add_deps_to_path(State) ->
diff --git a/src/rebar_hooks.erl b/src/rebar_hooks.erl
index ec6fe31..f2ef8a3 100644
--- a/src/rebar_hooks.erl
+++ b/src/rebar_hooks.erl
@@ -42,8 +42,9 @@ run_provider_hooks_(Dir, Type, Command, Providers, TypeHooks, State) ->
         [] ->
             State;
         HookProviders ->
-            PluginDepsPaths = lists:usort(rebar_state:code_paths(State, all_plugin_deps)),
-            code:add_pathsa(PluginDepsPaths),
+            %PluginDepsPaths = lists:usort(rebar_state:code_paths(State, all_plugin_deps)),
+            %code:add_pathsa(PluginDepsPaths),
+            rebar_paths:set_paths([plugins, deps], State),
             Providers1 = rebar_state:providers(State),
             State1 = rebar_state:providers(rebar_state:dir(State, Dir), Providers++Providers1),
             case rebar_core:do(HookProviders, State1) of
@@ -51,7 +52,8 @@ run_provider_hooks_(Dir, Type, Command, Providers, TypeHooks, State) ->
                     ?DEBUG(format_error({bad_provider, Type, Command, ProviderName}), []),
                     throw(?PRV_ERROR({bad_provider, Type, Command, ProviderName}));
                 {ok, State2} ->
-                    rebar_utils:remove_from_code_path(PluginDepsPaths),
+                    %rebar_utils:remove_from_code_path(PluginDepsPaths),
+                    rebar_paths:set_paths([deps, plugins], State2),
                     State2
             end
     end.
diff --git a/src/rebar_packages.erl b/src/rebar_packages.erl
index 7676213..096948c 100644
--- a/src/rebar_packages.erl
+++ b/src/rebar_packages.erl
@@ -82,7 +82,7 @@ get_package(Dep, Vsn, Hash, Repos, Table, State) ->
             not_found
     end.
 
-new_package_table() ->    
+new_package_table() ->
     ?PACKAGE_TABLE = ets:new(?PACKAGE_TABLE, [named_table, public, ordered_set, {keypos, 2}]),
     ets:insert(?PACKAGE_TABLE, {?PACKAGE_INDEX_VERSION, package_index_version}).
 
diff --git a/src/rebar_paths.erl b/src/rebar_paths.erl
new file mode 100644
index 0000000..bb43897
--- /dev/null
+++ b/src/rebar_paths.erl
@@ -0,0 +1,177 @@
+-module(rebar_paths).
+-include("rebar.hrl").
+
+-type target() :: deps | plugins.
+-type targets() :: [target(), ...].
+-export_type([target/0, targets/0]).
+-export([set_paths/2, unset_paths/2]).
+
+-spec set_paths(targets(), rebar_state:t()) -> ok.
+set_paths(UserTargets, State) ->
+    Targets = normalize_targets(UserTargets),
+    GroupPaths = path_groups(Targets, State),
+    Paths = lists:append([P || {_, P} <- GroupPaths]),
+    [code:del_path(P) || P <- Paths],
+    code:add_pathsa(lists:reverse(Paths)),
+    % set path breaks with escripts
+    %true = code:set_path(lists:append([P || {_, P} <- GroupPaths])),
+    AppGroups = app_groups(Targets, State),
+    purge_and_load(AppGroups, code:all_loaded(), sets:new()),
+    ok.
+
+-spec unset_paths(targets(), rebar_state:t()) -> ok.
+unset_paths(UserTargets, State) ->
+    Targets = normalize_targets(UserTargets),
+    GroupPaths = path_groups(Targets, State),
+    Paths = lists:append([P || {_, P} <- GroupPaths]),
+    [code:del_path(P) || P <- Paths],
+    purge(Paths, code:all_loaded()),
+    ok.
+
+
+%% The paths are to be set in the reverse order; i.e. the default
+%% path is always last when possible (minimize cases where a build
+%% tool version clashes with an app's), and put the highest priorities
+%% first.
+-spec normalize_targets(targets()) -> targets().
+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
+    TmpList = lists:foldl(
+      fun(deps, [deps | _] = Acc) -> Acc;
+         (plugins, [plugins | _] = Acc) -> Acc;
+         (deps, Acc) -> [deps | Acc -- [deps]];
+         (plugins, Acc) -> [plugins | Acc -- [plugins]];
+         (_, Acc) -> Acc
+      end,
+      [],
+      List
+    ),
+    lists:reverse(TmpList).
+
+purge_and_load([], _, _) ->
+    ok;
+purge_and_load([{_Group, Apps}|Rest], ModPaths, Seen) ->
+    %% We have: a list of all applications in the current priority group,
+    %% a list of all loaded modules with their active path, and a list of
+    %% seen applications.
+    %%
+    %% We do the following:
+    %% 1. identify the apps that have not been solved yet
+    %% 2. find the paths for all apps in the current group
+    %% 3. unload and reload apps that may have changed paths in order
+    %%    to get updated module lists and specs
+    %%    (we ignore started apps and apps that have not run for this)
+    %% 4. create a list of modules to check from that app list
+    %% 5. check the modules to match their currently loaded paths with
+    %%    the path set from the apps in the current group; modules
+    %%    that differ must be purged; others can stay
+
+    %% 1)
+    AppNames = [AppName || App <- Apps,
+                           AppName <- [rebar_app_info:name(App)],
+                           not sets:is_element(AppName, Seen)],
+    GoodApps = [App || AppName <- AppNames,
+                       App <- Apps,
+                       rebar_app_info:name(App) =:= AppName],
+    %% 2)
+    %% TODO: add extra dirs (and test), and possibly the stdlib
+    GoodAppPaths = [rebar_app_info:ebin_dir(App) || App <- GoodApps],
+                %% ++ [code:lib_dir()],
+    %% 3)
+    [begin
+         AtomApp = binary_to_atom(AppName, utf8),
+         %% blind load/unload won't interrupt an already-running app,
+         %% preventing odd errors, maybe!
+         case application:unload(AtomApp) of
+             ok -> application:load(AtomApp);
+             _ -> ok
+         end
+     end || AppName <- AppNames,
+            %% Shouldn't unload ourselves; rebar runs without ever
+            %% being started and unloading breaks logging!
+            AppName =/= <<"rebar">>],
+
+    %% 4)
+    CandidateMods = lists:append(
+        %% Start by asking the currently loaded app (if loaded)
+        %% since it would be the primary source of conflicting modules
+        [case application:get_key(AppName, modules) of
+             {ok, Mods} ->
+                 Mods;
+             undefined ->
+                 %% if not found, parse the app file on disk, in case
+                 %% the app's modules are used without it being loaded
+                 case rebar_app_info:app_details(App) of
+                     [] -> [];
+                     Details -> proplists:get_value(modules, Details, [])
+                 end
+         end || App <- GoodApps,
+                AppName <- [binary_to_atom(rebar_app_info:name(App), utf8)]]
+    ),
+
+    %% 5)
+    Mods = misloaded_modules(CandidateMods, GoodAppPaths, ModPaths),
+    [purge_mod(Mod) || Mod <- Mods],
+    purge_and_load(Rest, ModPaths,
+                   sets:union(Seen, sets:from_list(AppNames))).
+
+
+purge(Paths, ModPaths) ->
+    lists:map(fun purge_mod/1, lists:usort(
+        [Mod || {Mod, Path} <- ModPaths,
+                is_list(Path), % not 'preloaded' or mocked
+                any_prefix(Path, Paths)]
+    )).
+
+misloaded_modules(Mods, GoodAppPaths, ModPaths) ->
+    %% Identify paths that are invalid; i.e. app paths that cover an
+    %% app in the desired group, but are not in the desired group.
+    lists:usort(
+        [purge_mod(Mod)
+         || Mod <- Mods,
+            {_, Path} <- [lists:keyfind(Mod, 1, ModPaths)],
+            is_list(Path), % not 'preloaded' or mocked
+            not any_prefix(Path, GoodAppPaths)]
+    ).
+
+any_prefix(Path, Paths) ->
+    lists:any(fun(P) -> lists:prefix(P, Path) end, Paths).
+
+%% assume paths currently set are good; only unload a module so next call
+%% uses the correctly set paths
+purge_mod(Mod) ->
+    case erlang:check_process_code(self(), Mod) of
+        false ->
+            code:purge(Mod),
+            code:delete(Mod);
+        _ ->
+            %% cannot purge safely without killing ourselves
+            code:soft_purge(Mod) andalso
+            code:delete(Mod)
+    end.
+
+path_groups(Targets, State) ->
+    [{Target, get_paths(Target, State)} || Target <- Targets].
+
+app_groups(Targets, State) ->
+    [{Target, get_apps(Target, State)} || Target <- Targets].
+
+get_paths(deps, State) ->
+    rebar_state:code_paths(State, all_deps);
+get_paths(plugins, State) ->
+    rebar_state:code_paths(State, all_plugin_deps).
+
+get_apps(deps, State) ->
+    %% The code paths for deps also include the top level apps
+    %% and the extras, which we don't have here; we have to
+    %% add the apps by hand
+    rebar_state:all_deps(State) ++
+    case rebar_state:project_apps(State) of
+        undefined -> [];
+        List -> List
+    end;
+get_apps(plugins, State) ->
+    rebar_state:all_plugin_deps(State).
diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl
index d58675d..a9c7e00 100644
--- a/src/rebar_plugins.erl
+++ b/src/rebar_plugins.erl
@@ -122,12 +122,10 @@ 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, PreBuiltPaths++NewCodePaths),
+        rebar_paths:set_paths([plugins, deps], State4),
 
         {plugin_providers(Plugin), State4}
     catch
diff --git a/src/rebar_prv_common_test.erl b/src/rebar_prv_common_test.erl
index 9e71ee7..3d3bd8a 100644
--- a/src/rebar_prv_common_test.erl
+++ b/src/rebar_prv_common_test.erl
@@ -58,7 +58,7 @@ do(State) ->
 
 do(State, Tests) ->
     ?INFO("Running Common Test suites...", []),
-    rebar_utils:update_code(rebar_state:code_paths(State, all_deps), [soft_purge]),
+    rebar_paths:set_paths([deps, plugins], State),
 
     %% Run ct provider prehooks
     Providers = rebar_state:providers(State),
@@ -73,14 +73,14 @@ do(State, Tests) ->
                 ok    ->
                     %% Run ct provider post hooks for all project apps and top level project hooks
                     rebar_hooks:run_project_and_app_hooks(Cwd, post, ?PROVIDER, Providers, State),
-                    rebar_utils:cleanup_code_path(rebar_state:code_paths(State, default)),
+                    rebar_paths:set_paths([plugins, deps], State),
                     {ok, State};
                 Error ->
-                    rebar_utils:cleanup_code_path(rebar_state:code_paths(State, default)),
+                    rebar_paths:set_paths([plugins, deps], State),
                     Error
             end;
         Error ->
-            rebar_utils:cleanup_code_path(rebar_state:code_paths(State, default)),
+            rebar_paths:set_paths([plugins, deps], State),
             Error
     end.
 
@@ -250,11 +250,9 @@ select_tests(State, ProjectApps, CmdOpts, CfgOpts) ->
                             end, SysConfigs),
     %% NB: load the applications (from user directories too) to support OTP < 17
     %% to our best ability.
-    OldPath = code:get_path(),
-    code:add_pathsa(rebar_state:code_paths(State, all_deps)),
+    rebar_paths:set_paths([deps, plugins], State),
     [application:load(Application) || Config <- Configs, {Application, _} <- Config],
     rebar_utils:reread_config(Configs),
-    code:set_path(OldPath),
 
     Opts = merge_opts(CmdOpts,CfgOpts),
     discover_tests(State, ProjectApps, Opts).
diff --git a/src/rebar_prv_compile.erl b/src/rebar_prv_compile.erl
index a509704..405f73a 100644
--- a/src/rebar_prv_compile.erl
+++ b/src/rebar_prv_compile.erl
@@ -37,10 +37,7 @@ init(State) ->
 -spec do(rebar_state:t()) -> {ok, rebar_state:t()} | {error, string()}.
 do(State) ->
     IsDepsOnly = is_deps_only(State),
-    DepsPaths = rebar_state:code_paths(State, all_deps),
-    PluginDepsPaths = rebar_state:code_paths(State, all_plugin_deps),
-    rebar_utils:remove_from_code_path(PluginDepsPaths),
-    code:add_pathsa(DepsPaths),
+    rebar_paths:set_paths([deps, plugins], State),
 
     Providers = rebar_state:providers(State),
     Deps = rebar_state:deps_to_build(State),
@@ -50,11 +47,10 @@ do(State) ->
                  true ->
                      State;
                  false ->
-                     handle_project_apps(DepsPaths, Providers, State)
+                     handle_project_apps(Providers, State)
              end,
 
-    rebar_utils:cleanup_code_path(rebar_state:code_paths(State1, default)
-                                  ++ rebar_state:code_paths(State, all_plugin_deps)),
+    rebar_paths:set_paths([plugins, deps], State1),
 
     {ok, State1}.
 
@@ -62,7 +58,7 @@ is_deps_only(State) ->
     {Args, _} = rebar_state:command_parsed_args(State),
     proplists:get_value(deps_only, Args, false).
 
-handle_project_apps(DepsPaths, Providers, State) ->
+handle_project_apps(Providers, State) ->
     Cwd = rebar_state:dir(State),
     ProjectApps = rebar_state:project_apps(State),
     {ok, ProjectApps1} = rebar_digraph:compile_order(ProjectApps),
@@ -76,7 +72,7 @@ handle_project_apps(DepsPaths, Providers, State) ->
     %% projects with structures like /apps/foo,/apps/bar,/test
     build_extra_dirs(State, ProjectApps2),
 
-    State3 = update_code_paths(State2, ProjectApps2, DepsPaths),
+    State3 = update_code_paths(State2, ProjectApps2),
 
     rebar_hooks:run_all_hooks(Cwd, post, ?PROVIDER, Providers, State2),
     case rebar_state:has_all_artifacts(State3) of
@@ -176,11 +172,10 @@ compile(State, Providers, AppInfo) ->
     %% The rebar_otp_app compilation step is safe regarding the
     %% overall path management, so we can just load all plugins back
     %% in memory.
-    PluginDepsPaths = rebar_state:code_paths(State, all_plugin_deps),
-    code:add_pathsa(PluginDepsPaths),
+    rebar_paths:set_paths([plugins, deps], State),
     AppFileCompileResult = rebar_otp_app:compile(State, AppInfo4),
     %% Clean up after ourselves, leave things as they were.
-    rebar_utils:remove_from_code_path(PluginDepsPaths),
+    rebar_paths:set_paths([deps, plugins], State),
 
     case AppFileCompileResult of
         {ok, AppInfo5} ->
@@ -206,21 +201,22 @@ build_app(AppInfo, State) ->
             case lists:keyfind(Type, 1, ProjectBuilders) of
                 {_, Module} ->
                     %% load plugins since thats where project builders would be
-                    PluginDepsPaths = rebar_state:code_paths(State, all_plugin_deps),
-                    code:add_pathsa(PluginDepsPaths),
-                    case Module:build(AppInfo) of
-                        ok ->
-                            rebar_utils:remove_from_code_path(PluginDepsPaths);
-                        {error, Reason} ->
-                            throw({error, {Module, Reason}})
+                    rebar_paths:set_paths([plugins, deps], State),
+                    Res = Module:build(AppInfo),
+                    rebar_paths:set_paths([deps, plugins], State),
+                    case Res of
+                        ok -> ok;
+                        {error, Reason} -> throw({error, {Module, Reason}})
                     end;
                 _ ->
                     throw(?PRV_ERROR({unknown_project_type, rebar_app_info:name(AppInfo), Type}))
             end
     end.
-update_code_paths(State, ProjectApps, DepsPaths) ->
+
+update_code_paths(State, ProjectApps) ->
     ProjAppsPaths = paths_for_apps(ProjectApps),
     ExtrasPaths = paths_for_extras(State, ProjectApps),
+    DepsPaths = rebar_state:code_paths(State, all_deps),
     rebar_state:code_paths(State, all_deps, DepsPaths ++ ProjAppsPaths ++ ExtrasPaths).
 
 paths_for_apps(Apps) -> paths_for_apps(Apps, []).
diff --git a/src/rebar_prv_dialyzer.erl b/src/rebar_prv_dialyzer.erl
index 99a7698..585051c 100644
--- a/src/rebar_prv_dialyzer.erl
+++ b/src/rebar_prv_dialyzer.erl
@@ -85,7 +85,8 @@ short_desc() ->
 do(State) ->
     maybe_fix_env(),
     ?INFO("Dialyzer starting, this may take a while...", []),
-    code:add_pathsa(rebar_state:code_paths(State, all_deps)),
+    rebar_paths:unset_paths([plugins], State), % no plugins in analysis
+    rebar_paths:set_paths([deps], State),
     Plt = get_plt(State),
 
     try
@@ -104,7 +105,7 @@ do(State) ->
         throw:{output_file_error, _, _} = Error ->
             ?PRV_ERROR(Error)
     after
-        rebar_utils:cleanup_code_path(rebar_state:code_paths(State, default))
+        rebar_paths:set_paths([plugins,deps], State)
     end.
 
 %% This is used to workaround dialyzer quirk discussed here
diff --git a/src/rebar_prv_edoc.erl b/src/rebar_prv_edoc.erl
index 9517335..c78296a 100644
--- a/src/rebar_prv_edoc.erl
+++ b/src/rebar_prv_edoc.erl
@@ -32,7 +32,7 @@ init(State) ->
 -spec do(rebar_state:t()) ->
     {ok, rebar_state:t()} | {error, string()} | {error, {module(), any()}}.
 do(State) ->
-    code:add_pathsa(rebar_state:code_paths(State, all_deps)),
+    rebar_paths:set_paths([deps, plugins], State),
     ProjectApps = rebar_state:project_apps(State),
     Providers = rebar_state:providers(State),
     EdocOpts = rebar_state:get(State, edoc_opts, []),
@@ -64,7 +64,7 @@ do(State) ->
             {app_failed, AppName}
     end,
     rebar_hooks:run_all_hooks(Cwd, post, ?PROVIDER, Providers, State),
-    rebar_utils:cleanup_code_path(rebar_state:code_paths(State, default)),
+    rebar_paths:set_paths([plugins, deps], State),
     case Res of
         {app_failed, App} ->
             ?PRV_ERROR({app_failed, App});
diff --git a/src/rebar_prv_eunit.erl b/src/rebar_prv_eunit.erl
index 4b71416..f120926 100644
--- a/src/rebar_prv_eunit.erl
+++ b/src/rebar_prv_eunit.erl
@@ -54,7 +54,7 @@ do(State, Tests) ->
     ?INFO("Performing EUnit tests...", []),
 
     setup_name(State),
-    rebar_utils:update_code(rebar_state:code_paths(State, all_deps), [soft_purge]),
+    rebar_paths:set_paths([deps, plugins], State),
 
     %% Run eunit provider prehooks
     Providers = rebar_state:providers(State),
@@ -67,14 +67,14 @@ do(State, Tests) ->
                 {ok, State1} ->
                     %% Run eunit provider posthooks
                     rebar_hooks:run_project_and_app_hooks(Cwd, post, ?PROVIDER, Providers, State1),
-                    rebar_utils:cleanup_code_path(rebar_state:code_paths(State, default)),
+                    rebar_paths:set_paths([plugins, deps], State),
                     {ok, State1};
                 Error ->
-                    rebar_utils:cleanup_code_path(rebar_state:code_paths(State, default)),
+                    rebar_paths:set_paths([plugins, deps], State),
                     Error
             end;
         Error ->
-            rebar_utils:cleanup_code_path(rebar_state:code_paths(State, default)),
+            rebar_paths:set_paths([plugins, deps], State),
             Error
     end.
 
diff --git a/src/rebar_prv_xref.erl b/src/rebar_prv_xref.erl
index 2405ebb..a6b1f73 100644
--- a/src/rebar_prv_xref.erl
+++ b/src/rebar_prv_xref.erl
@@ -36,8 +36,7 @@ init(State) ->
 
 -spec do(rebar_state:t()) -> {ok, rebar_state:t()} | {error, string()}.
 do(State) ->
-    OldPath = code:get_path(),
-    code:add_pathsa(rebar_state:code_paths(State, all_deps)),
+    rebar_paths:set_paths([deps, plugins], State),
     XrefChecks = prepare(State),
     XrefIgnores = rebar_state:get(State, xref_ignores, []),
     %% Run xref checks
@@ -48,7 +47,7 @@ do(State) ->
     QueryChecks = rebar_state:get(State, xref_queries, []),
     QueryResults = lists:foldl(fun check_query/2, [], QueryChecks),
     stopped = xref:stop(xref),
-    rebar_utils:cleanup_code_path(OldPath),
+    rebar_paths:set_paths([plugins, deps], State),
     case XrefResults =:= [] andalso QueryResults =:= [] of
         true ->
             {ok, State};
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, <<Name/binary, ".app">>]), 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, <<Name/binary, ".beam">>]), 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, []}]),