properly support top level app erl_opts from REBAR_CONFIG os var (#1889)

When REBAR_CONFIG was set it would not effect the top level app's
configuration because app_discover was rereading the top level
rebar.config which ignored REBAR_CONFIG. Instead this patch has
it use the existing configuration from REBAR_CONFIG.

4 files changed, 54 insertions, 39 deletions

diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl
index 9b1346d..224539b 100644
--- a/src/rebar_app_discover.erl
+++ b/src/rebar_app_discover.erl
@@ -7,7 +7,7 @@
          find_unbuilt_apps/1,
          find_apps/1,
          find_apps/2,
-         find_apps/3,
+         find_apps/4,
          find_app/2,
          find_app/3]).
 
@@ -23,7 +23,7 @@ do(State, LibDirs) ->
     Dirs = [filename:join(BaseDir, LibDir) || LibDir <- LibDirs],
     RebarOpts = rebar_state:opts(State),
     SrcDirs = rebar_dir:src_dirs(RebarOpts, ["src"]),
-    Apps = find_apps(Dirs, SrcDirs, all),
+    Apps = find_apps(Dirs, SrcDirs, all, State),
     ProjectDeps = rebar_state:deps_names(State),
     DepsDir = rebar_dir:deps_dir(State),
     CurrentProfiles = rebar_state:current_profiles(State),
@@ -52,7 +52,7 @@ do(State, LibDirs) ->
                         Name = rebar_app_info:name(AppInfo),
                         case enable(State, AppInfo) of
                             true ->
-                                {AppInfo1, StateAcc1} = merge_deps(AppInfo, StateAcc),
+                                {AppInfo1, StateAcc1} = merge_opts(AppInfo, StateAcc),
                                 OutDir = filename:join(DepsDir, Name),
                                 AppInfo2 = rebar_app_info:out_dir(AppInfo1, OutDir),
                                 ProjectDeps1 = lists:delete(Name, ProjectDeps),
@@ -87,33 +87,34 @@ format_error({module_list, File}) ->
 format_error({missing_module, Module}) ->
     io_lib:format("Module defined in app file missing: ~p~n", [Module]).
 
-%% @doc handles the merging and application of profiles and overrides
-%% for a given application, within its own context.
--spec merge_deps(rebar_app_info:t(), rebar_state:t()) ->
+%% @doc merges configuration of a project app and the top level state
+%% some configuration like erl_opts must be merged into a subapp's opts
+%% while plugins and hooks need to be kept defined to only either the
+%% top level state or an individual application.
+-spec merge_opts(rebar_app_info:t(), rebar_state:t()) ->
     {rebar_app_info:t(), rebar_state:t()}.
-merge_deps(AppInfo, State) ->
+merge_opts(AppInfo, State) ->
     %% These steps make sure that hooks and artifacts are run in the context of
     %% the application they are defined at. If an umbrella structure is used and
     %% they are defined at the top level they will instead run in the context of
     %% the State and at the top level, not as part of an application.
     CurrentProfiles = rebar_state:current_profiles(State),
-    Default = reset_hooks(rebar_state:default(State), CurrentProfiles),
-    {AppInfo0, State1} = project_app_config(AppInfo, Default, State),
+    {AppInfo1, State1} = maybe_reset_hooks_plugins(AppInfo, State),
 
-    Name = rebar_app_info:name(AppInfo0),
+    Name = rebar_app_info:name(AppInfo1),
 
     %% We reset the opts here to default so no profiles are applied multiple times
-    AppInfo1 = rebar_app_info:apply_overrides(rebar_state:get(State1, overrides, []), AppInfo0),
-    AppInfo2 = rebar_app_info:apply_profiles(AppInfo1, CurrentProfiles),
+    AppInfo2 = rebar_app_info:apply_overrides(rebar_state:get(State1, overrides, []), AppInfo1),
+    AppInfo3 = rebar_app_info:apply_profiles(AppInfo2, CurrentProfiles),
 
     %% Will throw an exception if checks fail
-    rebar_app_info:verify_otp_vsn(AppInfo2),
+    rebar_app_info:verify_otp_vsn(AppInfo3),
 
     State2 = lists:foldl(fun(Profile, StateAcc) ->
-                                 handle_profile(Profile, Name, AppInfo2, StateAcc)
+                                 handle_profile(Profile, Name, AppInfo3, StateAcc)
                          end, State1, lists:reverse(CurrentProfiles)),
 
-    {AppInfo2, State2}.
+    {AppInfo3, State2}.
 
 %% @doc Applies a given profile for an app, ensuring the deps
 %% match the context it will require.
@@ -151,25 +152,15 @@ parse_profile_deps(Profile, Name, Deps, Opts, State) ->
                               ,Locks
                               ,1).
 
-%% @doc Find the app-level config and return the state updated
-%% with the relevant app-level data.
--spec project_app_config(rebar_app_info:t(), rebar_dict(), rebar_state:t()) ->
-    {Config, rebar_state:t()} when
-      Config :: [any()].
-project_app_config(AppInfo, Default, State) ->
-    C = rebar_config:consult(rebar_app_info:dir(AppInfo)),
-    AppInfo1 = rebar_app_info:update_opts(AppInfo, Default, C),
-    {AppInfo2, State1} = maybe_reset_hooks_plugins(AppInfo1, State),
-    {AppInfo2, State1}.
-
+%% reset the State hooks if there is a top level application
 -spec maybe_reset_hooks_plugins(AppInfo, State) ->  {AppInfo, State} when
       AppInfo :: rebar_app_info:t(),
       State :: rebar_state:t().
 maybe_reset_hooks_plugins(AppInfo, State) ->
     Dir = rebar_app_info:dir(AppInfo),
+    CurrentProfiles = rebar_state:current_profiles(State),
     case ec_file:real_dir_path(rebar_dir:root_dir(State)) of
         Dir ->
-            CurrentProfiles = rebar_state:current_profiles(State),
             Opts = reset_hooks(rebar_state:opts(State), CurrentProfiles),
             State1 = rebar_state:opts(State, Opts),
 
@@ -179,7 +170,11 @@ maybe_reset_hooks_plugins(AppInfo, State) ->
 
             {AppInfo1, State1};
         _ ->
-            {AppInfo, State}
+            %% if not in the top root directory then we need to merge in the
+            %% default state opts to this subapp's opts
+            Default = reset_hooks(rebar_state:default(State), CurrentProfiles),
+            AppInfo1 = rebar_app_info:update_opts(AppInfo, Default),
+            {AppInfo1, State}
     end.
 
 
@@ -211,8 +206,8 @@ reset_hooks(Opts, CurrentProfiles) ->
 -spec all_app_dirs([file:name()]) -> [{file:name(), [file:name()]}].
 all_app_dirs(LibDirs) ->
     lists:flatmap(fun(LibDir) ->
-                        {_, SrcDirs} = find_config_src(LibDir, ["src"]),
-                        app_dirs(LibDir, SrcDirs)
+                          {_, SrcDirs} = find_config_src(LibDir, ["src"]),
+                          app_dirs(LibDir, SrcDirs)
                   end, LibDirs).
 
 %% @private find the directories for all apps based on their source dirs
@@ -270,11 +265,11 @@ find_apps(LibDirs, Validate) ->
 %% @doc for each directory passed, with the configured source directories,
 %% find all apps according to the validity rule passed in.
 %% Returns all the related app info records.
--spec find_apps([file:filename_all()], [file:filename_all()], valid | invalid | all) -> [rebar_app_info:t()].
-find_apps(LibDirs, SrcDirs, Validate) ->
+-spec find_apps([file:filename_all()], [file:filename_all()], valid | invalid | all, rebar_state:t()) -> [rebar_app_info:t()].
+find_apps(LibDirs, SrcDirs, Validate, State) ->
     rebar_utils:filtermap(
       fun({AppDir, AppSrcDirs}) ->
-            find_app(rebar_app_info:new(), AppDir, AppSrcDirs, Validate)
+            find_app(rebar_app_info:new(), AppDir, AppSrcDirs, Validate, State)
       end,
       all_app_dirs(LibDirs, SrcDirs)
     ).
@@ -305,8 +300,19 @@ find_app(AppInfo, AppDir, Validate) ->
 %% the directories where source files can be located. Returns the related
 %% app info record.
 -spec find_app(rebar_app_info:t(), file:filename_all(),
-               [file:filename_all()], valid | invalid | all) ->
+               [file:filename_all()], valid | invalid | all, rebar_state:t()) ->
     {true, rebar_app_info:t()} | false.
+find_app(AppInfo, AppDir, SrcDirs, Validate, State) ->
+    AppInfo1 = case ec_file:real_dir_path(rebar_dir:root_dir(State)) of
+                   AppDir ->
+                       Opts = rebar_state:opts(State),
+                       rebar_app_info:default(rebar_app_info:opts(AppInfo, Opts), Opts);
+                   _ ->
+                       Config = rebar_config:consult(AppDir),
+                       rebar_app_info:update_opts(AppInfo, rebar_app_info:opts(AppInfo), Config)
+               end,
+    find_app_(AppInfo1, AppDir, SrcDirs, Validate).
+
 find_app(AppInfo, AppDir, SrcDirs, Validate) ->
     Config = rebar_config:consult(AppDir),
     AppInfo1 = rebar_app_info:update_opts(AppInfo, rebar_app_info:opts(AppInfo), Config),
diff --git a/src/rebar_app_info.erl b/src/rebar_app_info.erl
index 56ae4c0..eb95311 100644
--- a/src/rebar_app_info.erl
+++ b/src/rebar_app_info.erl
@@ -7,6 +7,7 @@
          new/4,
          new/5,
          update_opts/3,
+         update_opts/2,
          update_opts_deps/2,
          discover/1,
          name/1,
@@ -158,7 +159,7 @@ update_opts(AppInfo, Opts, Config) ->
                        %% don't set anything here.
                        [];
                    _ ->
-                       deps_from_config(dir(AppInfo), Config)
+                       deps_from_config(dir(AppInfo), proplists:get_value(deps, Config, []))
                end,
 
     Plugins = proplists:get_value(plugins, Config, []),
@@ -171,6 +172,13 @@ update_opts(AppInfo, Opts, Config) ->
     AppInfo#app_info_t{opts=NewOpts,
                        default=NewOpts}.
 
+%% @doc update current app info opts by merging in a new dict of opts
+-spec update_opts(t(), rebar_dict()) -> t().
+update_opts(AppInfo=#app_info_t{opts=LocalOpts}, Opts) ->
+    NewOpts = rebar_opts:merge_opts(LocalOpts, Opts),
+    AppInfo#app_info_t{opts=NewOpts,
+                       default=NewOpts}.
+
 %% @doc update the opts based on new deps, usually from an app's hex registry metadata
 -spec update_opts_deps(t(), [any()]) -> t().
 update_opts_deps(AppInfo=#app_info_t{opts=Opts}, Deps) ->
@@ -183,10 +191,10 @@ update_opts_deps(AppInfo=#app_info_t{opts=Opts}, Deps) ->
 
 %% @private extract the deps for an app in `Dir' based on its config file data
 -spec deps_from_config(file:filename(), [any()]) -> [{tuple(), any()}, ...].
-deps_from_config(Dir, Config) ->
+deps_from_config(Dir, ConfigDeps) ->
     case rebar_config:consult_lock_file(filename:join(Dir, ?LOCK_FILE)) of
         [] ->
-            [{{deps, default}, proplists:get_value(deps, Config, [])}];
+            [{{deps, default}, ConfigDeps}];
         D ->
             %% We want the top level deps only from the lock file.
             %% This ensures deterministic overrides for configs.
diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl
index 57e5398..c63ca5b 100644
--- a/src/rebar_erlc_compiler.erl
+++ b/src/rebar_erlc_compiler.erl
@@ -99,6 +99,7 @@ compile(AppInfo, CompileOpts) when element(1, AppInfo) == app_info_t ->
                {recursive, dir_recursive(RebarOpts, "src", CompileOpts)}],
     MibsOpts = [check_last_mod,
                 {recursive, dir_recursive(RebarOpts, "mibs", CompileOpts)}],
+
     rebar_base_compiler:run(RebarOpts,
                             check_files([filename:join(Dir, File)
                                          || File <- rebar_opts:get(RebarOpts, xrl_first_files, [])]),
diff --git a/src/rebar_prv_plugins.erl b/src/rebar_prv_plugins.erl
index 4bea3b3..d66b645 100644
--- a/src/rebar_prv_plugins.erl
+++ b/src/rebar_prv_plugins.erl
@@ -36,7 +36,7 @@ do(State) ->
     GlobalPlugins = rebar_state:get(GlobalConfig, plugins, []),
     GlobalSrcDirs = rebar_state:get(GlobalConfig, src_dirs, ["src"]),
     GlobalPluginsDir = filename:join([rebar_dir:global_cache_dir(rebar_state:opts(State)), "plugins", "*"]),
-    GlobalApps = rebar_app_discover:find_apps([GlobalPluginsDir], GlobalSrcDirs, all),
+    GlobalApps = rebar_app_discover:find_apps([GlobalPluginsDir], GlobalSrcDirs, all, State),
     display_plugins("Global plugins", GlobalApps, GlobalPlugins),
 
     RebarOpts = rebar_state:opts(State),
@@ -44,7 +44,7 @@ do(State) ->
     Plugins = rebar_state:get(State, plugins, []),
     PluginsDirs = filelib:wildcard(filename:join(rebar_dir:plugins_dir(State), "*")),
     CheckoutsDirs = filelib:wildcard(filename:join(rebar_dir:checkouts_dir(State), "*")),
-    Apps = rebar_app_discover:find_apps(CheckoutsDirs++PluginsDirs, SrcDirs, all),
+    Apps = rebar_app_discover:find_apps(CheckoutsDirs++PluginsDirs, SrcDirs, all, State),
     display_plugins("Local plugins", Apps, Plugins),
     {ok, State}.