Merge pull request #1907 from ferd/refactor-env-paths

Refactor env path handling and fix some bugs related to it

src/rebar3.erl

@@ -175,7 +175,20 @@ run_aux(State, RawArgs) ->
 
     State10 = rebar_state:code_paths(State9, default, code:get_path()),
 
-    rebar_core:init_command(rebar_state:command_args(State10, Args), Task).
+    case rebar_core:init_command(rebar_state:command_args(State10, Args), Task) of
+        {ok, State11} ->
+            case rebar_state:get(State11, caller, command_line) of
+                api ->
+                    rebar_paths:unset_paths([deps, plugins], State11),
+                    {ok, State11};
+                _ ->
+                    {ok, State11}
+            end;
+        Other ->
+            Other
+    end.
+
+
 
 %% @doc set up base configuration having to do with verbosity, where
 %% to find config files, and so on, and return an internal rebar3 state term.

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) ->

src/rebar_compiler.erl

@@ -34,11 +34,10 @@
 -define(RE_PREFIX, "^(?!\\._)").
 
 compile_all(Compilers, AppInfo) ->
-    OutDir = rebar_utils:to_list(rebar_app_info:out_dir(AppInfo)),
-
+    EbinDir = rebar_utils:to_list(rebar_app_info:ebin_dir(AppInfo)),
     %% Make sure that outdir is on the path
-    ok = rebar_file_utils:ensure_dir(OutDir),
-    true = code:add_patha(filename:absname(OutDir)),
+    ok = rebar_file_utils:ensure_dir(EbinDir),
+    true = code:add_patha(filename:absname(EbinDir)),
 
     %% necessary for erlang:function_exported/3 to work as expected
     %% called here for clarity as it's required by both opts_changed/2

src/rebar_hooks.erl

@@ -42,8 +42,7 @@ 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),
+            rebar_paths:set_paths([plugins], 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 +50,7 @@ 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_paths:set_paths([deps], State2),
                     State2
             end
     end.

src/rebar_packages.erl

@@ -84,7 +84,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}).
 

src/rebar_paths.erl

@@ -0,0 +1,208 @@
+-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]).
+-export([clashing_apps/2]).
+
+-ifdef(TEST).
+-export([misloaded_modules/2]).
+-endif.
+
+-spec set_paths(targets(), rebar_state:t()) -> ok.
+set_paths(UserTargets, State) ->
+    Targets = normalize_targets(UserTargets),
+    GroupPaths = path_groups(Targets, State),
+    Paths = lists:append(lists:reverse([P || {_, P} <- GroupPaths])),
+    code:add_pathsa(Paths),
+    AppGroups = app_groups(Targets, State),
+    purge_and_load(AppGroups, 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.
+
+-spec clashing_apps(targets(), rebar_state:t()) -> [{target(), [binary()]}].
+clashing_apps(Targets, State) ->
+    AppGroups = app_groups(Targets, State),
+    AppNames = [{G, sets:from_list(
+                    [rebar_app_info:name(App) || App <- Apps]
+                )} || {G, Apps} <- AppGroups],
+    clashing_app_names(sets:new(), AppNames, []).
+
+%%%%%%%%%%%%%%%
+%%% PRIVATE %%%
+%%%%%%%%%%%%%%%
+
+%% 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], 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)
+    %%    This part turns out to be the bottleneck of this module, so
+    %%    to speed it up, using clash detection proves useful:
+    %%    only reload apps that clashed since others are unlikely to
+    %%    conflict in significant ways
+    %% 4. create a list of modules to check from that app list—only loaded
+    %%    modules make sense to check.
+    %% 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)
+    %% (no need for extra_src_dirs since those get put into ebin;
+    %%  also no need for OTP libs; we want to allow overtaking them)
+    GoodAppPaths = [rebar_app_info:ebin_dir(App) || App <- GoodApps],
+    %% 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)]]
+    ),
+    ModPaths = [{Mod,Path} || Mod <- CandidateMods,
+                              erlang:function_exported(Mod, module_info, 0),
+                              {file, Path} <- [code:is_loaded(Mod)]],
+
+    %% 5)
+    Mods = misloaded_modules(GoodAppPaths, ModPaths),
+    [purge_mod(Mod) || Mod <- Mods],
+
+    purge_and_load(Rest, sets:union(Seen, sets:from_list(AppNames))).
+
+purge(Paths, ModPaths) ->
+    SortedPaths = lists:sort(Paths),
+    lists:map(fun purge_mod/1,
+              [Mod || {Mod, Path} <- ModPaths,
+                      is_list(Path), % not 'preloaded' or mocked
+                      any_prefix(Path, SortedPaths)]
+             ).
+
+misloaded_modules(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(
+        [Mod || {Mod, Path} <- 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) ->
+    code:soft_purge(Mod) andalso code:delete(Mod).
+
+
+%% This is a tricky O(n²) check since we want to
+%% know whether an app clashes with any of the top priority groups.
+%%
+%% For example, let's say we have `[deps, plugins]', then we want
+%% to find the plugins that clash with deps:
+%%
+%% `[{deps, [ClashingPlugins]}, {plugins, []}]'
+%%
+%% In case we'd ever have alternative or additional types, we can
+%% find all clashes from other 'groups'.
+clashing_app_names(_, [], Acc) ->
+    lists:reverse(Acc);
+clashing_app_names(PrevNames, [{G,AppNames} | Rest], Acc) ->
+    CurrentNames = sets:subtract(AppNames, PrevNames),
+    NextNames = sets:subtract(sets:union([A || {_, A} <- Rest]), PrevNames),
+    Clashes = sets:intersection(CurrentNames, NextNames),
+    NewAcc = [{G, sets:to_list(Clashes)} | Acc],
+    clashing_app_names(sets:union(PrevNames, CurrentNames), Rest, NewAcc).
+
+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
+    case rebar_state:project_apps(State) of
+        undefined -> [];
+        List -> List
+    end ++
+    rebar_state:all_deps(State);
+get_apps(plugins, State) ->
+    rebar_state:all_plugin_deps(State).

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], State4),
 
         {plugin_providers(Plugin), State4}
     catch

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).

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], 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], 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
@@ -119,7 +115,7 @@ copy_and_build_project_apps(State, Providers, Apps) ->
                    rebar_app_info:dir(AppInfo),
                    rebar_app_info:out_dir(AppInfo))
      || AppInfo <- Apps],
-    code:add_pathsa([rebar_app_info:out_dir(AppInfo) || AppInfo <- Apps]),
+    code:add_pathsa([rebar_app_info:ebin_dir(AppInfo) || AppInfo <- Apps]),
     [compile(State, Providers, AppInfo) || AppInfo <- Apps].
 
 
@@ -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], State),
     AppFileCompileResult = rebar_otp_app:compile(State, AppInfo4),
-    %% Clean up after ourselves, leave things as they were.
-    rebar_utils:remove_from_code_path(PluginDepsPaths),
+    %% Clean up after ourselves, leave things as they were with deps first
+    rebar_paths:set_paths([deps], 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], 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, []).

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

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});

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.
 

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], State),
     XrefChecks = prepare(State),
     XrefIgnores = rebar_state:get(State, xref_ignores, []),
     %% Run xref checks
@@ -48,7 +47,6 @@ 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),
     case XrefResults =:= [] andalso QueryResults =:= [] of
         true ->
             {ok, State};

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

test/rebar_paths_SUITE.erl

@@ -0,0 +1,240 @@
+-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,
+     misloaded_mods
+    ].
+
+%%%%%%%%%%%%%%%%%%
+%%% TEST SETUP %%%
+%%%%%%%%%%%%%%%%%%
+
+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).
+
+%%%%%%%%%%%%%
+%%% TESTS %%%
+%%%%%%%%%%%%%
+
+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([], 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.
+
+misloaded_mods(_Config) ->
+    Res = rebar_paths:misloaded_modules(
+      ["/1/2/3/4",
+       "/1/2/4",
+       "/2/1/1",
+       "/3/4/5"],
+      [{a, "/0/1/2/file.beam"},
+       {b, "/1/2/3/4/file.beam"},
+       {c, "/2/1/file.beam"},
+       {f, preloaded},
+       {d, "/3/5/7/file.beam"},
+       {e, "/3/4/5/file.beam"}]
+    ),
+    ?assertEqual([a,c,d], Res),
+    ok.
+
+%%%%%%%%%%%%%%%
+%%% HELPERS %%%
+%%%%%%%%%%%%%%%
+
+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.

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, []}]),