Merge pull request #470 from tsloughter/master

print and format error message for bad .app files and all bad configs

8 files changed, 69 insertions, 68 deletions

diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl
index 4eda199..34310c7 100644
--- a/src/rebar_app_discover.erl
+++ b/src/rebar_app_discover.erl
@@ -182,25 +182,21 @@ app_dir(AppFile) ->
 
 -spec create_app_info(file:name(), file:name()) -> rebar_app_info:t() | {error, term()}.
 create_app_info(AppDir, AppFile) ->
-    case file:consult(AppFile) of
-        {ok, [{application, AppName, AppDetails}]} ->
-            AppVsn = proplists:get_value(vsn, AppDetails),
-            Applications = proplists:get_value(applications, AppDetails, []),
-            IncludedApplications = proplists:get_value(included_applications, AppDetails, []),
-            {ok, AppInfo} = rebar_app_info:new(AppName, AppVsn, AppDir, []),
-            AppInfo1 = rebar_app_info:applications(
-                         rebar_app_info:app_details(AppInfo, AppDetails),
-                         IncludedApplications++Applications),
-            Valid = case rebar_app_utils:validate_application_info(AppInfo1) of
-                        true ->
-                            true;
-                        _ ->
-                            false
-                    end,
-            rebar_app_info:dir(rebar_app_info:valid(AppInfo1, Valid), AppDir);
-        {error, Reason} ->
-            {error, Reason}
-    end.
+    [{application, AppName, AppDetails}] = rebar_file_utils:try_consult(AppFile),
+    AppVsn = proplists:get_value(vsn, AppDetails),
+    Applications = proplists:get_value(applications, AppDetails, []),
+    IncludedApplications = proplists:get_value(included_applications, AppDetails, []),
+    {ok, AppInfo} = rebar_app_info:new(AppName, AppVsn, AppDir, []),
+    AppInfo1 = rebar_app_info:applications(
+                 rebar_app_info:app_details(AppInfo, AppDetails),
+                 IncludedApplications++Applications),
+    Valid = case rebar_app_utils:validate_application_info(AppInfo1) of
+                true ->
+                    true;
+                _ ->
+                    false
+            end,
+    rebar_app_info:dir(rebar_app_info:valid(AppInfo1, Valid), AppDir).
 
 dedup([]) -> [];
 dedup([A]) -> [A];
diff --git a/src/rebar_app_info.erl b/src/rebar_app_info.erl
index 247e5f4..1b87e0b 100644
--- a/src/rebar_app_info.erl
+++ b/src/rebar_app_info.erl
@@ -41,6 +41,8 @@
          valid/1,
          valid/2]).
 
+-include("rebar.hrl").
+
 -export_type([t/0]).
 
 -record(app_info_t, {name               :: binary(),
@@ -161,17 +163,18 @@ app_file(AppInfo=#app_info_t{}, AppFile) ->
 
 -spec app_details(t()) -> list().
 app_details(AppInfo=#app_info_t{app_details=[]}) ->
-    AppFile = case app_file(AppInfo) of
-                  undefined ->
-                      app_file_src(AppInfo);
-                  File ->
-                      File
-              end,
-    case file:consult(AppFile) of
-        {ok, [{application, _, AppDetails}]} ->
-            AppDetails;
-        _ ->
-            []
+    case app_file(AppInfo) of
+        undefined ->
+            rebar_file_utils:try_consult(app_file_src(AppInfo));
+        AppFile ->
+            try
+                rebar_file_utils:try_consult(AppFile)
+            catch
+                throw:{error, {Module, Reason}} ->
+                    ?DEBUG("Warning, falling back to .app.src because of: ~s",
+                          [Module:format_error(Reason)]),
+                    rebar_file_utils:try_consult(app_file_src(AppInfo))
+            end
     end;
 app_details(#app_info_t{app_details=AppDetails}) ->
     AppDetails.
diff --git a/src/rebar_config.erl b/src/rebar_config.erl
index c858fef..b8222bc 100644
--- a/src/rebar_config.erl
+++ b/src/rebar_config.erl
@@ -57,7 +57,7 @@ consult_file(File) ->
                     {ok, Terms} = consult_and_eval(File, Script),
                     Terms;
                 false ->
-                    try_consult(File)
+                    rebar_file_utils:try_consult(File)
             end
     end.
 
@@ -87,22 +87,12 @@ format_error({bad_dep_name, Dep}) ->
 
 consult_and_eval(File, Script) ->
     ?DEBUG("Evaluating config script ~p", [Script]),
-    StateData = try_consult(File),
+    StateData = rebar_file_utils:try_consult(File),
     file:script(Script, bs([{'CONFIG', StateData}, {'SCRIPT', Script}])).
 
 remove_script_ext(F) ->
     filename:rootname(F, ".script").
 
-try_consult(File) ->
-    case file:consult(File) of
-        {ok, Terms} ->
-            Terms;
-        {error, enoent} ->
-            [];
-        {error, Reason} ->
-            ?ABORT("Failed to read config file ~s:~n ~p", [File, Reason])
-    end.
-
 bs(Vars) ->
     lists:foldl(fun({K,V}, Bs) ->
                         erl_eval:add_binding(K, V, Bs)
diff --git a/src/rebar_file_utils.erl b/src/rebar_file_utils.erl
index ef2c70f..ad30172 100644
--- a/src/rebar_file_utils.erl
+++ b/src/rebar_file_utils.erl
@@ -26,7 +26,9 @@
 %% -------------------------------------------------------------------
 -module(rebar_file_utils).
 
--export([symlink_or_copy/2,
+-export([try_consult/1,
+         format_error/1,
+         symlink_or_copy/2,
          rm_rf/1,
          cp_r/2,
          mv/2,
@@ -37,11 +39,25 @@
          reset_dir/1]).
 
 -include("rebar.hrl").
+-include_lib("providers/include/providers.hrl").
 
 %% ===================================================================
 %% Public API
 %% ===================================================================
 
+try_consult(File) ->
+    case file:consult(File) of
+        {ok, Terms} ->
+            Terms;
+        {error, enoent} ->
+            [];
+        {error, Reason} ->
+            throw(?PRV_ERROR({bad_term_file, File, Reason}))
+    end.
+
+format_error({bad_term_file, AppFile, Reason}) ->
+    io_lib:format("Error reading file ~s: ~s", [AppFile, file:format_error(Reason)]).
+
 symlink_or_copy(Source, Target) ->
     Link = case os:type() of
                {win32, _} ->
diff --git a/src/rebar_otp_app.erl b/src/rebar_otp_app.erl
index c457999..a8c2d62 100644
--- a/src/rebar_otp_app.erl
+++ b/src/rebar_otp_app.erl
@@ -51,10 +51,10 @@ compile(State, App) ->
     %% Load the app file and validate it.
     validate_app(State, App1).
 
-format_error(invalid_app_file) ->
-    "Failed to read app file";
+format_error({missing_app_file, Filename}) ->
+    io_lib:format("App file is missing: ~s", [Filename]);
 format_error({file_read, File, Reason}) ->
-    io_lib:format("Failed to read ~s for processing: ~p", [File, Reason]);
+    io_lib:format("Failed to read app file ~s for processing: ~p", [File, file:format_error(Reason)]);
 format_error({invalid_name, File, AppName}) ->
     io_lib:format("Invalid ~s: name of application (~p) must match filename.", [File, AppName]).
 
@@ -197,7 +197,7 @@ ensure_registered(AppData) ->
 consult_app_file(Filename) ->
     case filelib:is_file(Filename) of
         false ->
-            throw(?PRV_ERROR(invalid_app_file));
+            {error, enoent};
         true ->
             case lists:suffix(".app.src", Filename) of
                 false ->
@@ -214,7 +214,7 @@ app_vsn(AppFile, State) ->
             Resources = rebar_state:resources(State),
             rebar_utils:vcs_vsn(get_value(vsn, AppData, AppFile), AppDir, Resources);
         {error, Reason} ->
-            ?ABORT("Failed to consult app file ~s: ~p\n", [AppFile, Reason])
+            throw(?PRV_ERROR({file_read, AppFile, Reason}))
     end.
 
 get_value(Key, AppInfo, AppFile) ->
diff --git a/src/rebar_prv_app_discovery.erl b/src/rebar_prv_app_discovery.erl
index 97862c1..0e53bb4 100644
--- a/src/rebar_prv_app_discovery.erl
+++ b/src/rebar_prv_app_discovery.erl
@@ -38,7 +38,7 @@ do(State) ->
         State1 = rebar_app_discover:do(State, LibDirs),
         {ok, State1}
     catch
-        throw:{error, Error}->
+        throw:{error, Error} ->
             ?PRV_ERROR(Error)
     end.
 
@@ -46,12 +46,17 @@ do(State) ->
 format_error({multiple_app_files, Files}) ->
     io_lib:format("Multiple app files found in one app dir: ~s", [string:join(Files, " and ")]);
 format_error({invalid_app_file, File, Reason}) ->
-    case Reason of 
+    case Reason of
         {Line, erl_parse, Description} ->
-            io_lib:format("Invalid app file ~s at line ~b: ~p", 
+            io_lib:format("Invalid app file ~s at line ~b: ~p",
                 [File, Line, lists:flatten(Description)]);
         _ ->
             io_lib:format("Invalid app file ~s: ~p", [File, Reason])
     end;
+%% Provide a slightly more informative error message for consult of app file failure
+format_error({rebar_file_utils, {bad_term_file, AppFile, Reason}}) ->
+    io_lib:format("Error in app file ~s: ~s", [rebar_dir:make_relative_path(AppFile,
+                                                                            rebar_dir:get_cwd()),
+                                               file:format_error(Reason)]);
 format_error(Reason) ->
     io_lib:format("~p", [Reason]).
diff --git a/src/rebar_prv_dialyzer.erl b/src/rebar_prv_dialyzer.erl
index 96e2277..95529f8 100644
--- a/src/rebar_prv_dialyzer.erl
+++ b/src/rebar_prv_dialyzer.erl
@@ -237,8 +237,8 @@ ebin_to_info(EbinDir, AppName) ->
             {IncApps ++ DepApps, Files, Warnings};
         {error, enoent} when AppName =:= erts ->
             {[], ebin_files(EbinDir), []};
-        _ ->
-            Error = io_lib:format("Could not parse ~p", [AppFile]),
+        {error, Reason} ->
+            Error = io_lib:format("Could not parse ~s: ~p", [AppFile, file:format_error(Reason)]),
             throw({dialyzer_error, Error})
     end.
 
diff --git a/src/rebar_prv_shell.erl b/src/rebar_prv_shell.erl
index ec2f692..8c0b7ff 100644
--- a/src/rebar_prv_shell.erl
+++ b/src/rebar_prv_shell.erl
@@ -136,20 +136,18 @@ reread_config(State) ->
     case find_config(State) of
         no_config ->
             ok;
-        {ok, ConfigList} ->
+        ConfigList ->
             lists:foreach(fun ({Application, Items}) ->
                                   lists:foreach(fun ({Key, Val}) ->
                                                         application:set_env(Application, Key, Val)
                                                 end,
                                                 Items)
                           end,
-                          ConfigList);
-        {error, Error} ->
-            ?ABORT("Error while attempting to read configuration file: ~p", [Error])
+                          ConfigList)
     end.
 
 % First try the --config flag, then try the relx sys_config
--spec find_config(rebar_state:t()) -> {ok, [tuple()]}|no_config|{error, tuple()}.
+-spec find_config(rebar_state:t()) -> [tuple()] | no_config.
 find_config(State) ->
     case find_config_option(State) of
         no_config ->
@@ -158,7 +156,7 @@ find_config(State) ->
             Result
     end.
 
--spec find_config_option(rebar_state:t()) -> {ok, [tuple()]}|no_config|{error, tuple()}.
+-spec find_config_option(rebar_state:t()) -> [tuple()] | no_config.
 find_config_option(State) ->
     {Opts, _} = rebar_state:command_parsed_args(State),
     case proplists:get_value(config, Opts) of
@@ -168,7 +166,7 @@ find_config_option(State) ->
             consult_config(State, Filename)
     end.
 
--spec find_config_relx(rebar_state:t()) -> {ok, [tuple()]}|no_config|{error, tuple()}.
+-spec find_config_relx(rebar_state:t()) -> [tuple()] | no_config.
 find_config_relx(State) ->
     case proplists:get_value(sys_config, rebar_state:get(State, relx, [])) of
         undefined ->
@@ -181,11 +179,4 @@ find_config_relx(State) ->
 consult_config(State, Filename) ->
     Fullpath = filename:join(rebar_dir:root_dir(State), Filename),
     ?DEBUG("Loading configuration from ~p", [Fullpath]),
-    case file:consult(Fullpath) of
-        {ok, [Config]} ->
-            {ok, Config};
-        {ok, []} ->
-            {ok, []};
-        {error, Error} ->
-            {error, {Error, Fullpath}}
-    end.
+    rebar_file_utils:try_consult(Fullpath).