clean up and better error messages when validating apps

3 files changed, 62 insertions, 92 deletions

diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl
index aad8985..8add9e7 100644
--- a/src/rebar_app_discover.erl
+++ b/src/rebar_app_discover.erl
@@ -1,12 +1,15 @@
 -module(rebar_app_discover).
 
 -export([do/2,
+         format_error/1,
          find_unbuilt_apps/1,
          find_apps/1,
          find_apps/2,
          find_app/2,
          validate_application_info/1]).
 
+-include_lib("providers/include/providers.hrl").
+
 do(State, LibDirs) ->
     BaseDir = rebar_state:dir(State),
     Dirs = [filename:join(BaseDir, LibDir) || LibDir <- LibDirs],
@@ -17,6 +20,11 @@ do(State, LibDirs) ->
                         rebar_state:project_apps(StateAcc, rebar_app_info:deps(AppInfo, ProjectDeps1))
             end, State, Apps).
 
+format_error({module_list, File}) ->
+    io_lib:format("Error reading module list from ~p~n", [File]);
+format_error({missing_module, Module}) ->
+    io_lib:format("Module defined in app file missing: ~p~n", [Module]).
+
 -spec all_app_dirs(list(file:name())) -> list(file:name()).
 all_app_dirs(LibDirs) ->
     lists:flatmap(fun(LibDir) ->
@@ -142,7 +150,7 @@ validate_application_info(AppInfo) ->
                 {ok, List} ->
                     has_all_beams(EbinDir, List);
                 _Error ->
-                    false
+                    ?PRV_ERROR({modules_list, AppFile})
             end
     end.
 
@@ -158,7 +166,7 @@ get_modules_list(AppFile, AppDetail) ->
             {ok, ModulesList}
     end.
 
--spec has_all_beams(file:filename_all(), list()) -> boolean().
+-spec has_all_beams(file:filename_all(), list()) -> true | providers:error().
 has_all_beams(EbinDir, [Module | ModuleList]) ->
     BeamFile = filename:join([EbinDir,
                               ec_cnv:to_list(Module) ++ ".beam"]),
@@ -166,7 +174,7 @@ has_all_beams(EbinDir, [Module | ModuleList]) ->
         true ->
             has_all_beams(EbinDir, ModuleList);
         false ->
-            false
+            ?PRV_ERROR({missing_module, Module})
     end;
 has_all_beams(_, []) ->
     true.
diff --git a/src/rebar_otp_app.erl b/src/rebar_otp_app.erl
index e1a1eab..4b22ceb 100644
--- a/src/rebar_otp_app.erl
+++ b/src/rebar_otp_app.erl
@@ -27,51 +27,63 @@
 -module(rebar_otp_app).
 
 -export([compile/2,
+         format_error/1,
          clean/2]).
 
-%% for internal use only
--export([info/2]).
-
 -include("rebar.hrl").
+-include_lib("providers/include/providers.hrl").
 
 %% ===================================================================
 %% Public API
 %% ===================================================================
 
-compile(Config, App) ->
+compile(State, App) ->
     %% If we get an .app.src file, it needs to be pre-processed and
     %% written out as a ebin/*.app file. That resulting file will then
     %% be validated as usual.
     Dir = ec_cnv:to_list(rebar_app_info:dir(App)),
-    {Config2, App1} = case rebar_app_info:app_file_src(App) of
+    {State2, App1} = case rebar_app_info:app_file_src(App) of
                           undefined ->
-                              {Config, App};
+                              {State, App};
                           AppFileSrc ->
-                              {Config1, File} = preprocess(Config, Dir, AppFileSrc),
-                              {Config1, rebar_app_info:app_file(App, File)}
+                              {State1, File} = preprocess(State, Dir, AppFileSrc),
+                              {State1, rebar_app_info:app_file(App, File)}
                       end,
 
     %% Load the app file and validate it.
     AppFile = rebar_app_info:app_file(App1),
-    case rebar_app_utils:load_app_file(Config2, AppFile) of
-        {ok, Config3, AppName, AppData} ->
+    case rebar_app_utils:load_app_file(State2, AppFile) of
+        {ok, State3, AppName, AppData} ->
             AppVsn = proplists:get_value(vsn, AppData),
-            validate_name(AppName, AppFile),
-            %% In general, the list of modules is an important thing to validate
-            %% for compliance with OTP guidelines and upgrade procedures.
-            %% However, some people prefer not to validate this list.
-            case rebar_state:get(Config3, validate_app_modules, true) of
-                true ->
-                    Modules = proplists:get_value(modules, AppData),
-                    {validate_modules(Dir, AppName, Modules), rebar_app_info:original_vsn(App1, AppVsn)};
-                false ->
-                    {ok, rebar_app_info:original_vsn(App1, AppVsn)}
+            case validate_name(AppName, AppFile) of
+                ok ->
+                    %% In general, the list of modules is an important thing to validate
+                    %% for compliance with OTP guidelines and upgrade procedures.
+                    %% However, some people prefer not to validate this list.
+                    case rebar_state:get(State3, validate_app_modules, true) of
+                        true ->
+                            case rebar_app_discover:validate_application_info(App1) of
+                                true ->
+                                    {ok, rebar_app_info:original_vsn(App1, AppVsn)};
+                                Error ->
+                                    Error
+                            end;
+                        false ->
+                            {ok, rebar_app_info:original_vsn(App1, AppVsn)}
+                    end;
+                Error ->
+                    Error
             end;
         {error, Reason} ->
-            ?ABORT("Failed to load app file ~s: ~p", [AppFile, Reason])
+            ?PRV_ERROR({file_read, AppFile, Reason})
     end.
 
-clean(_Config, File) ->
+format_error({file_read, File, Reason}) ->
+    io_lib:format("Failed to read ~s for processing: ~p", [File, Reason]);
+format_error({invalid_name, File, AppName}) ->
+    io_lib:format("Invalid ~s: name of application (~p) must match filename.", [File, AppName]).
+
+clean(_State, File) ->
     %% If the app file is a .app.src, delete the generated .app file
     case rebar_app_utils:is_app_src(File) of
         true ->
@@ -92,34 +104,17 @@ clean(_Config, File) ->
 %% Internal functions
 %% ===================================================================
 
-info(help, compile) ->
-    info_help("Validate .app file");
-info(help, clean) ->
-    info_help("Delete .app file if generated from .app.src").
-
-info_help(Description) ->
-    ?CONSOLE(
-       "~s.~n"
-       "~n"
-       "Valid rebar.config options:~n"
-       "  ~p",
-       [
-        Description,
-        {validate_app_modules, true}
-       ]).
-
-preprocess(Config, Dir, AppSrcFile) ->
-    case rebar_app_utils:load_app_file(Config, AppSrcFile) of
-        {ok, Config1, AppName, AppData} ->
+preprocess(State, Dir, AppSrcFile) ->
+    case rebar_app_utils:load_app_file(State, AppSrcFile) of
+        {ok, State1, AppName, AppData} ->
             %% Look for a configuration file with vars we want to
             %% substitute. Note that we include the list of modules available in
             %% ebin/ and update the app data accordingly.
-            AppVars = load_app_vars(Config1) ++ [{modules, ebin_modules(Dir)}],
+            AppVars = load_app_vars(State1) ++ [{modules, ebin_modules(Dir)}],
             A1 = apply_app_vars(AppVars, AppData),
 
-
             %% AppSrcFile may contain instructions for generating a vsn number
-            {Config2, Vsn} = rebar_app_utils:app_vsn(Config1, AppSrcFile),
+            {State2, Vsn} = rebar_app_utils:app_vsn(State1, AppSrcFile),
             A2 = lists:keystore(vsn, 1, A1, {vsn, Vsn}),
 
             %% systools:make_relup/4 fails with {missing_param, registered}
@@ -137,14 +132,13 @@ preprocess(Config, Dir, AppSrcFile) ->
             %% on the code path
             true = code:add_path(filename:absname(filename:dirname(AppFile))),
 
-            {Config2, AppFile};
+            {State2, AppFile};
         {error, Reason} ->
-            ?ABORT("Failed to read ~s for preprocessing: ~p",
-                   [AppSrcFile, Reason])
+            ?PRV_ERROR({file_read, AppSrcFile, Reason})
     end.
 
-load_app_vars(Config) ->
-    case rebar_state:get(Config, app_vars_file, undefined) of
+load_app_vars(State) ->
+    case rebar_state:get(State, app_vars_file, undefined) of
         undefined ->
             ?DEBUG("No app_vars_file defined.", []),
             [];
@@ -168,44 +162,7 @@ validate_name(AppName, File) ->
         true ->
             ok;
         false ->
-            ?ERROR("Invalid ~s: name of application (~p) "
-                   "must match filename.", [File, AppName]),
-            ?FAIL
-    end.
-
-validate_modules(_Dir, AppName, undefined) ->
-    ?ERROR("Missing modules declaration in ~p.app", [AppName]),
-    ?FAIL;
-
-validate_modules(Dir, AppName, Mods) ->
-    %% Construct two sets -- one for the actual .beam files in ebin/
-    %% and one for the modules
-    %% listed in the .app file
-    EbinSet = ordsets:from_list(ebin_modules(Dir)),
-    ModSet = ordsets:from_list(Mods),
-
-    %% Identify .beam files listed in the .app, but not present in ebin/
-    case ordsets:subtract(ModSet, EbinSet) of
-        [] ->
-            ok;
-        MissingBeams ->
-            Msg1 = lists:flatten([io_lib:format("\t* ~p", [M]) ||
-                                     M <- MissingBeams]),
-            ?ERROR("One or more modules listed in ~p.app are not "
-                   "present in ebin/*.beam:~n~s", [AppName, Msg1]),
-            ?FAIL
-    end,
-
-    %% Identify .beam files NOT list in the .app, but present in ebin/
-    case ordsets:subtract(EbinSet, ModSet) of
-        [] ->
-            ok;
-        MissingMods ->
-            Msg2 = lists:flatten([io_lib:format("\t* ~p\n", [M]) ||
-                                     M <- MissingMods]),
-            ?ERROR("One or more .beam files exist that are not "
-                   "listed in ~p.app:\n~s", [AppName, Msg2]),
-            ?FAIL
+            ?PRV_ERROR({invalid_name, File, AppName})
     end.
 
 ebin_modules(Dir) ->
diff --git a/src/rebar_prv_compile.erl b/src/rebar_prv_compile.erl
index b247603..073394c 100644
--- a/src/rebar_prv_compile.erl
+++ b/src/rebar_prv_compile.erl
@@ -85,8 +85,13 @@ build_apps(State, Apps) ->
 build(State, AppInfo) ->
     ?INFO("Compiling ~s", [rebar_app_info:name(AppInfo)]),
     rebar_erlc_compiler:compile(State, ec_cnv:to_list(rebar_app_info:dir(AppInfo))),
-    {ok, AppInfo1} = rebar_otp_app:compile(State, AppInfo),
-    AppInfo1.
+    case rebar_otp_app:compile(State, AppInfo) of
+        {ok, AppInfo1} ->
+            AppInfo1;
+        Error ->
+            throw(Error)
+    end.
+
 
 %% ===================================================================
 %% Internal functions